addressing pr comments

Author: joaosaffran-zz

clang/lib/CodeGen/CGBuiltin.cpp

@@ -104,6 +104,7 @@ static Value *handleHlslClip(const CallExpr *E, CodeGenFunction *CGF) {
 
   Constant *FZeroConst = ConstantFP::getZero(CGF->FloatTy);
   Value *CMP;
+  Value *LastInstr;
 
   if (const auto *VecTy = E->getArg(0)->getType()->getAs<clang::VectorType>()) {
     FZeroConst = ConstantVector::getSplat(
@@ -116,23 +117,27 @@ static Value *handleHlslClip(const CallExpr *E, CodeGenFunction *CGF) {
     CMP = CGF->Builder.CreateFCmpOLT(Op0, FZeroConst);
 
   if (CGF->CGM.getTarget().getTriple().isDXIL())
-    return CGF->Builder.CreateIntrinsic(CGF->VoidTy, llvm::Intrinsic::dx_clip,
-                                        {CMP}, nullptr);
+    LastInstr = CGF->Builder.CreateIntrinsic(
+        CGF->VoidTy, llvm::Intrinsic::dx_clip, {CMP}, nullptr);
+  else if (CGF->CGM.getTarget().getTriple().isSPIRV()) {
+    BasicBlock *LT0 = CGF->createBasicBlock("lt0", CGF->CurFn);
+    BasicBlock *End = CGF->createBasicBlock("end", CGF->CurFn);
 
-  BasicBlock *LT0 = CGF->createBasicBlock("lt0", CGF->CurFn);
-  BasicBlock *End = CGF->createBasicBlock("end", CGF->CurFn);
+    CGF->Builder.CreateCondBr(CMP, LT0, End);
 
-  CGF->Builder.CreateCondBr(CMP, LT0, End);
+    CGF->Builder.SetInsertPoint(LT0);
 
-  CGF->Builder.SetInsertPoint(LT0);
+    CGF->Builder.CreateIntrinsic(CGF->VoidTy, llvm::Intrinsic::spv_clip, {},
+                                 nullptr);
 
-  CGF->Builder.CreateIntrinsic(CGF->VoidTy, llvm::Intrinsic::spv_clip, {},
-                               nullptr);
+    LastInstr = CGF->Builder.CreateBr(End);
 
-  auto *BrCall = CGF->Builder.CreateBr(End);
+    CGF->Builder.SetInsertPoint(End);
+  } else {
+    llvm_unreachable("Backend Codegen not supported.");
+  }
 
-  CGF->Builder.SetInsertPoint(End);
-  return BrCall;
+  return LastInstr;
 }
 
 static Value *handleHlslSplitdouble(const CallExpr *E, CodeGenFunction *CGF) {

clang/lib/CodeGen/CGHLSLRuntime.h

@@ -94,7 +94,6 @@ class CGHLSLRuntime {
   GENERATE_HLSL_INTRINSIC_FUNCTION(WaveActiveCountBits, wave_active_countbits)
   GENERATE_HLSL_INTRINSIC_FUNCTION(WaveIsFirstLane, wave_is_first_lane)
   GENERATE_HLSL_INTRINSIC_FUNCTION(WaveReadLaneAt, wave_readlane)
-  GENERATE_HLSL_INTRINSIC_FUNCTION(Clip, clip)  
   GENERATE_HLSL_INTRINSIC_FUNCTION(FirstBitUHigh, firstbituhigh)
   GENERATE_HLSL_INTRINSIC_FUNCTION(FirstBitSHigh, firstbitshigh)
   GENERATE_HLSL_INTRINSIC_FUNCTION(NClamp, nclamp)

llvm/docs/SPIRVUsage.rst

@@ -149,6 +149,8 @@ list of supported SPIR-V extensions, sorted alphabetically by their extension na
      - Adds atomic min and max instruction on floating-point numbers.
    * - ``SPV_EXT_arithmetic_fence``
      - Adds an instruction that prevents fast-math optimizations between its argument and the expression that contains it.
+   * - ``SPV_EXT_demote_to_helper_invocation``
+     - Adds an instruction that demotes a fragment shader invocation to a helper invocation.
    * - ``SPV_INTEL_arbitrary_precision_integers``
      - Allows generating arbitrary width integer types.
    * - ``SPV_INTEL_bfloat16_conversion``

llvm/include/llvm/IR/IntrinsicsSPIRV.td

@@ -91,7 +91,7 @@ let TargetPrefix = "spv" in {
   def int_spv_sign : DefaultAttrsIntrinsic<[LLVMScalarOrSameVectorWidth<0, llvm_i32_ty>], [llvm_any_ty], [IntrNoMem]>;
   def int_spv_radians : DefaultAttrsIntrinsic<[LLVMMatchType<0>], [llvm_anyfloat_ty], [IntrNoMem]>;
   def int_spv_group_memory_barrier_with_group_sync : DefaultAttrsIntrinsic<[], [], []>;
-  def int_spv_clip : DefaultAttrsIntrinsic<[], [llvm_i1_ty], [IntrNoMem]>;
+  def int_spv_clip : DefaultAttrsIntrinsic<[], [], []>;
   def int_spv_uclamp : DefaultAttrsIntrinsic<[llvm_anyint_ty], [LLVMMatchType<0>, LLVMMatchType<0>, LLVMMatchType<0>], [IntrNoMem]>;
   def int_spv_sclamp : DefaultAttrsIntrinsic<[llvm_anyint_ty], [LLVMMatchType<0>, LLVMMatchType<0>, LLVMMatchType<0>], [IntrNoMem]>;
   def int_spv_nclamp : DefaultAttrsIntrinsic<[llvm_anyfloat_ty], [LLVMMatchType<0>, LLVMMatchType<0>, LLVMMatchType<0>], [IntrNoMem]>;

llvm/lib/Target/DirectX/DXIL.td

@@ -871,15 +871,6 @@ def WaveIsFirstLane :  DXILOp<110, waveIsFirstLane> {
   let attributes = [Attributes<DXIL1_0, [ReadNone]>];
 }
 
-def WaveGetLaneIndex : DXILOp<111, waveGetLaneIndex> {
-  let Doc = "returns the index of the current lane in the wave";
-  let LLVMIntrinsic = int_dx_wave_getlaneindex;
-  let arguments = [];
-  let result = Int32Ty;
-  let stages = [Stages<DXIL1_0, [all_stages]>];
-  let attributes = [Attributes<DXIL1_0, [ReadNone]>];
-}
-
 def WaveReadLaneAt:  DXILOp<117, waveReadLaneAt> {
   let Doc = "returns the value from the specified lane";
   let LLVMIntrinsic = int_dx_wave_readlane;
@@ -890,24 +881,10 @@ def WaveReadLaneAt:  DXILOp<117, waveReadLaneAt> {
   let attributes = [Attributes<DXIL1_0, [ReadNone]>];
 }
 
-def AnnotateHandle : DXILOp<217, annotateHandle> {
-  let Doc = "annotate handle with resource properties";
-  let arguments = [HandleTy, ResPropsTy];
-  let result = HandleTy;
-  let stages = [Stages<DXIL1_6, [all_stages]>];
-}
-
-def CreateHandleFromBinding : DXILOp<218, createHandleFromBinding> {
-  let Doc = "create resource handle from binding";
-  let arguments = [ResBindTy, Int32Ty, Int1Ty];
-  let result = HandleTy;
-  let stages = [Stages<DXIL1_6, [all_stages]>];
-}
-
-def WaveAllBitCount : DXILOp<135, waveAllOp> {
-  let Doc = "returns the count of bits set to 1 across the wave";
-  let LLVMIntrinsic = int_dx_wave_active_countbits;
-  let arguments = [Int1Ty];
+def WaveGetLaneIndex : DXILOp<111, waveGetLaneIndex> {
+  let Doc = "returns the index of the current lane in the wave";
+  let LLVMIntrinsic = int_dx_wave_getlaneindex;
+  let arguments = [];
   let result = Int32Ty;
   let stages = [Stages<DXIL1_0, [all_stages]>];
   let attributes = [Attributes<DXIL1_0, [ReadNone]>];

llvm/lib/Target/SPIRV/SPIRVCommandLine.cpp

@@ -30,6 +30,8 @@ static const std::map<std::string, SPIRV::Extension::Extension>
          SPIRV::Extension::Extension::SPV_EXT_shader_atomic_float_min_max},
         {"SPV_EXT_arithmetic_fence",
          SPIRV::Extension::Extension::SPV_EXT_arithmetic_fence},
+        {"SPV_EXT_demote_to_helper_invocation",
+         SPIRV::Extension::Extension::SPV_EXT_demote_to_helper_invocation},
         {"SPV_INTEL_arbitrary_precision_integers",
          SPIRV::Extension::Extension::SPV_INTEL_arbitrary_precision_integers},
         {"SPV_INTEL_cache_controls",
@@ -75,9 +77,7 @@ static const std::map<std::string, SPIRV::Extension::Extension>
         {"SPV_KHR_cooperative_matrix",
          SPIRV::Extension::Extension::SPV_KHR_cooperative_matrix},
         {"SPV_KHR_non_semantic_info",
-         SPIRV::Extension::Extension::SPV_KHR_non_semantic_info},
-        {"SPV_EXT_demote_to_helper_invocation",
-         SPIRV::Extension::Extension::SPV_EXT_demote_to_helper_invocation}};
+         SPIRV::Extension::Extension::SPV_KHR_non_semantic_info}};
 
 bool SPIRVExtensionsParser::parse(cl::Option &O, llvm::StringRef ArgName,
                                   llvm::StringRef ArgValue,

llvm/lib/Target/SPIRV/SPIRVInstructionSelector.cpp

@@ -2163,13 +2163,9 @@ bool SPIRVInstructionSelector::selectClip(Register ResVReg,
 
   unsigned Opcode;
 
-  if (STI.isAtLeastSPIRVVer(VersionTuple(1, 6))) {
-    if (!STI.canUseExtension(
-            SPIRV::Extension::SPV_EXT_demote_to_helper_invocation))
-      report_fatal_error(
-          "llvm.spv.clip intrinsic: this instruction requires the following "
-          "SPIR-V extension: SPV_EXT_demote_to_helper_invocation",
-          false);
+  if (STI.canUseExtension(
+          SPIRV::Extension::SPV_EXT_demote_to_helper_invocation) ||
+      STI.isAtLeastSPIRVVer(llvm::VersionTuple(1, 6))) {
     Opcode = SPIRV::OpDemoteToHelperInvocation;
   } else {
     Opcode = SPIRV::OpKill;

llvm/lib/Target/SPIRV/SPIRVModuleAnalysis.cpp

@@ -633,7 +633,8 @@ void RequirementHandler::initAvailableCapabilities(const SPIRVSubtarget &ST) {
   if (ST.isAtLeastSPIRVVer(VersionTuple(1, 6)))
     addAvailableCaps({Capability::DotProduct, Capability::DotProductInputAll,
                       Capability::DotProductInput4x8Bit,
-                      Capability::DotProductInput4x8BitPacked});
+                      Capability::DotProductInput4x8BitPacked,
+                      Capability::DemoteToHelperInvocation});
 
   // Add capabilities enabled by extensions.
   for (auto Extension : ST.getAllAvailableExtensions()) {
@@ -1407,21 +1408,23 @@ void addInstrRequirements(const MachineInstr &MI,
       Reqs.addCapability(SPIRV::Capability::SplitBarrierINTEL);
     }
     break;
-  case SPIRV::OpSDot:
-  case SPIRV::OpUDot:
-    AddDotProductRequirements(MI, Reqs, ST);
-    break;
-
   case SPIRV::OpKill: {
     Reqs.addCapability(SPIRV::Capability::Shader);
   } break;
   case SPIRV::OpDemoteToHelperInvocation:
+    Reqs.addCapability(SPIRV::Capability::DemoteToHelperInvocation);
+
     if (ST.canUseExtension(
             SPIRV::Extension::SPV_EXT_demote_to_helper_invocation)) {
-      Reqs.addExtension(SPIRV::Extension::SPV_EXT_demote_to_helper_invocation);
-      Reqs.addCapability(SPIRV::Capability::DemoteToHelperInvocation);
+      if (!ST.isAtLeastSPIRVVer(llvm::VersionTuple(1, 6)))
+        Reqs.addExtension(
+            SPIRV::Extension::SPV_EXT_demote_to_helper_invocation);
     }
     break;
+  case SPIRV::OpSDot:
+  case SPIRV::OpUDot:
+    AddDotProductRequirements(MI, Reqs, ST);
+    break;
   default:
     break;
   }

llvm/lib/Target/SPIRV/SPIRVSymbolicOperands.td

@@ -456,7 +456,7 @@ defm VulkanMemoryModelDeviceScopeKHR : CapabilityOperand<5346, 0, 0, [], []>;
 defm ImageFootprintNV : CapabilityOperand<5282, 0, 0, [], []>;
 defm FragmentBarycentricNV : CapabilityOperand<5284, 0, 0, [], []>;
 defm ComputeDerivativeGroupQuadsNV : CapabilityOperand<5288, 0, 0, [], []>;
-defm DemoteToHelperInvocation : CapabilityOperand<5379, 0, 0, [SPV_EXT_demote_to_helper_invocation], []>;
+defm DemoteToHelperInvocation : CapabilityOperand<5379, 0x10600, 0, [SPV_EXT_demote_to_helper_invocation], []>;
 defm ComputeDerivativeGroupLinearNV : CapabilityOperand<5350, 0, 0, [], []>;
 defm FragmentDensityEXT : CapabilityOperand<5291, 0, 0, [], [Shader]>;
 defm PhysicalStorageBufferAddressesEXT : CapabilityOperand<5347, 0, 0, [], [Shader]>;

llvm/test/CodeGen/SPIRV/hlsl-intrinsics/clip.ll

@@ -1,26 +1,28 @@
-; RUN: llc -verify-machineinstrs -O0 -mtriple=spirv-unknown-unknown %s -o - | FileCheck %s --check-prefixes=CHECK,SPIRV15
-; RUN: llc -verify-machineinstrs -spirv-ext=+SPV_EXT_demote_to_helper_invocation -O0 -mtriple=spirv32v1.6-unknown-unknown %s -o - | FileCheck %s --check-prefixes=CHECK,SPIRV16
-; RUN: %if spirv-tools %{ llc -O0 -mtriple=spirv-unknown-unknown %s -o - -filetype=obj | spirv-val %}
+; RUN: llc -verify-machineinstrs -O0 -mtriple=spirv1.5-unknown-unknown %s -o - | FileCheck %s --check-prefixes=CHECK,SPIRV15
+; RUN: llc -verify-machineinstrs -spirv-ext=+SPV_EXT_demote_to_helper_invocation -O0 -mtriple=spirv1.5-unknown-unknown %s -o - | FileCheck %s --check-prefixes=CHECK,SPIRV16,WITH-EXTENSION,WITH-CAPABILITY
+; RUN: llc -verify-machineinstrs -O0 -mtriple=spirv1.6-unknown-unknown %s -o - | FileCheck %s --check-prefixes=CHECK,SPIRV16,WITH-CAPABILITY
+; RUN: %if spirv-tools %{ llc -O0 -mtriple=spirv1.5-unknown-unknown %s -o - -filetype=obj | spirv-val %}
+; RUN: %if spirv-tools %{ llc -O0 -spirv-ext=+SPV_EXT_demote_to_helper_invocation -mtriple=spirv1.5-unknown-unknown %s -o - -filetype=obj | spirv-val %}
+; RUN: %if spirv-tools %{ llc -O0 -mtriple=spirv1.6-unknown-unknown %s -o - -filetype=obj | spirv-val %}
 
 
 ; Make sure lowering is correctly generating spirv code.
 
+; WITH-CAPABILITY-DAG: OpCapability DemoteToHelperInvocation
+; WITH-EXTENSION-DAG: OpExtension "SPV_EXT_demote_to_helper_invocation"
+
 ; CHECK-DAG:   %[[#float:]] = OpTypeFloat 32
 ; CHECK-DAG:   %[[#void:]]  = OpTypeVoid
 ; CHECK-DAG:   %[[#bool:]]  = OpTypeBool
 ; CHECK-DAG:   %[[#v4bool:]]  = OpTypeVector %[[#bool]] 4
 ; CHECK-DAG:   %[[#v4float:]] = OpTypeVector %[[#float]] 4
 ; CHECK-DAG:   %[[#fzero:]] = OpConstant %[[#float]] 0
 ; CHECK-DAG:   %[[#v4fzero:]] = OpConstantNull %[[#v4float]]
-; SPIRV16-DAG: %[[#vecfuncopptr:]] = OpTypePointer Function %[[#v4float]]
-; SPIRV16-DAG: %[[#funcopptr:]] = OpTypePointer Function %[[#float]]
 
 define void @test_scalar(float noundef %Buf) {
 entry:
 ; CHECK-LABEL: ; -- Begin function test_scalar
-; SPIRV16:     %[[#param:]] = OpVariable %[[#funcopptr]] Function
-; SPIRV16:     %[[#load:]] = OpLoad %[[#float]] %[[#param]] Aligned 4
-; SPIRV15:     %[[#load:]] = OpFunctionParameter %[[#float]]
+; CHECK:       %[[#load:]] = OpFunctionParameter %[[#float]]
 ; CHECK:       %[[#cmplt:]] = OpFOrdLessThan %[[#bool]] %[[#load]] %[[#fzero]]
 ; CHECK:       OpBranchConditional %[[#cmplt]] %[[#truel:]] %[[#endl:]]
 ; CHECK:       %[[#truel]] = OpLabel
@@ -48,9 +50,7 @@ declare void @llvm.spv.clip()
 define void @test_vector(<4 x float> noundef %Buf) {
 entry:
 ; CHECK-LABEL: ; -- Begin function test_vector
-; SPIRV16:     %[[#param:]] = OpVariable %[[#vecfuncopptr]] Function
-; SPIRV16:     %[[#loadvec:]] = OpLoad %[[#v4float]] %[[#param]] Aligned 16
-; SPIRV15:     %[[#loadvec:]] = OpFunctionParameter %[[#v4float]]
+; CHECK:       %[[#loadvec:]] = OpFunctionParameter %[[#v4float]]
 ; CHECK:       %[[#cmplt:]] = OpFOrdLessThan %[[#v4bool]] %[[#loadvec]] %[[#v4fzero]]
 ; CHECK:       %[[#opany:]] = OpAny %[[#bool]] %[[#cmplt]]
 ; CHECK:       OpBranchConditional %[[#opany]]  %[[#truel:]] %[[#endl:]]