[libspirv][remangler] Don't remangle internal functions. #20148
Conversation
There is no need to remangle functions with local (internal/private) linkage, they won't be called from outside libclc. Skipping them avoids issues with compiler added suffixes like ".0". We could fix remangling to handle those suffixes, but skipping them is simpler, and we don't expect suffixes on externally visible functions anyway. Skipping over internal functions could also improve performance when remangling large modules with many internal functions. This fixes an issue we see when building libclc for a downstream target, the error was: ```plaintext [11/22] Generating ../../lib/clc/remangled-l32-signed_char.libspirv-<target-triple>.bc FAILED: lib/clc/remangled-l32-signed_char.libspirv-<target-triple>.bc llvm/lib/clc/remangled-l32-signed_char.libspirv-<target-triple>.bc cd llvm/tools/libclc && cmake -E make_directory llvm/./lib/clc && llvm/bin/libclc-remangler -o llvm/./lib/clc/remangled-l32-signed_char.libspirv-<target-triple>.bc --triple=<target-triple> --long-width=l32 --char-signedness=signed --input-ir=llvm/./lib/clc/libspirv-<target-triple>.bc llvm/./lib/clc/libclc_dummy_in.cc Unhandled type: __clc_copysign(float, float) Unhandled type. UNREACHABLE executed at llvm/libclc/utils/libclc-remangler/LibclcRemangler.cpp:590! ``` due to a function named `@_Z14__clc_copysignff.9` with internal linkage.
Maetveis
temporarily deployed
to
WindowsCILock
GitHub Actions
Inactive
Maetveis
temporarily deployed
to
WindowsCILock
GitHub Actions
Inactive
Maetveis
had a problem deploying
to
WindowsCILock
GitHub Actions
Failure
| } | ||
|
|
||
| bool remangleFunction(Function &Func, llvm::Module *M) { | ||
| if (Func.hasLocalLinkage()) |
There was a problem hiding this comment.
IIUC remangling is handling function parameter type and address space, but is unrelated to function linkage type.
I think the remangling process could be refined to skip functions like __clc_copysign whose parameter types don't need remangling.
There was a problem hiding this comment.
IIUC remangling is handling function parameter type and address space, but is unrelated to function linkage type.
Isn't the purpose of remangling that code compiled in SYCL (C++) language mode be able to call into libspirv functions which are compiled as OpenCL C, as there are differences between OpenCL and C++ with regards to e.g. long long size and char signedness?
If there's a function in libspirv that is internal linkage user code (the code we're linking libspirv into at user code compile time) shouldn't be calling it by definition. If it isn't being called by user code there can be no issues with C++ code looking for a different mangled name than what libspirv would provide.
I think the remangling process could be refined to skip functions like __clc_copysign whose parameter types don't need remangling.
The same problem could occur with functions that have types that need renaming.
There was a problem hiding this comment.
If there's a function in libspirv that is internal linkage user code (the code we're linking libspirv into at user code compile time) shouldn't be calling it by definition. If it isn't being called by user code there can be no issues with C++ code looking for a different mangled name than what libspirv would provide.
Right. It seems you're correct.
There was a problem hiding this comment.
I believe @Maetveis is right. The point of remangling is to give C++ code the opportunity to link against the OpenCL-C-based libclc, without there being signedness/size issues. Since functions with internal linkage shouldn't interface with C++ it should be fine to skip them during remangling.
Maetveis
temporarily deployed
to
WindowsCILock
GitHub Actions
Inactive
|
@intel/llvm-gatekeepers please consider merging |
3 participants
There is no need to remangle functions with local (internal/private) linkage, they won't be called from outside libclc.
Skipping them avoids issues with compiler added suffixes like ".0". We could fix remangling to handle those suffixes, but skipping them is simpler, and we don't expect suffixes on externally visible functions anyway.
Skipping over internal functions could also improve performance when remangling large modules with many internal functions.
This fixes an issue we see when building libclc for a downstream target, where we build libspirv from smaller bitcode files, the error there was:
due to a function named
@_Z14__clc_copysignff.9with internal linkage. The function was likely created byllvm-linkwhen merging multiple copies ofcopysigninto the resulting module.