[libclc] Don't run remangler on OpenCL library #18697
Conversation
The remangler is for libspirv. Don't run remangler on OpenCL library until there is a use case. This would avoids confusions on unused remangled OpenCL library in build folder.
libclc/cmake/modules/AddLibclc.cmake
Outdated
| @@ -483,7 +483,7 @@ function(add_libclc_builtin_set) | |||
| install( FILES ${builtins_lib} DESTINATION ${CMAKE_INSTALL_DATADIR}/clc ) | |||
|
|
|||
| # Generate remangled variants if requested | |||
| if( LIBCLC_GENERATE_REMANGLED_VARIANTS ) | |||
| if( LIBCLC_GENERATE_REMANGLED_VARIANTS AND "${ARG_TARGET_ENV} " MATCHES "^libspirv" ) | |||
There was a problem hiding this comment.
Just because matching ARG_TARGET_ENV with a regex is a little gnarly we could perhaps alternatively pass in something based on LIBCLC_GENERATE_REMANGLED_VARIANTS as an arg to add_libclc_builtin_set, and only pass it in for the libspirv version?
That said, this is all pretty messy (checking and changing obj_suffix, below) and could do with a refactor. Perhaps we can get rid of the remangler entirely.
There was a problem hiding this comment.
Just because matching
ARG_TARGET_ENVwith a regex is a little gnarly we could perhaps alternatively pass in something based onLIBCLC_GENERATE_REMANGLED_VARIANTSas an arg toadd_libclc_builtin_set, and only pass it in for the libspirv version?
done, thanks
Perhaps we can get rid of the remangler entirely.
Do you mean we don't run remangler on libspirv? Can we rename built-in in device code when necessary in a pass during linking of libspirv?
There was a problem hiding this comment.
I'm still not 100% sure, we haven't started looking at it in detail. Perhaps the compiler could provide headers that forward the host types on to the OpenCL types. Or perhaps we could change the OpenCL mangling itself via an extension or compiler option. There's nothing in the OpenCL spec to say that a char or long has to be mangled in a specific way.
Or yes it could perhaps be a compiler pass - but moving things out of the 'clang' space is likely going to be difficult, since we currently rely on clang for the remangler. I suspect having it in LLVM would lead to more assumptions. Basically I'd really like to avoid doing any actual demangling and remangling as it's not very robust,.. or nice.
There was a problem hiding this comment.
Perhaps the compiler could provide headers that forward the host types on to the OpenCL types. Or perhaps we could change the OpenCL mangling itself via an extension or compiler option. There's nothing in the OpenCL spec to say that a
charorlonghas to be mangled in a specific way.
thanks @frasercrmck, it looks very promising to me. Without the above support, renaming is always required for libspirv, making it awkward to compile libspirv using OpenCL C.
|
@intel/llvm-gatekeepers please merge, thanks, BMG fail is probably infrastructure issue because they pass in the runs of the first commit. |
The remangler is for libspirv. Don't run remangler on OpenCL library until there is a use case. This would avoids confusions on unused remangled OpenCL library in build folder.