[clang] [NFC][HLSL] Allow target intrinsic switching to optionally be set. (PR #117648)

Chris B via cfe-commits cfe-commits at lists.llvm.org
Mon Dec 2 12:00:15 PST 2024


================
@@ -30,22 +30,36 @@
 #include <optional>
 #include <vector>
 
+#define GENERATE_HLSL_INTRINSIC_FUNCTION_DEFAULT(FunctionName,                 \
+                                                 IntrinsicPostfix)             \
+  GENERATE_HLSL_INTRINSIC_FUNCTION(FunctionName, IntrinsicPostfix, 1, 1)
+
 // A function generator macro for picking the right intrinsic
 // for the target backend
-#define GENERATE_HLSL_INTRINSIC_FUNCTION(FunctionName, IntrinsicPostfix)       \
+#define GENERATE_HLSL_INTRINSIC_FUNCTION(FunctionName, IntrinsicPostfix,       \
+                                         IncludeDXIL, IncludeSPIRV)            \
   llvm::Intrinsic::ID get##FunctionName##Intrinsic() {                         \
     llvm::Triple::ArchType Arch = getArch();                                   \
     switch (Arch) {                                                            \
-    case llvm::Triple::dxil:                                                   \
-      return llvm::Intrinsic::dx_##IntrinsicPostfix;                           \
-    case llvm::Triple::spirv:                                                  \
-      return llvm::Intrinsic::spv_##IntrinsicPostfix;                          \
+      /* Include DXIL case only if IncludeDXIL is true */                      \
+      IF_INCLUDE(IncludeDXIL, case llvm::Triple::dxil                          \
+                 : return llvm::Intrinsic::dx_##IntrinsicPostfix;)             \
+      /* Include SPIRV case only if IncludeSPIRV is true */                    \
+      IF_INCLUDE(IncludeSPIRV, case llvm::Triple::spirv                        \
+                 : return llvm::Intrinsic::spv_##IntrinsicPostfix;)            \
+                                                                               \
     default:                                                                   \
       llvm_unreachable("Intrinsic " #IntrinsicPostfix                          \
                        " not supported by target architecture");               \
     }                                                                          \
   }
 
+#define IF_INCLUDE(Condition, Code) IF_INCLUDE_IMPL(Condition, Code)
+#define IF_INCLUDE_IMPL(Condition, Code) IF_INCLUDE_##Condition(Code)
+
+#define IF_INCLUDE_1(Code) Code
+#define IF_INCLUDE_0(Code)
----------------
llvm-beanz wrote:

I feel like this is a bit of extra preprocessor complexity. Instead of having `GENERATE_HLSL_INTRINSIC_FUNCTION` change to take arguments, we could just define target-specific macros (i.e. `GENERATE_HLSL_DX_INTRINSIC_FUNCTION` and `GENERATE_HLSL_SPV_INTRINSIC_FUNCTION`).

I also generally think putting `DEFAULT` in the name of something is a bit of a red flag since you're making the name longer, but not adding relevant context.

https://github.com/llvm/llvm-project/pull/117648


More information about the cfe-commits mailing list