================
@@ -454,10 +454,22 @@ function(add_libclc_builtin_set)
 
     set( obj_suffix ${ARG_ARCH_SUFFIX}.bc )
     set( libclc_builtins_lib ${LIBCLC_OUTPUT_LIBRARY_DIR}/${obj_suffix} )
-    add_custom_command( OUTPUT ${libclc_builtins_lib}
-      COMMAND ${prepare_builtins_exe} -o ${libclc_builtins_lib} 
${builtins_opt_lib}
-      DEPENDS ${builtins_opt_lib} ${builtins_opt_lib_tgt} 
${prepare_builtins_target}
-    )
+    if( TARGET prepare_builtins )
+      # FIXME: Is this utility even necessary? The `-mlink-builtin-bitcode`
+      # option used to link the library in discards the modified linkage.
----------------
wenju-he wrote:

> This is probably a leftover from before we did that.

Thanks for the information. I wasn't aware of it.

> Unfortunately I don't know how people really use `libclc`. I'd really like to 
> just remove it but I can imagine someone complaining about this being gone, 
> because `-mlink-builtin-bitcode` makes the linkage change unnecessary and the 
> linker deduplication now makes the manual handling unnecessary

Our downstream targets also use `-mlink-builtin-bitcode` to link libclc bitcode 
files.
I think it is probably fine to drop this prepare_builtins utility given that 
the linkage change is unnecessary. I have just tried to drop the linkage change 
in our downstream code and doesn't find anything wrong in some basic testing.

There are a few additional changes that prepare_builtins do in our downstream 
code, e.g.
https://github.com/intel/llvm/blob/4decbf0da29f7daba8a87361456a264a331e2b5d/libclc/utils/prepare-builtins.cpp#L85-L110
https://github.com/intel/llvm/blob/4decbf0da29f7daba8a87361456a264a331e2b5d/libclc/utils/prepare-builtins.cpp#L129-L146
I'll check if they can be removed in the downstream as well.

https://github.com/llvm/llvm-project/pull/174022
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to