[libclc] [libclc] Fix dependencies on generated convert builtins (PR #127515)

[Fraser Cormack via cfe-commits]

https://github.com/frasercrmck created https://github.com/llvm/llvm-project/pull/127515

In #127378 it was reported that builds without clspv targets enabled were failing after #124727, as all targets had a dependency on a file that only clspv targets generated.

A quick fix was merged in #127315 which wasn't correct. It moved the dependency on those generated files to the spirv targets, instead of onto the clspv targets. This means a build with spirv targets and without clspv targets would see the same problems as #127378 reported.

I tried simply removing the requirement to explicitly add dependencies to the custom command, relying instead on the file-level dependencies. This didn't seem reliable enough; in some cases on a Makefiles build, the clang command compiling (e.g.,) convert.cl would begin before the file was fully written.

Instead, we keep the target-level dependency but automatically infer it based on the generated file name, to avoid manual book-keeping of pairs of files and targets.

This commit also fixes what looks like an unintended bug where, when ENABLE_RUNTIME_SUBNORMAL was enabled, the OpenCL conversions weren't being compiled.

>From fcbce268befa82d9580c9437c7b6327c251a5299 Mon Sep 17 00:00:00 2001
From: Fraser Cormack <fraser at codeplay.com>
Date: Mon, 17 Feb 2025 15:59:09 +0000
Subject: [PATCH] [libclc] Fix dependencies on generated convert builtins

In #127378 it was reported that builds without clspv targets enabled
were failing after #124727, as all targets had a dependency on a file
that only clspv targets generated.

A quick fix was merged in #127315 which wasn't correct. It moved the
dependency on those generated files to the spirv targets, instead of
onto the clspv targets. This means a build with spirv targets and
without clspv targets would see the same problems as #127378 reported.

I tried simply removing the requirement to explicitly add dependencies
to the custom command, relying instead on the file-level dependencies.
This didn't seem reliable enough; in some cases on a Makefiles build,
the clang command compiling (e.g.,) convert.cl would begin before the
file was fully written.

Instead, we keep the target-level dependency but automatically infer it
based on the generated file name, to avoid manual book-keeping of
pairs of files and targets.

This commit also fixes what looks like an unintended bug where, when
ENABLE_RUNTIME_SUBNORMAL was enabled, the OpenCL conversions weren't
being compiled.
---
 libclc/CMakeLists.txt                | 22 ++++++++++++----------
 libclc/cmake/modules/AddLibclc.cmake | 14 +++++++-------
 2 files changed, 19 insertions(+), 17 deletions(-)

diff --git a/libclc/CMakeLists.txt b/libclc/CMakeLists.txt
index c88ea9700d100..5cefa8a264310 100644
--- a/libclc/CMakeLists.txt
+++ b/libclc/CMakeLists.txt
@@ -243,30 +243,30 @@ add_custom_command(
   OUTPUT convert.cl
   COMMAND ${Python3_EXECUTABLE} ${script_loc} > convert.cl
   DEPENDS ${script_loc} )
-add_custom_target( "generate_convert.cl" DEPENDS convert.cl )
-set_target_properties( "generate_convert.cl" PROPERTIES FOLDER "libclc/Sourcegenning" )
+add_custom_target( generate-convert.cl DEPENDS convert.cl )
+set_target_properties( generate-convert.cl PROPERTIES FOLDER "libclc/Sourcegenning" )
 
 add_custom_command(
   OUTPUT clc-convert.cl
   COMMAND ${Python3_EXECUTABLE} ${script_loc} --clc > clc-convert.cl
   DEPENDS ${script_loc} )
-add_custom_target( "clc-generate_convert.cl" DEPENDS clc-convert.cl )
-set_target_properties( "clc-generate_convert.cl" PROPERTIES FOLDER "libclc/Sourcegenning" )
+add_custom_target( generate-clc-convert.cl DEPENDS clc-convert.cl )
+set_target_properties( generate-clc-convert.cl PROPERTIES FOLDER "libclc/Sourcegenning" )
 
 if ( clspv-- IN_LIST LIBCLC_TARGETS_TO_BUILD OR clspv64-- IN_LIST LIBCLC_TARGETS_TO_BUILD )
   add_custom_command(
     OUTPUT clspv-convert.cl
     COMMAND ${Python3_EXECUTABLE} ${script_loc} --clspv > clspv-convert.cl
     DEPENDS ${script_loc} )
-  add_custom_target( "clspv-generate_convert.cl" DEPENDS clspv-convert.cl )
-  set_target_properties( "clspv-generate_convert.cl" PROPERTIES FOLDER "libclc/Sourcegenning" )
+  add_custom_target( generate-clspv-convert.cl DEPENDS clspv-convert.cl )
+  set_target_properties( generate-clspv-convert.cl PROPERTIES FOLDER "libclc/Sourcegenning" )
 
   add_custom_command(
     OUTPUT clc-clspv-convert.cl
     COMMAND ${Python3_EXECUTABLE} ${script_loc} --clc --clspv > clc-clspv-convert.cl
     DEPENDS ${script_loc} )
-  add_custom_target( "clc-clspv-generate_convert.cl" DEPENDS clc-clspv-convert.cl )
-  set_target_properties( "clc-clspv-generate_convert.cl" PROPERTIES FOLDER "libclc/Sourcegenning" )
+  add_custom_target( generate-clc-clspv-convert.cl DEPENDS clc-clspv-convert.cl )
+  set_target_properties( generate-clc-clspv-convert.cl PROPERTIES FOLDER "libclc/Sourcegenning" )
 endif()
 
 enable_testing()
@@ -324,9 +324,11 @@ foreach( t ${LIBCLC_TARGETS_TO_BUILD} )
   if( NOT ARCH STREQUAL spirv AND NOT ARCH STREQUAL spirv64 )
     if( ARCH STREQUAL clspv OR ARCH STREQUAL clspv64 )
       list( APPEND opencl_gen_files clspv-convert.cl )
-    elseif ( NOT ENABLE_RUNTIME_SUBNORMAL )
+    else()
       list( APPEND opencl_gen_files convert.cl )
-      list( APPEND opencl_lib_files generic/lib/subnormal_use_default.ll )
+      if ( NOT ENABLE_RUNTIME_SUBNORMAL )
+        list( APPEND opencl_lib_files generic/lib/subnormal_use_default.ll )
+      endif()
     endif()
   endif()
 
diff --git a/libclc/cmake/modules/AddLibclc.cmake b/libclc/cmake/modules/AddLibclc.cmake
index a3b311f12a1e3..5347b0822477b 100644
--- a/libclc/cmake/modules/AddLibclc.cmake
+++ b/libclc/cmake/modules/AddLibclc.cmake
@@ -230,11 +230,17 @@ function(add_libclc_builtin_set)
     # We need to take each file and produce an absolute input file, as well
     # as a unique architecture-specific output file. We deal with a mix of
     # different input files, which makes this trickier.
+    set( input_file_dep )
     if( ${file} IN_LIST ARG_GEN_FILES )
       # Generated files are given just as file names, which we must make
       # absolute to the binary directory.
       set( input_file ${CMAKE_CURRENT_BINARY_DIR}/${file} )
       set( output_file "${LIBCLC_ARCH_OBJFILE_DIR}/${file}.bc" )
+      # If a target exists that generates this file, add that as a dependency
+      # of the custom command.
+      if( TARGET generate-${file} )
+        set( input_file_dep generate-${file} )
+      endif()
     else()
       # Other files are originally relative to each SOURCE file, which are
       # then make relative to the libclc root directory. We must normalize
@@ -249,19 +255,13 @@ function(add_libclc_builtin_set)
 
     get_filename_component( file_dir ${file} DIRECTORY )
 
-    if( ARG_ARCH STREQUAL spirv OR ARG_ARCH STREQUAL spirv64 )
-      set(CONVERT_DEP clspv-generate_convert.cl)
-    else()
-      set(CONVERT_DEP generate_convert.cl)
-    endif()
-
     compile_to_bc(
       TRIPLE ${ARG_TRIPLE}
       INPUT ${input_file}
       OUTPUT ${output_file}
       EXTRA_OPTS -fno-builtin -nostdlib
         "${ARG_COMPILE_FLAGS}" -I${CMAKE_CURRENT_SOURCE_DIR}/${file_dir}
-      DEPENDENCIES ${CONVERT_DEP}
+      DEPENDENCIES ${input_file_dep}
     )
     list( APPEND bytecode_files ${output_file} )
   endforeach()