From 910cb2ca8cb48032cefd21addec8a5b6f3dc4e02 Mon Sep 17 00:00:00 2001 From: kmpeng Date: Tue, 29 Apr 2025 16:46:08 -0700 Subject: [PATCH 1/9] WIP - debug `matchSelectToFaceForward` --- .../lib/Headers/hlsl/hlsl_intrinsic_helpers.h | 9 +- llvm/lib/Target/SPIRV/SPIRVCombine.td | 9 +- llvm/lib/Target/SPIRV/SPIRVCombinerHelper.cpp | 99 +++++++++++++++++++ llvm/lib/Target/SPIRV/SPIRVCombinerHelper.h | 2 + ...egalizercombiner-select-to-faceforward.mir | 33 +++++++ .../SPIRV/hlsl-intrinsics/faceforward.ll | 44 ++------- 6 files changed, 154 insertions(+), 42 deletions(-) create mode 100644 llvm/test/CodeGen/SPIRV/GlobalISel/InstCombine/prelegalizercombiner-select-to-faceforward.mir diff --git a/clang/lib/Headers/hlsl/hlsl_intrinsic_helpers.h b/clang/lib/Headers/hlsl/hlsl_intrinsic_helpers.h index c877234479ad1..b9dd7d3930d4f 100644 --- a/clang/lib/Headers/hlsl/hlsl_intrinsic_helpers.h +++ b/clang/lib/Headers/hlsl/hlsl_intrinsic_helpers.h @@ -137,11 +137,12 @@ template constexpr vector lit_impl(T NDotL, T NDotH, T M) { } template constexpr T faceforward_impl(T N, T I, T Ng) { -#if (__has_builtin(__builtin_spirv_faceforward)) - return __builtin_spirv_faceforward(N, I, Ng); -#else +// #if (__has_builtin(__builtin_spirv_faceforward)) +// return __builtin_spirv_faceforward(N, I, Ng); +// #else +// return select(dot(I, Ng) < 0, N, -N); +// #endif return select(dot(I, Ng) < 0, N, -N); -#endif } template constexpr T ldexp_impl(T X, T Exp) { diff --git a/llvm/lib/Target/SPIRV/SPIRVCombine.td b/llvm/lib/Target/SPIRV/SPIRVCombine.td index fde56c4d3632a..991a5de1c4e83 100644 --- a/llvm/lib/Target/SPIRV/SPIRVCombine.td +++ b/llvm/lib/Target/SPIRV/SPIRVCombine.td @@ -15,8 +15,15 @@ def vector_length_sub_to_distance_lowering : GICombineRule < (apply [{ Helper.applySPIRVDistance(*${root}); }]) >; +def vector_select_to_faceforward_lowering : GICombineRule < + (defs root:$root), + (match (wip_match_opcode G_SELECT):$root, + [{ return Helper.matchSelectToFaceForward(*${root}); }]), + (apply [{ Helper.applySPIRVFaceForward(*${root}); }]) +>; + def SPIRVPreLegalizerCombiner : GICombiner<"SPIRVPreLegalizerCombinerImpl", - [vector_length_sub_to_distance_lowering]> { + [vector_length_sub_to_distance_lowering, vector_select_to_faceforward_lowering]> { let CombineAllMethodName = "tryCombineAllImpl"; } diff --git a/llvm/lib/Target/SPIRV/SPIRVCombinerHelper.cpp b/llvm/lib/Target/SPIRV/SPIRVCombinerHelper.cpp index 267794c1e0bc5..8dcf2b42c3555 100644 --- a/llvm/lib/Target/SPIRV/SPIRVCombinerHelper.cpp +++ b/llvm/lib/Target/SPIRV/SPIRVCombinerHelper.cpp @@ -58,3 +58,102 @@ void SPIRVCombinerHelper::applySPIRVDistance(MachineInstr &MI) const { MI.eraseFromParent(); } + +/// This match is part of a combine that +/// rewrites select(fcmp(dot(I, Ng), 0), N, 0 - N) to faceforward(N, I, Ng) +/// (vXf32 (g_select +/// (g_fcmp +/// (g_intrinsic dot(vXf32 I) (vXf32 Ng) +/// 0) +/// (vXf32 N) +/// (vXf32 g_fsub (0) (vXf32 N)))) +/// -> +/// (vXf32 (g_intrinsic faceforward +/// (vXf32 N) (vXf32 I) (vXf32 Ng))) +/// +bool SPIRVCombinerHelper::matchSelectToFaceForward(MachineInstr &MI) const { + if (MI.getOpcode() != TargetOpcode::G_SELECT) + return false; + + // Check if the condition is a comparison between a dot product and zero + Register CondReg = MI.getOperand(1).getReg(); + MachineInstr *CondInstr = MRI.getVRegDef(CondReg); + if (!CondInstr || CondInstr->getOpcode() != TargetOpcode::G_FCMP) + return false; + + if (!CondInstr->getOperand(2).getReg()) + return false; + Register DotReg = CondInstr->getOperand(2).getReg(); + MachineInstr *DotInstr = MRI.getVRegDef(DotReg); + // fmul? + if (DotInstr->getOpcode() != TargetOpcode::G_INTRINSIC || cast(DotInstr)->getIntrinsicID() != Intrinsic::spv_fdot) + return false; + // // is using getImm() correct? + // if (!CondInstr->getOperand(3).isImm() || + // CondInstr->getOperand(3).getImm() != 0) + // return false; + + // // Check if the false operand is the negation of the true operand + // Register TrueReg = MI.getOperand(2).getReg(); + // Register FalseReg = MI.getOperand(3).getReg(); + // MachineInstr *FalseInstr = MRI.getVRegDef(FalseReg); + // // getImm() correct? + // if (FalseInstr->getOpcode() != TargetOpcode::G_FSUB || + // FalseInstr->getOperand(1).getImm() != 0 || + // FalseInstr->getOperand(2).getReg() != TrueReg) + // return false; + + return true; +} + +void SPIRVCombinerHelper::applySPIRVFaceForward(MachineInstr &MI) const { + // Extract the operands for N, I, and Ng from the match criteria. + Register CondReg = MI.getOperand(1).getReg(); + MachineInstr *CondInstr = MRI.getVRegDef(CondReg); + Register DotReg = CondInstr->getOperand(1).getReg(); + MachineInstr *DotInstr = MRI.getVRegDef(DotReg); + Register DotOperand1 = DotInstr->getOperand(2).getReg(); // I + Register DotOperand2 = DotInstr->getOperand(3).getReg(); // Ng + Register TrueReg = MI.getOperand(2).getReg(); // N + Register FalseReg = MI.getOperand(3).getReg(); + MachineInstr *FalseInstr = MRI.getVRegDef(FalseReg); + + // Remove the original `select` instruction + Register ResultReg = MI.getOperand(0).getReg(); + DebugLoc DL = MI.getDebugLoc(); + MachineBasicBlock &MBB = *MI.getParent(); + MachineBasicBlock::iterator InsertPt = MI.getIterator(); + + // Build the `spv_faceforward` intrinsic + MachineInstrBuilder NewInstr = + BuildMI(MBB, InsertPt, DL, Builder.getTII().get(TargetOpcode::G_INTRINSIC)); + NewInstr + .addDef(ResultReg) // Result register + .addIntrinsicID(Intrinsic::spv_faceforward) // Intrinsic ID + .addUse(TrueReg) // Operand N + .addUse(DotOperand1) // Operand I + .addUse(DotOperand2); // Operand Ng + + SPIRVGlobalRegistry *GR = + MI.getMF()->getSubtarget().getSPIRVGlobalRegistry(); + auto RemoveAllUses = [&](Register Reg) { + SmallVector UsesToErase; + for (auto &UseMI : MRI.use_instructions(Reg)) + UsesToErase.push_back(&UseMI); + + // calling eraseFromParent to early invalidates the iterator. + for (auto *MIToErase : UsesToErase) { + GR->invalidateMachineInstr(MIToErase); + MIToErase->eraseFromParent(); + } + }; + RemoveAllUses(CondReg); // remove all uses of FCMP Result + GR->invalidateMachineInstr(CondInstr); + CondInstr->eraseFromParent(); // remove FCMP instruction + RemoveAllUses(DotReg); // remove all uses of spv_fdot Result + GR->invalidateMachineInstr(DotInstr); + DotInstr->eraseFromParent(); // remove spv_fdot instruction + RemoveAllUses(FalseReg); // remove all uses of FSUB Result + GR->invalidateMachineInstr(FalseInstr); + FalseInstr->eraseFromParent(); // remove FSUB instruction +} diff --git a/llvm/lib/Target/SPIRV/SPIRVCombinerHelper.h b/llvm/lib/Target/SPIRV/SPIRVCombinerHelper.h index 0b39d3408aab6..3118cdc744b8f 100644 --- a/llvm/lib/Target/SPIRV/SPIRVCombinerHelper.h +++ b/llvm/lib/Target/SPIRV/SPIRVCombinerHelper.h @@ -31,6 +31,8 @@ class SPIRVCombinerHelper : public CombinerHelper { bool matchLengthToDistance(MachineInstr &MI) const; void applySPIRVDistance(MachineInstr &MI) const; + bool matchSelectToFaceForward(MachineInstr &MI) const; + void applySPIRVFaceForward(MachineInstr &MI) const; }; } // end namespace llvm diff --git a/llvm/test/CodeGen/SPIRV/GlobalISel/InstCombine/prelegalizercombiner-select-to-faceforward.mir b/llvm/test/CodeGen/SPIRV/GlobalISel/InstCombine/prelegalizercombiner-select-to-faceforward.mir new file mode 100644 index 0000000000000..e7f1fde0becee --- /dev/null +++ b/llvm/test/CodeGen/SPIRV/GlobalISel/InstCombine/prelegalizercombiner-select-to-faceforward.mir @@ -0,0 +1,33 @@ +# RUN: llc -verify-machineinstrs -O0 -mtriple spirv-unknown-unknown -run-pass=spirv-prelegalizer-combiner %s -o - | FileCheck %s +# REQUIRES: asserts +--- +name: faceforward_instcombine_float4 +tracksRegLiveness: true +legalized: true +body: | + bb.1.entry: + ; CHECK-LABEL: name: faceforward_instcombine_float4 + ; CHECK-NOT: %6:_(<4 x s32>) = G_FSUB %2, %3 + ; CHECK-NOT: %7:id(s32) = G_INTRINSIC intrinsic(@llvm.spv.length), %6(<4 x s32>) + ; CHECK: %7:id(s32) = G_INTRINSIC intrinsic(@llvm.spv.distance), %2(<4 x s32>), %3(<4 x s32>) + %4:type(s64) = OpTypeVector %3:type(s64), 4 + %6:type(s64) = OpTypeFunction %4:type(s64), %4:type(s64), %4:type(s64), %4:type(s64) + %3:type(s64) = OpTypeFloat 32 + OpName %0:vfid(<4 x s32>), 97 + OpName %1:vfid(<4 x s32>), 98 + OpName %2:vfid(<4 x s32>), 99 + %5:iid(s64) = OpFunction %4:type(s64), 0, %6:type(s64) + %0:vfid(<4 x s32>) = OpFunctionParameter %4:type(s64) + %1:vfid(<4 x s32>) = OpFunctionParameter %4:type(s64) + %2:vfid(<4 x s32>) = OpFunctionParameter %4:type(s64) + OpName %5:iid(s64), 1701011814, 2003988326, 1600418401, 1953721961, 1651339107, 1600482921, 1634692198, 13428 + OpDecorate %5:iid(s64), 41, 1701011814, 2003988326, 1600418401, 1953721961, 1651339107, 1600482921, 1634692198, 13428, 0 + %10:_(s32) = G_FCONSTANT float 0.000000e+00 + %9:_(s32) = G_INTRINSIC intrinsic(@llvm.spv.fdot), %1:vfid(<4 x s32>), %2:vfid(<4 x s32>) + %11:_(s1) = G_FCMP floatpred(olt), %9:_(s32), %10:_ + %12:_(<4 x s32>) = G_INTRINSIC_W_SIDE_EFFECTS intrinsic(@llvm.spv.const.composite) + %13:_(<4 x s32>) = G_INTRINSIC_W_SIDE_EFFECTS intrinsic(@llvm.spv.track.constant), %12:_(<4 x s32>), <0x55555d14f908> + %14:_(<4 x s32>) = G_FSUB %13:_, %0:vfid + %15:id(<4 x s32>) = G_SELECT %11:_(s1), %0:vfid, %14:_ + OpReturnValue %15:id(<4 x s32>) + \ No newline at end of file diff --git a/llvm/test/CodeGen/SPIRV/hlsl-intrinsics/faceforward.ll b/llvm/test/CodeGen/SPIRV/hlsl-intrinsics/faceforward.ll index df11694703f82..5f84881cddf9c 100644 --- a/llvm/test/CodeGen/SPIRV/hlsl-intrinsics/faceforward.ll +++ b/llvm/test/CodeGen/SPIRV/hlsl-intrinsics/faceforward.ll @@ -11,49 +11,19 @@ ; CHECK-DAG: %[[#float_32:]] = OpTypeFloat 32 ; CHECK-DAG: %[[#vec4_float_32:]] = OpTypeVector %[[#float_32]] 4 -define noundef half @faceforward_half(half noundef %a, half noundef %b, half noundef %c) { -entry: - ; CHECK: %[[#]] = OpFunction %[[#float_16]] None %[[#]] - ; CHECK: %[[#arg0:]] = OpFunctionParameter %[[#float_16]] - ; CHECK: %[[#arg1:]] = OpFunctionParameter %[[#float_16]] - ; CHECK: %[[#arg2:]] = OpFunctionParameter %[[#float_16]] - ; CHECK: %[[#]] = OpExtInst %[[#float_16]] %[[#op_ext_glsl]] FaceForward %[[#arg0]] %[[#arg1]] %[[#arg2]] - %spv.faceforward = call half @llvm.spv.faceforward.f16(half %a, half %b, half %c) - ret half %spv.faceforward -} - -define noundef float @faceforward_float(float noundef %a, float noundef %b, float noundef %c) { -entry: - ; CHECK: %[[#]] = OpFunction %[[#float_32]] None %[[#]] - ; CHECK: %[[#arg0:]] = OpFunctionParameter %[[#float_32]] - ; CHECK: %[[#arg1:]] = OpFunctionParameter %[[#float_32]] - ; CHECK: %[[#arg2:]] = OpFunctionParameter %[[#float_32]] - ; CHECK: %[[#]] = OpExtInst %[[#float_32]] %[[#op_ext_glsl]] FaceForward %[[#arg0]] %[[#arg1]] %[[#arg2]] - %spv.faceforward = call float @llvm.spv.faceforward.f32(float %a, float %b, float %c) - ret float %spv.faceforward -} - -define noundef <4 x half> @faceforward_half4(<4 x half> noundef %a, <4 x half> noundef %b, <4 x half> noundef %c) { -entry: - ; CHECK: %[[#]] = OpFunction %[[#vec4_float_16]] None %[[#]] - ; CHECK: %[[#arg0:]] = OpFunctionParameter %[[#vec4_float_16]] - ; CHECK: %[[#arg1:]] = OpFunctionParameter %[[#vec4_float_16]] - ; CHECK: %[[#arg2:]] = OpFunctionParameter %[[#vec4_float_16]] - ; CHECK: %[[#]] = OpExtInst %[[#vec4_float_16]] %[[#op_ext_glsl]] FaceForward %[[#arg0]] %[[#arg1]] %[[#arg2]] - %spv.faceforward = call <4 x half> @llvm.spv.faceforward.v4f16(<4 x half> %a, <4 x half> %b, <4 x half> %c) - ret <4 x half> %spv.faceforward -} - -define noundef <4 x float> @faceforward_float4(<4 x float> noundef %a, <4 x float> noundef %b, <4 x float> noundef %c) { +define noundef <4 x float> @faceforward_instcombine_float4(<4 x float> noundef %a, <4 x float> noundef %b, <4 x float> noundef %c) { entry: ; CHECK: %[[#]] = OpFunction %[[#vec4_float_32]] None %[[#]] ; CHECK: %[[#arg0:]] = OpFunctionParameter %[[#vec4_float_32]] ; CHECK: %[[#arg1:]] = OpFunctionParameter %[[#vec4_float_32]] ; CHECK: %[[#arg2:]] = OpFunctionParameter %[[#vec4_float_32]] ; CHECK: %[[#]] = OpExtInst %[[#vec4_float_32]] %[[#op_ext_glsl]] FaceForward %[[#arg0]] %[[#arg1]] %[[#arg2]] - %spv.faceforward = call <4 x float> @llvm.spv.faceforward.v4f32(<4 x float> %a, <4 x float> %b, <4 x float> %c) - ret <4 x float> %spv.faceforward -} + %spv.fdot = call float @llvm.spv.fdot.v4f32(<4 x float> %b, <4 x float> %c) + %fcmp = fcmp olt float %spv.fdot, 0.000000e+00 + %delta = fsub <4 x float> zeroinitializer, %a + %select = select i1 %fcmp, <4 x float> %a, <4 x float> %delta + ret <4 x float> %select + } declare half @llvm.spv.faceforward.f16(half, half, half) declare float @llvm.spv.faceforward.f32(float, float, float) From 4a45d79e7143beeb201f44c1eab4b0f4165d94ad Mon Sep 17 00:00:00 2001 From: kmpeng Date: Tue, 13 May 2025 16:15:11 -0700 Subject: [PATCH 2/9] finished implementation, modified tests --- .../lib/Headers/hlsl/hlsl_intrinsic_helpers.h | 5 - .../CodeGenHLSL/builtins/faceforward.hlsl | 56 ++++++--- llvm/lib/Target/SPIRV/SPIRVCombinerHelper.cpp | 81 ++++++------- ...egalizercombiner-select-to-faceforward.mir | 17 +-- .../SPIRV/hlsl-intrinsics/faceforward.ll | 108 ++++++++++++------ .../CodeGen/SPIRV/opencl/faceforward-error.ll | 13 +++ 6 files changed, 175 insertions(+), 105 deletions(-) create mode 100644 llvm/test/CodeGen/SPIRV/opencl/faceforward-error.ll diff --git a/clang/lib/Headers/hlsl/hlsl_intrinsic_helpers.h b/clang/lib/Headers/hlsl/hlsl_intrinsic_helpers.h index b9dd7d3930d4f..4926e8092223a 100644 --- a/clang/lib/Headers/hlsl/hlsl_intrinsic_helpers.h +++ b/clang/lib/Headers/hlsl/hlsl_intrinsic_helpers.h @@ -137,11 +137,6 @@ template constexpr vector lit_impl(T NDotL, T NDotH, T M) { } template constexpr T faceforward_impl(T N, T I, T Ng) { -// #if (__has_builtin(__builtin_spirv_faceforward)) -// return __builtin_spirv_faceforward(N, I, Ng); -// #else -// return select(dot(I, Ng) < 0, N, -N); -// #endif return select(dot(I, Ng) < 0, N, -N); } diff --git a/clang/test/CodeGenHLSL/builtins/faceforward.hlsl b/clang/test/CodeGenHLSL/builtins/faceforward.hlsl index d2ece57aba4ae..f12f75f384964 100644 --- a/clang/test/CodeGenHLSL/builtins/faceforward.hlsl +++ b/clang/test/CodeGenHLSL/builtins/faceforward.hlsl @@ -12,8 +12,11 @@ // CHECK: %hlsl.select.i = select reassoc nnan ninf nsz arcp afn i1 %cmp.i, half %{{.*}}, half %fneg.i // CHECK: ret half %hlsl.select.i // SPVCHECK-LABEL: test_faceforward_half -// SPVCHECK: %spv.faceforward.i = call reassoc nnan ninf nsz arcp afn noundef half @llvm.spv.faceforward.f16(half %{{.*}}, half %{{.*}}, half %{{.*}}) -// SPVCHECK: ret half %spv.faceforward.i +// SPVCHECK: %hlsl.dot.i = fmul reassoc nnan ninf nsz arcp afn half %{{.*}}, %{{.*}} +// SPVCHECK: %cmp.i = fcmp reassoc nnan ninf nsz arcp afn olt half %hlsl.dot.i, 0xH0000 +// SPVCHECK: %fneg.i = fneg reassoc nnan ninf nsz arcp afn half %{{.*}} +// SPVCHECK: %hlsl.select.i = select reassoc nnan ninf nsz arcp afn i1 %cmp.i, half %{{.*}}, half %fneg.i +// SPVCHECK: ret half %hlsl.select.i half test_faceforward_half(half N, half I, half Ng) { return faceforward(N, I, Ng); } // CHECK-LABEL: test_faceforward_half2 @@ -23,8 +26,11 @@ half test_faceforward_half(half N, half I, half Ng) { return faceforward(N, I, N // CHECK: %hlsl.select.i = select reassoc nnan ninf nsz arcp afn i1 %cmp.i, <2 x half> %{{.*}}, <2 x half> %fneg.i // CHECK: ret <2 x half> %hlsl.select.i // SPVCHECK-LABEL: test_faceforward_half2 -// SPVCHECK: %spv.faceforward.i = call reassoc nnan ninf nsz arcp afn noundef <2 x half> @llvm.spv.faceforward.v2f16(<2 x half> %{{.*}}, <2 x half> %{{.*}}, <2 x half> %{{.*}}) -// SPVCHECK: ret <2 x half> %spv.faceforward.i +// SPVCHECK: %hlsl.dot.i = call reassoc nnan ninf nsz arcp afn half @llvm.spv.fdot.v2f16(<2 x half> %{{.*}}, <2 x half> %{{.*}}) +// SPVCHECK: %cmp.i = fcmp reassoc nnan ninf nsz arcp afn olt half %hlsl.dot.i, 0xH0000 +// SPVCHECK: %fneg.i = fneg reassoc nnan ninf nsz arcp afn <2 x half> %{{.*}} +// SPVCHECK: %hlsl.select.i = select reassoc nnan ninf nsz arcp afn i1 %cmp.i, <2 x half> %{{.*}}, <2 x half> %fneg.i +// SPVCHECK: ret <2 x half> %hlsl.select.i half2 test_faceforward_half2(half2 N, half2 I, half2 Ng) { return faceforward(N, I, Ng); } // CHECK-LABEL: test_faceforward_half3 @@ -34,8 +40,11 @@ half2 test_faceforward_half2(half2 N, half2 I, half2 Ng) { return faceforward(N, // CHECK: %hlsl.select.i = select reassoc nnan ninf nsz arcp afn i1 %cmp.i, <3 x half> %{{.*}}, <3 x half> %fneg.i // CHECK: ret <3 x half> %hlsl.select.i // SPVCHECK-LABEL: test_faceforward_half3 -// SPVCHECK: %spv.faceforward.i = call reassoc nnan ninf nsz arcp afn noundef <3 x half> @llvm.spv.faceforward.v3f16(<3 x half> %{{.*}}, <3 x half> %{{.*}}, <3 x half> %{{.*}}) -// SPVCHECK: ret <3 x half> %spv.faceforward.i +// SPVCHECK: %hlsl.dot.i = call reassoc nnan ninf nsz arcp afn half @llvm.spv.fdot.v3f16(<3 x half> %{{.*}}, <3 x half> %{{.*}}) +// SPVCHECK: %cmp.i = fcmp reassoc nnan ninf nsz arcp afn olt half %hlsl.dot.i, 0xH0000 +// SPVCHECK: %fneg.i = fneg reassoc nnan ninf nsz arcp afn <3 x half> %{{.*}} +// SPVCHECK: %hlsl.select.i = select reassoc nnan ninf nsz arcp afn i1 %cmp.i, <3 x half> %{{.*}}, <3 x half> %fneg.i +// SPVCHECK: ret <3 x half> %hlsl.select.i half3 test_faceforward_half3(half3 N, half3 I, half3 Ng) { return faceforward(N, I, Ng); } // CHECK-LABEL: test_faceforward_half4 @@ -45,8 +54,11 @@ half3 test_faceforward_half3(half3 N, half3 I, half3 Ng) { return faceforward(N, // CHECK: %hlsl.select.i = select reassoc nnan ninf nsz arcp afn i1 %cmp.i, <4 x half> %{{.*}}, <4 x half> %fneg.i // CHECK: ret <4 x half> %hlsl.select.i // SPVCHECK-LABEL: test_faceforward_half4 -// SPVCHECK: %spv.faceforward.i = call reassoc nnan ninf nsz arcp afn noundef <4 x half> @llvm.spv.faceforward.v4f16(<4 x half> %{{.*}}, <4 x half> %{{.*}}, <4 x half> %{{.*}}) -// SPVCHECK: ret <4 x half> %spv.faceforward.i +// SPVCHECK: %hlsl.dot.i = call reassoc nnan ninf nsz arcp afn half @llvm.spv.fdot.v4f16(<4 x half> %{{.*}}, <4 x half> %{{.*}}) +// SPVCHECK: %cmp.i = fcmp reassoc nnan ninf nsz arcp afn olt half %hlsl.dot.i, 0xH0000 +// SPVCHECK: %fneg.i = fneg reassoc nnan ninf nsz arcp afn <4 x half> %{{.*}} +// SPVCHECK: %hlsl.select.i = select reassoc nnan ninf nsz arcp afn i1 %cmp.i, <4 x half> %{{.*}}, <4 x half> %fneg.i +// SPVCHECK: ret <4 x half> %hlsl.select.i half4 test_faceforward_half4(half4 N, half4 I, half4 Ng) { return faceforward(N, I, Ng); } // CHECK-LABEL: test_faceforward_float @@ -56,8 +68,11 @@ half4 test_faceforward_half4(half4 N, half4 I, half4 Ng) { return faceforward(N, // CHECK: %hlsl.select.i = select reassoc nnan ninf nsz arcp afn i1 %cmp.i, float %{{.*}}, float %fneg.i // CHECK: ret float %hlsl.select.i // SPVCHECK-LABEL: test_faceforward_float -// SPVCHECK: %spv.faceforward.i = call reassoc nnan ninf nsz arcp afn noundef float @llvm.spv.faceforward.f32(float %{{.*}}, float %{{.*}}, float %{{.*}}) -// SPVCHECK: ret float %spv.faceforward.i +// SPVCHECK: %hlsl.dot.i = fmul reassoc nnan ninf nsz arcp afn float %{{.*}}, %{{.*}} +// SPVCHECK: %cmp.i = fcmp reassoc nnan ninf nsz arcp afn olt float %hlsl.dot.i, 0.000000e+00 +// SPVCHECK: %fneg.i = fneg reassoc nnan ninf nsz arcp afn float %{{.*}} +// SPVCHECK: %hlsl.select.i = select reassoc nnan ninf nsz arcp afn i1 %cmp.i, float %{{.*}}, float %fneg.i +// SPVCHECK: ret float %hlsl.select.i float test_faceforward_float(float N, float I, float Ng) { return faceforward(N, I, Ng); } // CHECK-LABEL: test_faceforward_float2 @@ -67,8 +82,11 @@ float test_faceforward_float(float N, float I, float Ng) { return faceforward(N, // CHECK: %hlsl.select.i = select reassoc nnan ninf nsz arcp afn i1 %cmp.i, <2 x float> %{{.*}}, <2 x float> %fneg.i // CHECK: ret <2 x float> %hlsl.select.i // SPVCHECK-LABEL: test_faceforward_float2 -// SPVCHECK: %spv.faceforward.i = call reassoc nnan ninf nsz arcp afn noundef <2 x float> @llvm.spv.faceforward.v2f32(<2 x float> %{{.*}}, <2 x float> %{{.*}}, <2 x float> %{{.*}}) -// SPVCHECK: ret <2 x float> %spv.faceforward.i +// SPVCHECK: %hlsl.dot.i = call reassoc nnan ninf nsz arcp afn float @llvm.spv.fdot.v2f32(<2 x float> %{{.*}}, <2 x float> %{{.*}}) +// SPVCHECK: %cmp.i = fcmp reassoc nnan ninf nsz arcp afn olt float %hlsl.dot.i, 0.000000e+00 +// SPVCHECK: %fneg.i = fneg reassoc nnan ninf nsz arcp afn <2 x float> %{{.*}} +// SPVCHECK: %hlsl.select.i = select reassoc nnan ninf nsz arcp afn i1 %cmp.i, <2 x float> %{{.*}}, <2 x float> %fneg.i +// SPVCHECK: ret <2 x float> %hlsl.select.i float2 test_faceforward_float2(float2 N, float2 I, float2 Ng) { return faceforward(N, I, Ng); } // CHECK-LABEL: test_faceforward_float3 @@ -78,8 +96,11 @@ float2 test_faceforward_float2(float2 N, float2 I, float2 Ng) { return faceforwa // CHECK: %hlsl.select.i = select reassoc nnan ninf nsz arcp afn i1 %cmp.i, <3 x float> %{{.*}}, <3 x float> %fneg.i // CHECK: ret <3 x float> %hlsl.select.i // SPVCHECK-LABEL: test_faceforward_float3 -// SPVCHECK: %spv.faceforward.i = call reassoc nnan ninf nsz arcp afn noundef <3 x float> @llvm.spv.faceforward.v3f32(<3 x float> %{{.*}}, <3 x float> %{{.*}}, <3 x float> %{{.*}}) -// SPVCHECK: ret <3 x float> %spv.faceforward.i +// SPVCHECK: %hlsl.dot.i = call reassoc nnan ninf nsz arcp afn float @llvm.spv.fdot.v3f32(<3 x float> %{{.*}}, <3 x float> %{{.*}}) +// SPVCHECK: %cmp.i = fcmp reassoc nnan ninf nsz arcp afn olt float %hlsl.dot.i, 0.000000e+00 +// SPVCHECK: %fneg.i = fneg reassoc nnan ninf nsz arcp afn <3 x float> %{{.*}} +// SPVCHECK: %hlsl.select.i = select reassoc nnan ninf nsz arcp afn i1 %cmp.i, <3 x float> %{{.*}}, <3 x float> %fneg.i +// SPVCHECK: ret <3 x float> %hlsl.select.i float3 test_faceforward_float3(float3 N, float3 I, float3 Ng) { return faceforward(N, I, Ng); } // CHECK-LABEL: test_faceforward_float4 @@ -89,6 +110,9 @@ float3 test_faceforward_float3(float3 N, float3 I, float3 Ng) { return faceforwa // CHECK: %hlsl.select.i = select reassoc nnan ninf nsz arcp afn i1 %cmp.i, <4 x float> %{{.*}}, <4 x float> %fneg.i // CHECK: ret <4 x float> %hlsl.select.i // SPVCHECK-LABEL: test_faceforward_float4 -// SPVCHECK: %spv.faceforward.i = call reassoc nnan ninf nsz arcp afn noundef <4 x float> @llvm.spv.faceforward.v4f32(<4 x float> %{{.*}}, <4 x float> %{{.*}}, <4 x float> %{{.*}}) -// SPVCHECK: ret <4 x float> %spv.faceforward.i +// SPVCHECK: %hlsl.dot.i = call reassoc nnan ninf nsz arcp afn float @llvm.spv.fdot.v4f32(<4 x float> %{{.*}}, <4 x float> %{{.*}}) +// SPVCHECK: %cmp.i = fcmp reassoc nnan ninf nsz arcp afn olt float %hlsl.dot.i, 0.000000e+00 +// SPVCHECK: %fneg.i = fneg reassoc nnan ninf nsz arcp afn <4 x float> %{{.*}} +// SPVCHECK: %hlsl.select.i = select reassoc nnan ninf nsz arcp afn i1 %cmp.i, <4 x float> %{{.*}}, <4 x float> %fneg.i +// SPVCHECK: ret <4 x float> %hlsl.select.i float4 test_faceforward_float4(float4 N, float4 I, float4 Ng) { return faceforward(N, I, Ng); } diff --git a/llvm/lib/Target/SPIRV/SPIRVCombinerHelper.cpp b/llvm/lib/Target/SPIRV/SPIRVCombinerHelper.cpp index 8dcf2b42c3555..aed477d9e8d45 100644 --- a/llvm/lib/Target/SPIRV/SPIRVCombinerHelper.cpp +++ b/llvm/lib/Target/SPIRV/SPIRVCombinerHelper.cpp @@ -21,6 +21,18 @@ SPIRVCombinerHelper::SPIRVCombinerHelper( const SPIRVSubtarget &STI) : CombinerHelper(Observer, B, IsPreLegalize, VT, MDT, LI), STI(STI) {} +static void removeAllUses(Register Reg, MachineRegisterInfo &MRI, + SPIRVGlobalRegistry *GR) { + SmallVector UsesToErase( + llvm::make_pointer_range(MRI.use_instructions(Reg))); + + // calling eraseFromParent too early invalidates the iterator. + for (auto *MIToErase : UsesToErase) { + GR->invalidateMachineInstr(MIToErase); + MIToErase->eraseFromParent(); + } +} + /// This match is part of a combine that /// rewrites length(X - Y) to distance(X, Y) /// (f32 (g_intrinsic length @@ -74,34 +86,34 @@ void SPIRVCombinerHelper::applySPIRVDistance(MachineInstr &MI) const { bool SPIRVCombinerHelper::matchSelectToFaceForward(MachineInstr &MI) const { if (MI.getOpcode() != TargetOpcode::G_SELECT) return false; - + // Check if the condition is a comparison between a dot product and zero Register CondReg = MI.getOperand(1).getReg(); MachineInstr *CondInstr = MRI.getVRegDef(CondReg); if (!CondInstr || CondInstr->getOpcode() != TargetOpcode::G_FCMP) return false; - if (!CondInstr->getOperand(2).getReg()) - return false; Register DotReg = CondInstr->getOperand(2).getReg(); MachineInstr *DotInstr = MRI.getVRegDef(DotReg); // fmul? - if (DotInstr->getOpcode() != TargetOpcode::G_INTRINSIC || cast(DotInstr)->getIntrinsicID() != Intrinsic::spv_fdot) + if (DotInstr->getOpcode() != TargetOpcode::G_INTRINSIC || + cast(DotInstr)->getIntrinsicID() != Intrinsic::spv_fdot) + return false; + + Register CondZeroReg = CondInstr->getOperand(3).getReg(); + MachineInstr *CondZeroInstr = MRI.getVRegDef(CondZeroReg); + if (CondZeroInstr->getOpcode() != TargetOpcode::G_FCONSTANT || + !CondZeroInstr->getOperand(1).getFPImm()->isZero()) + return false; + + // Check if the false operand is the negation of the true operand + Register TrueReg = MI.getOperand(2).getReg(); + Register FalseReg = MI.getOperand(3).getReg(); + MachineInstr *FalseInstr = MRI.getVRegDef(FalseReg); + if (FalseInstr->getOpcode() != TargetOpcode::G_FNEG) + return false; + if (TrueReg != FalseInstr->getOperand(1).getReg()) return false; - // // is using getImm() correct? - // if (!CondInstr->getOperand(3).isImm() || - // CondInstr->getOperand(3).getImm() != 0) - // return false; - - // // Check if the false operand is the negation of the true operand - // Register TrueReg = MI.getOperand(2).getReg(); - // Register FalseReg = MI.getOperand(3).getReg(); - // MachineInstr *FalseInstr = MRI.getVRegDef(FalseReg); - // // getImm() correct? - // if (FalseInstr->getOpcode() != TargetOpcode::G_FSUB || - // FalseInstr->getOperand(1).getImm() != 0 || - // FalseInstr->getOperand(2).getReg() != TrueReg) - // return false; return true; } @@ -110,13 +122,11 @@ void SPIRVCombinerHelper::applySPIRVFaceForward(MachineInstr &MI) const { // Extract the operands for N, I, and Ng from the match criteria. Register CondReg = MI.getOperand(1).getReg(); MachineInstr *CondInstr = MRI.getVRegDef(CondReg); - Register DotReg = CondInstr->getOperand(1).getReg(); + Register DotReg = CondInstr->getOperand(2).getReg(); MachineInstr *DotInstr = MRI.getVRegDef(DotReg); Register DotOperand1 = DotInstr->getOperand(2).getReg(); // I Register DotOperand2 = DotInstr->getOperand(3).getReg(); // Ng - Register TrueReg = MI.getOperand(2).getReg(); // N - Register FalseReg = MI.getOperand(3).getReg(); - MachineInstr *FalseInstr = MRI.getVRegDef(FalseReg); + Register TrueReg = MI.getOperand(2).getReg(); // N // Remove the original `select` instruction Register ResultReg = MI.getOperand(0).getReg(); @@ -128,32 +138,15 @@ void SPIRVCombinerHelper::applySPIRVFaceForward(MachineInstr &MI) const { MachineInstrBuilder NewInstr = BuildMI(MBB, InsertPt, DL, Builder.getTII().get(TargetOpcode::G_INTRINSIC)); NewInstr - .addDef(ResultReg) // Result register + .addDef(ResultReg) // Result register .addIntrinsicID(Intrinsic::spv_faceforward) // Intrinsic ID - .addUse(TrueReg) // Operand N - .addUse(DotOperand1) // Operand I - .addUse(DotOperand2); // Operand Ng + .addUse(TrueReg) // Operand N + .addUse(DotOperand1) // Operand I + .addUse(DotOperand2); // Operand Ng SPIRVGlobalRegistry *GR = MI.getMF()->getSubtarget().getSPIRVGlobalRegistry(); - auto RemoveAllUses = [&](Register Reg) { - SmallVector UsesToErase; - for (auto &UseMI : MRI.use_instructions(Reg)) - UsesToErase.push_back(&UseMI); - - // calling eraseFromParent to early invalidates the iterator. - for (auto *MIToErase : UsesToErase) { - GR->invalidateMachineInstr(MIToErase); - MIToErase->eraseFromParent(); - } - }; - RemoveAllUses(CondReg); // remove all uses of FCMP Result + removeAllUses(CondReg, MRI, GR); // remove all uses of FCMP Result GR->invalidateMachineInstr(CondInstr); CondInstr->eraseFromParent(); // remove FCMP instruction - RemoveAllUses(DotReg); // remove all uses of spv_fdot Result - GR->invalidateMachineInstr(DotInstr); - DotInstr->eraseFromParent(); // remove spv_fdot instruction - RemoveAllUses(FalseReg); // remove all uses of FSUB Result - GR->invalidateMachineInstr(FalseInstr); - FalseInstr->eraseFromParent(); // remove FSUB instruction } diff --git a/llvm/test/CodeGen/SPIRV/GlobalISel/InstCombine/prelegalizercombiner-select-to-faceforward.mir b/llvm/test/CodeGen/SPIRV/GlobalISel/InstCombine/prelegalizercombiner-select-to-faceforward.mir index e7f1fde0becee..4fbc3d0735b68 100644 --- a/llvm/test/CodeGen/SPIRV/GlobalISel/InstCombine/prelegalizercombiner-select-to-faceforward.mir +++ b/llvm/test/CodeGen/SPIRV/GlobalISel/InstCombine/prelegalizercombiner-select-to-faceforward.mir @@ -7,9 +7,12 @@ legalized: true body: | bb.1.entry: ; CHECK-LABEL: name: faceforward_instcombine_float4 - ; CHECK-NOT: %6:_(<4 x s32>) = G_FSUB %2, %3 - ; CHECK-NOT: %7:id(s32) = G_INTRINSIC intrinsic(@llvm.spv.length), %6(<4 x s32>) - ; CHECK: %7:id(s32) = G_INTRINSIC intrinsic(@llvm.spv.distance), %2(<4 x s32>), %3(<4 x s32>) + ; CHECK-NOT: %10:_(s32) = G_FCONSTANT float 0.000000e+00 + ; CHECK-NOT: %9:_(s32) = G_INTRINSIC intrinsic(@llvm.spv.fdot), %1:vfid(<4 x s32>), %2:vfid(<4 x s32>) + ; CHECK-NOT: %11:_(s1) = G_FCMP floatpred(olt), %9:_(s32), %10:_ + ; CHECK-NOT: %12:_(<4 x s32>) = G_FNEG %0:vfid + ; CHECK-NOT: %13:id(<4 x s32>) = G_SELECT %11:_(s1), %0:vfid, %12:_ + ; CHECK: %11:id(<4 x s32>) = G_INTRINSIC intrinsic(@llvm.spv.faceforward), %3(<4 x s32>), %4(<4 x s32>), %5(<4 x s32>) %4:type(s64) = OpTypeVector %3:type(s64), 4 %6:type(s64) = OpTypeFunction %4:type(s64), %4:type(s64), %4:type(s64), %4:type(s64) %3:type(s64) = OpTypeFloat 32 @@ -25,9 +28,7 @@ body: | %10:_(s32) = G_FCONSTANT float 0.000000e+00 %9:_(s32) = G_INTRINSIC intrinsic(@llvm.spv.fdot), %1:vfid(<4 x s32>), %2:vfid(<4 x s32>) %11:_(s1) = G_FCMP floatpred(olt), %9:_(s32), %10:_ - %12:_(<4 x s32>) = G_INTRINSIC_W_SIDE_EFFECTS intrinsic(@llvm.spv.const.composite) - %13:_(<4 x s32>) = G_INTRINSIC_W_SIDE_EFFECTS intrinsic(@llvm.spv.track.constant), %12:_(<4 x s32>), <0x55555d14f908> - %14:_(<4 x s32>) = G_FSUB %13:_, %0:vfid - %15:id(<4 x s32>) = G_SELECT %11:_(s1), %0:vfid, %14:_ - OpReturnValue %15:id(<4 x s32>) + %12:_(<4 x s32>) = G_FNEG %0:vfid + %13:id(<4 x s32>) = G_SELECT %11:_(s1), %0:vfid, %12:_ + OpReturnValue %13:id(<4 x s32>) \ No newline at end of file diff --git a/llvm/test/CodeGen/SPIRV/hlsl-intrinsics/faceforward.ll b/llvm/test/CodeGen/SPIRV/hlsl-intrinsics/faceforward.ll index 5f84881cddf9c..47c0d321594c6 100644 --- a/llvm/test/CodeGen/SPIRV/hlsl-intrinsics/faceforward.ll +++ b/llvm/test/CodeGen/SPIRV/hlsl-intrinsics/faceforward.ll @@ -1,32 +1,76 @@ -; RUN: llc -O0 -verify-machineinstrs -mtriple=spirv-unknown-vulkan %s -o - | FileCheck %s -; RUN: %if spirv-tools %{ llc -O0 -mtriple=spirv-unknown-vulkan %s -o - -filetype=obj | spirv-val --target-env spv1.4 %} - -; FIXME(#136344): Change --target-env to vulkan1.3 and update this test accordingly once the issue is resolved. - -; Make sure SPIRV operation function calls for faceforward are lowered correctly. - -; CHECK-DAG: %[[#op_ext_glsl:]] = OpExtInstImport "GLSL.std.450" -; CHECK-DAG: %[[#float_16:]] = OpTypeFloat 16 -; CHECK-DAG: %[[#vec4_float_16:]] = OpTypeVector %[[#float_16]] 4 -; CHECK-DAG: %[[#float_32:]] = OpTypeFloat 32 -; CHECK-DAG: %[[#vec4_float_32:]] = OpTypeVector %[[#float_32]] 4 - -define noundef <4 x float> @faceforward_instcombine_float4(<4 x float> noundef %a, <4 x float> noundef %b, <4 x float> noundef %c) { -entry: - ; CHECK: %[[#]] = OpFunction %[[#vec4_float_32]] None %[[#]] - ; CHECK: %[[#arg0:]] = OpFunctionParameter %[[#vec4_float_32]] - ; CHECK: %[[#arg1:]] = OpFunctionParameter %[[#vec4_float_32]] - ; CHECK: %[[#arg2:]] = OpFunctionParameter %[[#vec4_float_32]] - ; CHECK: %[[#]] = OpExtInst %[[#vec4_float_32]] %[[#op_ext_glsl]] FaceForward %[[#arg0]] %[[#arg1]] %[[#arg2]] - %spv.fdot = call float @llvm.spv.fdot.v4f32(<4 x float> %b, <4 x float> %c) - %fcmp = fcmp olt float %spv.fdot, 0.000000e+00 - %delta = fsub <4 x float> zeroinitializer, %a - %select = select i1 %fcmp, <4 x float> %a, <4 x float> %delta - ret <4 x float> %select - } - -declare half @llvm.spv.faceforward.f16(half, half, half) -declare float @llvm.spv.faceforward.f32(float, float, float) - -declare <4 x half> @llvm.spv.faceforward.v4f16(<4 x half>, <4 x half>, <4 x half>) -declare <4 x float> @llvm.spv.faceforward.v4f32(<4 x float>, <4 x float>, <4 x float>) +; RUN: llc -O0 -verify-machineinstrs -mtriple=spirv-unknown-vulkan %s -o - | FileCheck %s +; RUN: %if spirv-tools %{ llc -O0 -mtriple=spirv-unknown-vulkan %s -o - -filetype=obj | spirv-val --target-env spv1.4 %} + +; FIXME(#136344): Change --target-env to vulkan1.3 and update this test accordingly once the issue is resolved. + +; Make sure SPIRV operation function calls for faceforward are lowered correctly. + +; CHECK-DAG: %[[#op_ext_glsl:]] = OpExtInstImport "GLSL.std.450" +; CHECK-DAG: %[[#float_16:]] = OpTypeFloat 16 +; CHECK-DAG: %[[#vec4_float_16:]] = OpTypeVector %[[#float_16]] 4 +; CHECK-DAG: %[[#float_32:]] = OpTypeFloat 32 +; CHECK-DAG: %[[#vec4_float_32:]] = OpTypeVector %[[#float_32]] 4 + +define noundef half @faceforward_half(half noundef %a, half noundef %b, half noundef %c) { +entry: + ; CHECK: %[[#]] = OpFunction %[[#float_16]] None %[[#]] + ; CHECK: %[[#arg0:]] = OpFunctionParameter %[[#float_16]] + ; CHECK: %[[#arg1:]] = OpFunctionParameter %[[#float_16]] + ; CHECK: %[[#arg2:]] = OpFunctionParameter %[[#float_16]] + ; CHECK: %[[#]] = OpExtInst %[[#float_16]] %[[#op_ext_glsl]] FaceForward %[[#arg0]] %[[#arg1]] %[[#arg2]] + %spv.faceforward = call half @llvm.spv.faceforward.f16(half %a, half %b, half %c) + ret half %spv.faceforward +} + +define noundef float @faceforward_float(float noundef %a, float noundef %b, float noundef %c) { +entry: + ; CHECK: %[[#]] = OpFunction %[[#float_32]] None %[[#]] + ; CHECK: %[[#arg0:]] = OpFunctionParameter %[[#float_32]] + ; CHECK: %[[#arg1:]] = OpFunctionParameter %[[#float_32]] + ; CHECK: %[[#arg2:]] = OpFunctionParameter %[[#float_32]] + ; CHECK: %[[#]] = OpExtInst %[[#float_32]] %[[#op_ext_glsl]] FaceForward %[[#arg0]] %[[#arg1]] %[[#arg2]] + %spv.faceforward = call float @llvm.spv.faceforward.f32(float %a, float %b, float %c) + ret float %spv.faceforward +} + +define noundef <4 x half> @faceforward_half4(<4 x half> noundef %a, <4 x half> noundef %b, <4 x half> noundef %c) { +entry: + ; CHECK: %[[#]] = OpFunction %[[#vec4_float_16]] None %[[#]] + ; CHECK: %[[#arg0:]] = OpFunctionParameter %[[#vec4_float_16]] + ; CHECK: %[[#arg1:]] = OpFunctionParameter %[[#vec4_float_16]] + ; CHECK: %[[#arg2:]] = OpFunctionParameter %[[#vec4_float_16]] + ; CHECK: %[[#]] = OpExtInst %[[#vec4_float_16]] %[[#op_ext_glsl]] FaceForward %[[#arg0]] %[[#arg1]] %[[#arg2]] + %spv.faceforward = call <4 x half> @llvm.spv.faceforward.v4f16(<4 x half> %a, <4 x half> %b, <4 x half> %c) + ret <4 x half> %spv.faceforward +} + +define noundef <4 x float> @faceforward_float4(<4 x float> noundef %a, <4 x float> noundef %b, <4 x float> noundef %c) { +entry: + ; CHECK: %[[#]] = OpFunction %[[#vec4_float_32]] None %[[#]] + ; CHECK: %[[#arg0:]] = OpFunctionParameter %[[#vec4_float_32]] + ; CHECK: %[[#arg1:]] = OpFunctionParameter %[[#vec4_float_32]] + ; CHECK: %[[#arg2:]] = OpFunctionParameter %[[#vec4_float_32]] + ; CHECK: %[[#]] = OpExtInst %[[#vec4_float_32]] %[[#op_ext_glsl]] FaceForward %[[#arg0]] %[[#arg1]] %[[#arg2]] + %spv.faceforward = call <4 x float> @llvm.spv.faceforward.v4f32(<4 x float> %a, <4 x float> %b, <4 x float> %c) + ret <4 x float> %spv.faceforward +} + +define noundef <4 x float> @faceforward_instcombine_float4(<4 x float> noundef %a, <4 x float> noundef %b, <4 x float> noundef %c) { +entry: + ; CHECK: %[[#]] = OpFunction %[[#vec4_float_32]] None %[[#]] + ; CHECK: %[[#arg0:]] = OpFunctionParameter %[[#vec4_float_32]] + ; CHECK: %[[#arg1:]] = OpFunctionParameter %[[#vec4_float_32]] + ; CHECK: %[[#arg2:]] = OpFunctionParameter %[[#vec4_float_32]] + ; CHECK: %[[#]] = OpExtInst %[[#vec4_float_32]] %[[#op_ext_glsl]] FaceForward %[[#arg0]] %[[#arg1]] %[[#arg2]] + %spv.fdot = call float @llvm.spv.fdot.v4f32(<4 x float> %b, <4 x float> %c) + %fcmp = fcmp olt float %spv.fdot, 0.000000e+00 + %delta = fneg <4 x float> %a + %select = select i1 %fcmp, <4 x float> %a, <4 x float> %delta + ret <4 x float> %select + } + +declare half @llvm.spv.faceforward.f16(half, half, half) +declare float @llvm.spv.faceforward.f32(float, float, float) + +declare <4 x half> @llvm.spv.faceforward.v4f16(<4 x half>, <4 x half>, <4 x half>) +declare <4 x float> @llvm.spv.faceforward.v4f32(<4 x float>, <4 x float>, <4 x float>) diff --git a/llvm/test/CodeGen/SPIRV/opencl/faceforward-error.ll b/llvm/test/CodeGen/SPIRV/opencl/faceforward-error.ll new file mode 100644 index 0000000000000..9334e0977347a --- /dev/null +++ b/llvm/test/CodeGen/SPIRV/opencl/faceforward-error.ll @@ -0,0 +1,13 @@ +; RUN: not llc -verify-machineinstrs -O0 -mtriple=spirv64-unknown-unknown %s -o /dev/null 2>&1 | FileCheck %s +; RUN: not llc -verify-machineinstrs -O0 -mtriple=spirv32-unknown-unknown %s -o /dev/null 2>&1 | FileCheck %s + +; CHECK: LLVM ERROR: %{{.*}} = G_INTRINSIC intrinsic(@llvm.spv.faceforward), %{{.*}}, %{{.*}}, %{{.*}} is only supported with the GLSL extended instruction set. + +define noundef <4 x float> @faceforward_float4(<4 x float> noundef %a, <4 x float> noundef %b, <4 x float> noundef %c) { +entry: + %spv.faceforward = call <4 x float> @llvm.spv.faceforward.v4f32(<4 x float> %a, <4 x float> %b, <4 x float> %c) + ret <4 x float> %spv.faceforward +} + +declare <4 x float> @llvm.spv.faceforward.v4f32(<4 x float>, <4 x float>, <4 x float>) + From f284a0b38eda5486195060e9958757de8d3acaae Mon Sep 17 00:00:00 2001 From: kmpeng Date: Tue, 13 May 2025 17:18:53 -0700 Subject: [PATCH 3/9] fixed FIXME in faceforward.ll, edited comments --- llvm/lib/Target/SPIRV/SPIRVCombinerHelper.cpp | 15 +++++----- .../SPIRV/hlsl-intrinsics/faceforward.ll | 28 +++++++++++-------- 2 files changed, 24 insertions(+), 19 deletions(-) diff --git a/llvm/lib/Target/SPIRV/SPIRVCombinerHelper.cpp b/llvm/lib/Target/SPIRV/SPIRVCombinerHelper.cpp index aed477d9e8d45..1f4c3b05ea194 100644 --- a/llvm/lib/Target/SPIRV/SPIRVCombinerHelper.cpp +++ b/llvm/lib/Target/SPIRV/SPIRVCombinerHelper.cpp @@ -87,7 +87,7 @@ bool SPIRVCombinerHelper::matchSelectToFaceForward(MachineInstr &MI) const { if (MI.getOpcode() != TargetOpcode::G_SELECT) return false; - // Check if the condition is a comparison between a dot product and zero + // Check if select's condition is a comparison between a dot product and 0. Register CondReg = MI.getOperand(1).getReg(); MachineInstr *CondInstr = MRI.getVRegDef(CondReg); if (!CondInstr || CondInstr->getOpcode() != TargetOpcode::G_FCMP) @@ -95,7 +95,6 @@ bool SPIRVCombinerHelper::matchSelectToFaceForward(MachineInstr &MI) const { Register DotReg = CondInstr->getOperand(2).getReg(); MachineInstr *DotInstr = MRI.getVRegDef(DotReg); - // fmul? if (DotInstr->getOpcode() != TargetOpcode::G_INTRINSIC || cast(DotInstr)->getIntrinsicID() != Intrinsic::spv_fdot) return false; @@ -106,7 +105,7 @@ bool SPIRVCombinerHelper::matchSelectToFaceForward(MachineInstr &MI) const { !CondZeroInstr->getOperand(1).getFPImm()->isZero()) return false; - // Check if the false operand is the negation of the true operand + // Check if select's false operand is the negation of the true operand. Register TrueReg = MI.getOperand(2).getReg(); Register FalseReg = MI.getOperand(3).getReg(); MachineInstr *FalseInstr = MRI.getVRegDef(FalseReg); @@ -124,17 +123,17 @@ void SPIRVCombinerHelper::applySPIRVFaceForward(MachineInstr &MI) const { MachineInstr *CondInstr = MRI.getVRegDef(CondReg); Register DotReg = CondInstr->getOperand(2).getReg(); MachineInstr *DotInstr = MRI.getVRegDef(DotReg); - Register DotOperand1 = DotInstr->getOperand(2).getReg(); // I - Register DotOperand2 = DotInstr->getOperand(3).getReg(); // Ng - Register TrueReg = MI.getOperand(2).getReg(); // N + Register DotOperand1 = DotInstr->getOperand(2).getReg(); + Register DotOperand2 = DotInstr->getOperand(3).getReg(); + Register TrueReg = MI.getOperand(2).getReg(); - // Remove the original `select` instruction + // Remove the original `select` instruction. Register ResultReg = MI.getOperand(0).getReg(); DebugLoc DL = MI.getDebugLoc(); MachineBasicBlock &MBB = *MI.getParent(); MachineBasicBlock::iterator InsertPt = MI.getIterator(); - // Build the `spv_faceforward` intrinsic + // Build the `spv_faceforward` intrinsic. MachineInstrBuilder NewInstr = BuildMI(MBB, InsertPt, DL, Builder.getTII().get(TargetOpcode::G_INTRINSIC)); NewInstr diff --git a/llvm/test/CodeGen/SPIRV/hlsl-intrinsics/faceforward.ll b/llvm/test/CodeGen/SPIRV/hlsl-intrinsics/faceforward.ll index 47c0d321594c6..f181be8ee1200 100644 --- a/llvm/test/CodeGen/SPIRV/hlsl-intrinsics/faceforward.ll +++ b/llvm/test/CodeGen/SPIRV/hlsl-intrinsics/faceforward.ll @@ -1,7 +1,5 @@ -; RUN: llc -O0 -verify-machineinstrs -mtriple=spirv-unknown-vulkan %s -o - | FileCheck %s -; RUN: %if spirv-tools %{ llc -O0 -mtriple=spirv-unknown-vulkan %s -o - -filetype=obj | spirv-val --target-env spv1.4 %} - -; FIXME(#136344): Change --target-env to vulkan1.3 and update this test accordingly once the issue is resolved. +; RUN: llc -O0 -verify-machineinstrs -mtriple=spirv-vulkan1.3-unknown %s -o - | FileCheck %s +; RUN: %if spirv-tools %{ llc -O0 -mtriple=spirv-vulkan1.3-unknown %s -o - -filetype=obj | spirv-val --target-env vulkan1.3 %} ; Make sure SPIRV operation function calls for faceforward are lowered correctly. @@ -11,7 +9,7 @@ ; CHECK-DAG: %[[#float_32:]] = OpTypeFloat 32 ; CHECK-DAG: %[[#vec4_float_32:]] = OpTypeVector %[[#float_32]] 4 -define noundef half @faceforward_half(half noundef %a, half noundef %b, half noundef %c) { +define internal noundef half @faceforward_half(half noundef %a, half noundef %b, half noundef %c) { entry: ; CHECK: %[[#]] = OpFunction %[[#float_16]] None %[[#]] ; CHECK: %[[#arg0:]] = OpFunctionParameter %[[#float_16]] @@ -22,7 +20,7 @@ entry: ret half %spv.faceforward } -define noundef float @faceforward_float(float noundef %a, float noundef %b, float noundef %c) { +define internal noundef float @faceforward_float(float noundef %a, float noundef %b, float noundef %c) { entry: ; CHECK: %[[#]] = OpFunction %[[#float_32]] None %[[#]] ; CHECK: %[[#arg0:]] = OpFunctionParameter %[[#float_32]] @@ -33,7 +31,7 @@ entry: ret float %spv.faceforward } -define noundef <4 x half> @faceforward_half4(<4 x half> noundef %a, <4 x half> noundef %b, <4 x half> noundef %c) { +define internal noundef <4 x half> @faceforward_half4(<4 x half> noundef %a, <4 x half> noundef %b, <4 x half> noundef %c) { entry: ; CHECK: %[[#]] = OpFunction %[[#vec4_float_16]] None %[[#]] ; CHECK: %[[#arg0:]] = OpFunctionParameter %[[#vec4_float_16]] @@ -44,7 +42,7 @@ entry: ret <4 x half> %spv.faceforward } -define noundef <4 x float> @faceforward_float4(<4 x float> noundef %a, <4 x float> noundef %b, <4 x float> noundef %c) { +define internal noundef <4 x float> @faceforward_float4(<4 x float> noundef %a, <4 x float> noundef %b, <4 x float> noundef %c) { entry: ; CHECK: %[[#]] = OpFunction %[[#vec4_float_32]] None %[[#]] ; CHECK: %[[#arg0:]] = OpFunctionParameter %[[#vec4_float_32]] @@ -55,7 +53,7 @@ entry: ret <4 x float> %spv.faceforward } -define noundef <4 x float> @faceforward_instcombine_float4(<4 x float> noundef %a, <4 x float> noundef %b, <4 x float> noundef %c) { +define internal noundef <4 x float> @faceforward_instcombine_float4(<4 x float> noundef %a, <4 x float> noundef %b, <4 x float> noundef %c) { entry: ; CHECK: %[[#]] = OpFunction %[[#vec4_float_32]] None %[[#]] ; CHECK: %[[#arg0:]] = OpFunctionParameter %[[#vec4_float_32]] @@ -64,13 +62,21 @@ entry: ; CHECK: %[[#]] = OpExtInst %[[#vec4_float_32]] %[[#op_ext_glsl]] FaceForward %[[#arg0]] %[[#arg1]] %[[#arg2]] %spv.fdot = call float @llvm.spv.fdot.v4f32(<4 x float> %b, <4 x float> %c) %fcmp = fcmp olt float %spv.fdot, 0.000000e+00 - %delta = fneg <4 x float> %a - %select = select i1 %fcmp, <4 x float> %a, <4 x float> %delta + %fneg = fneg <4 x float> %a + %select = select i1 %fcmp, <4 x float> %a, <4 x float> %fneg ret <4 x float> %select } +; The other fucntions are the test, but a entry point is required to have a valid SPIR-V module. +define void @main() #1 { +entry: + ret void +} + declare half @llvm.spv.faceforward.f16(half, half, half) declare float @llvm.spv.faceforward.f32(float, float, float) declare <4 x half> @llvm.spv.faceforward.v4f16(<4 x half>, <4 x half>, <4 x half>) declare <4 x float> @llvm.spv.faceforward.v4f32(<4 x float>, <4 x float>, <4 x float>) + +attributes #1 = { convergent noinline norecurse "hlsl.numthreads"="1,1,1" "hlsl.shader"="compute" "no-trapping-math"="true" "stack-protector-buffer-size"="8" } From 154c8df390fd598aef66847ef170143b7b1c9d2c Mon Sep 17 00:00:00 2001 From: kmpeng Date: Wed, 14 May 2025 13:04:29 -0700 Subject: [PATCH 4/9] added scalar implementation and tests --- llvm/lib/Target/SPIRV/SPIRVCombinerHelper.cpp | 19 ++++++++---- ...egalizercombiner-select-to-faceforward.mir | 29 +++++++++++++++++++ .../SPIRV/hlsl-intrinsics/faceforward.ll | 14 +++++++++ 3 files changed, 56 insertions(+), 6 deletions(-) diff --git a/llvm/lib/Target/SPIRV/SPIRVCombinerHelper.cpp b/llvm/lib/Target/SPIRV/SPIRVCombinerHelper.cpp index 1f4c3b05ea194..daf2b26ae6177 100644 --- a/llvm/lib/Target/SPIRV/SPIRVCombinerHelper.cpp +++ b/llvm/lib/Target/SPIRV/SPIRVCombinerHelper.cpp @@ -95,8 +95,9 @@ bool SPIRVCombinerHelper::matchSelectToFaceForward(MachineInstr &MI) const { Register DotReg = CondInstr->getOperand(2).getReg(); MachineInstr *DotInstr = MRI.getVRegDef(DotReg); - if (DotInstr->getOpcode() != TargetOpcode::G_INTRINSIC || - cast(DotInstr)->getIntrinsicID() != Intrinsic::spv_fdot) + if (DotInstr->getOpcode() != TargetOpcode::G_FMUL && + (DotInstr->getOpcode() != TargetOpcode::G_INTRINSIC || + cast(DotInstr)->getIntrinsicID() != Intrinsic::spv_fdot)) return false; Register CondZeroReg = CondInstr->getOperand(3).getReg(); @@ -123,8 +124,14 @@ void SPIRVCombinerHelper::applySPIRVFaceForward(MachineInstr &MI) const { MachineInstr *CondInstr = MRI.getVRegDef(CondReg); Register DotReg = CondInstr->getOperand(2).getReg(); MachineInstr *DotInstr = MRI.getVRegDef(DotReg); - Register DotOperand1 = DotInstr->getOperand(2).getReg(); - Register DotOperand2 = DotInstr->getOperand(3).getReg(); + Register DotOperand1, DotOperand2; + if (DotInstr->getOpcode() == TargetOpcode::G_FMUL) { + DotOperand1 = DotInstr->getOperand(1).getReg(); + DotOperand2 = DotInstr->getOperand(2).getReg(); + } else { + DotOperand1 = DotInstr->getOperand(2).getReg(); + DotOperand2 = DotInstr->getOperand(3).getReg(); + } Register TrueReg = MI.getOperand(2).getReg(); // Remove the original `select` instruction. @@ -134,8 +141,8 @@ void SPIRVCombinerHelper::applySPIRVFaceForward(MachineInstr &MI) const { MachineBasicBlock::iterator InsertPt = MI.getIterator(); // Build the `spv_faceforward` intrinsic. - MachineInstrBuilder NewInstr = - BuildMI(MBB, InsertPt, DL, Builder.getTII().get(TargetOpcode::G_INTRINSIC)); + MachineInstrBuilder NewInstr = BuildMI( + MBB, InsertPt, DL, Builder.getTII().get(TargetOpcode::G_INTRINSIC)); NewInstr .addDef(ResultReg) // Result register .addIntrinsicID(Intrinsic::spv_faceforward) // Intrinsic ID diff --git a/llvm/test/CodeGen/SPIRV/GlobalISel/InstCombine/prelegalizercombiner-select-to-faceforward.mir b/llvm/test/CodeGen/SPIRV/GlobalISel/InstCombine/prelegalizercombiner-select-to-faceforward.mir index 4fbc3d0735b68..08f03942460ba 100644 --- a/llvm/test/CodeGen/SPIRV/GlobalISel/InstCombine/prelegalizercombiner-select-to-faceforward.mir +++ b/llvm/test/CodeGen/SPIRV/GlobalISel/InstCombine/prelegalizercombiner-select-to-faceforward.mir @@ -1,6 +1,35 @@ # RUN: llc -verify-machineinstrs -O0 -mtriple spirv-unknown-unknown -run-pass=spirv-prelegalizer-combiner %s -o - | FileCheck %s # REQUIRES: asserts --- +name: faceforward_instcombine_float +tracksRegLiveness: true +legalized: true +body: | + bb.1.entry: + ; CHECK-LABEL: name: faceforward_instcombine_float + ; CHECK-NOT: %9:_(s32) = G_FCONSTANT float 0.000000e+00 + ; CHECK-NOT: %8:_(s32) = G_FMUL %1:fid, %2:fid + ; CHECK-NOT: %10:_(s1) = G_FCMP floatpred(olt), %8:_(s32), %9:_ + ; CHECK-NOT: %11:_(s32) = G_FNEG %0:fid + ; CHECK-NOT: %12:id(s32) = G_SELECT %10:_(s1), %0:fid, %11:_ + ; CHECK: %10:id(s32) = G_INTRINSIC intrinsic(@llvm.spv.faceforward), %2(s32), %3(s32), %4(s32) + %3:type(s64) = OpTypeFloat 32 + %5:type(s64) = OpTypeFunction %3:type(s64), %3:type(s64), %3:type(s64), %3:type(s64) + OpName %0:fid(s32), 97 + OpName %1:fid(s32), 98 + OpName %2:fid(s32), 99 + %4:iid(s64) = OpFunction %3:type(s64), 0, %5:type(s64) + %0:fid(s32) = OpFunctionParameter %3:type(s64) + %1:fid(s32) = OpFunctionParameter %3:type(s64) + %2:fid(s32) = OpFunctionParameter %3:type(s64) + OpName %4:iid(s64), 1701011814, 2003988326, 1600418401, 1953721961, 1651339107, 1600482921, 1634692198, 116 + %9:_(s32) = G_FCONSTANT float 0.000000e+00 + %8:_(s32) = G_FMUL %1:fid, %2:fid + %10:_(s1) = G_FCMP floatpred(olt), %8:_(s32), %9:_ + %11:_(s32) = G_FNEG %0:fid + %12:id(s32) = G_SELECT %10:_(s1), %0:fid, %11:_ + OpReturnValue %12:id(s32) +--- name: faceforward_instcombine_float4 tracksRegLiveness: true legalized: true diff --git a/llvm/test/CodeGen/SPIRV/hlsl-intrinsics/faceforward.ll b/llvm/test/CodeGen/SPIRV/hlsl-intrinsics/faceforward.ll index f181be8ee1200..6811f95bbab17 100644 --- a/llvm/test/CodeGen/SPIRV/hlsl-intrinsics/faceforward.ll +++ b/llvm/test/CodeGen/SPIRV/hlsl-intrinsics/faceforward.ll @@ -53,6 +53,20 @@ entry: ret <4 x float> %spv.faceforward } +define internal noundef float @faceforward_instcombine_float(float noundef %a, float noundef %b, float noundef %c) { +entry: + ; CHECK: %[[#]] = OpFunction %[[#float_32]] None %[[#]] + ; CHECK: %[[#arg0:]] = OpFunctionParameter %[[#float_32]] + ; CHECK: %[[#arg1:]] = OpFunctionParameter %[[#float_32]] + ; CHECK: %[[#arg2:]] = OpFunctionParameter %[[#float_32]] + ; CHECK: %[[#]] = OpExtInst %[[#float_32]] %[[#op_ext_glsl]] FaceForward %[[#arg0]] %[[#arg1]] %[[#arg2]] + %fmul= fmul float %b, %c + %fcmp = fcmp olt float %fmul, 0.000000e+00 + %fneg = fneg float %a + %select = select i1 %fcmp, float %a, float %fneg + ret float %select + } + define internal noundef <4 x float> @faceforward_instcombine_float4(<4 x float> noundef %a, <4 x float> noundef %b, <4 x float> noundef %c) { entry: ; CHECK: %[[#]] = OpFunction %[[#vec4_float_32]] None %[[#]] From 48d3ccb89e3ca94f6e2da6787ef091e53d844760 Mon Sep 17 00:00:00 2001 From: kmpeng Date: Fri, 16 May 2025 16:51:35 -0700 Subject: [PATCH 5/9] partially address PR comments --- llvm/lib/Target/SPIRV/SPIRVCombinerHelper.cpp | 20 +++++++++++++++---- ...egalizercombiner-select-to-faceforward.mir | 4 +++- 2 files changed, 19 insertions(+), 5 deletions(-) diff --git a/llvm/lib/Target/SPIRV/SPIRVCombinerHelper.cpp b/llvm/lib/Target/SPIRV/SPIRVCombinerHelper.cpp index daf2b26ae6177..cf1055fb09f3c 100644 --- a/llvm/lib/Target/SPIRV/SPIRVCombinerHelper.cpp +++ b/llvm/lib/Target/SPIRV/SPIRVCombinerHelper.cpp @@ -83,7 +83,13 @@ void SPIRVCombinerHelper::applySPIRVDistance(MachineInstr &MI) const { /// (vXf32 (g_intrinsic faceforward /// (vXf32 N) (vXf32 I) (vXf32 Ng))) /// +/// This only works for Vulkan targets. +/// bool SPIRVCombinerHelper::matchSelectToFaceForward(MachineInstr &MI) const { + const SPIRVSubtarget &ST = MI.getMF()->getSubtarget(); + if (!ST.isVulkanEnv()) + return false; + if (MI.getOpcode() != TargetOpcode::G_SELECT) return false; @@ -95,10 +101,16 @@ bool SPIRVCombinerHelper::matchSelectToFaceForward(MachineInstr &MI) const { Register DotReg = CondInstr->getOperand(2).getReg(); MachineInstr *DotInstr = MRI.getVRegDef(DotReg); - if (DotInstr->getOpcode() != TargetOpcode::G_FMUL && - (DotInstr->getOpcode() != TargetOpcode::G_INTRINSIC || - cast(DotInstr)->getIntrinsicID() != Intrinsic::spv_fdot)) - return false; + if ((DotInstr->getOpcode() != TargetOpcode::G_INTRINSIC || + cast(DotInstr)->getIntrinsicID() != Intrinsic::spv_fdot)) { + if (DotInstr->getOpcode() != TargetOpcode::G_FMUL) + return false; + Register DotOperand1 = DotInstr->getOperand(1).getReg(); + Register DotOperand2 = DotInstr->getOperand(2).getReg(); + if (MRI.getType(DotOperand1).isVector() || + MRI.getType(DotOperand2).isVector()) + return false; + } Register CondZeroReg = CondInstr->getOperand(3).getReg(); MachineInstr *CondZeroInstr = MRI.getVRegDef(CondZeroReg); diff --git a/llvm/test/CodeGen/SPIRV/GlobalISel/InstCombine/prelegalizercombiner-select-to-faceforward.mir b/llvm/test/CodeGen/SPIRV/GlobalISel/InstCombine/prelegalizercombiner-select-to-faceforward.mir index 08f03942460ba..dc7cc03e11613 100644 --- a/llvm/test/CodeGen/SPIRV/GlobalISel/InstCombine/prelegalizercombiner-select-to-faceforward.mir +++ b/llvm/test/CodeGen/SPIRV/GlobalISel/InstCombine/prelegalizercombiner-select-to-faceforward.mir @@ -13,6 +13,7 @@ body: | ; CHECK-NOT: %11:_(s32) = G_FNEG %0:fid ; CHECK-NOT: %12:id(s32) = G_SELECT %10:_(s1), %0:fid, %11:_ ; CHECK: %10:id(s32) = G_INTRINSIC intrinsic(@llvm.spv.faceforward), %2(s32), %3(s32), %4(s32) + ; CHECK: OpReturnValue %10(s32) %3:type(s64) = OpTypeFloat 32 %5:type(s64) = OpTypeFunction %3:type(s64), %3:type(s64), %3:type(s64), %3:type(s64) OpName %0:fid(s32), 97 @@ -42,6 +43,7 @@ body: | ; CHECK-NOT: %12:_(<4 x s32>) = G_FNEG %0:vfid ; CHECK-NOT: %13:id(<4 x s32>) = G_SELECT %11:_(s1), %0:vfid, %12:_ ; CHECK: %11:id(<4 x s32>) = G_INTRINSIC intrinsic(@llvm.spv.faceforward), %3(<4 x s32>), %4(<4 x s32>), %5(<4 x s32>) + ; CHECK: OpReturnValue %11(<4 x s32>) %4:type(s64) = OpTypeVector %3:type(s64), 4 %6:type(s64) = OpTypeFunction %4:type(s64), %4:type(s64), %4:type(s64), %4:type(s64) %3:type(s64) = OpTypeFloat 32 @@ -60,4 +62,4 @@ body: | %12:_(<4 x s32>) = G_FNEG %0:vfid %13:id(<4 x s32>) = G_SELECT %11:_(s1), %0:vfid, %12:_ OpReturnValue %13:id(<4 x s32>) - \ No newline at end of file + From 2a651752ef460025e28666a9143ea5618e9d8a9e Mon Sep 17 00:00:00 2001 From: kmpeng Date: Fri, 12 Sep 2025 16:06:34 -0700 Subject: [PATCH 6/9] add opencl test, fix .mir test, fix vulkan check in combiner --- llvm/lib/Target/SPIRV/SPIRVCombinerHelper.cpp | 2 +- ...egalizercombiner-select-to-faceforward.mir | 2 +- llvm/test/CodeGen/SPIRV/opencl/faceforward.ll | 29 +++++++++++++++++++ 3 files changed, 31 insertions(+), 2 deletions(-) create mode 100644 llvm/test/CodeGen/SPIRV/opencl/faceforward.ll diff --git a/llvm/lib/Target/SPIRV/SPIRVCombinerHelper.cpp b/llvm/lib/Target/SPIRV/SPIRVCombinerHelper.cpp index cf1055fb09f3c..cfde19e5fcb1c 100644 --- a/llvm/lib/Target/SPIRV/SPIRVCombinerHelper.cpp +++ b/llvm/lib/Target/SPIRV/SPIRVCombinerHelper.cpp @@ -87,7 +87,7 @@ void SPIRVCombinerHelper::applySPIRVDistance(MachineInstr &MI) const { /// bool SPIRVCombinerHelper::matchSelectToFaceForward(MachineInstr &MI) const { const SPIRVSubtarget &ST = MI.getMF()->getSubtarget(); - if (!ST.isVulkanEnv()) + if (!ST.isShader()) return false; if (MI.getOpcode() != TargetOpcode::G_SELECT) diff --git a/llvm/test/CodeGen/SPIRV/GlobalISel/InstCombine/prelegalizercombiner-select-to-faceforward.mir b/llvm/test/CodeGen/SPIRV/GlobalISel/InstCombine/prelegalizercombiner-select-to-faceforward.mir index dc7cc03e11613..7fd29c2d101a6 100644 --- a/llvm/test/CodeGen/SPIRV/GlobalISel/InstCombine/prelegalizercombiner-select-to-faceforward.mir +++ b/llvm/test/CodeGen/SPIRV/GlobalISel/InstCombine/prelegalizercombiner-select-to-faceforward.mir @@ -1,4 +1,4 @@ -# RUN: llc -verify-machineinstrs -O0 -mtriple spirv-unknown-unknown -run-pass=spirv-prelegalizer-combiner %s -o - | FileCheck %s +# RUN: llc -verify-machineinstrs -O0 -mtriple spirv-vulkan1.3-unknown -run-pass=spirv-prelegalizer-combiner %s -o - | FileCheck %s # REQUIRES: asserts --- name: faceforward_instcombine_float diff --git a/llvm/test/CodeGen/SPIRV/opencl/faceforward.ll b/llvm/test/CodeGen/SPIRV/opencl/faceforward.ll new file mode 100644 index 0000000000000..4da57f80058e1 --- /dev/null +++ b/llvm/test/CodeGen/SPIRV/opencl/faceforward.ll @@ -0,0 +1,29 @@ +; RUN: llc -verify-machineinstrs -O0 -mtriple=spirv64-unknown-unknown %s -o - | FileCheck %s +; RUN: llc -verify-machineinstrs -O0 -mtriple=spirv32-unknown-unknown %s -o - | FileCheck %s +; RUN: %if spirv-tools %{ llc -O0 -mtriple=spirv64-unknown-unknown %s -o - -filetype=obj | spirv-val %} +; RUN: %if spirv-tools %{ llc -O0 -mtriple=spirv32-unknown-unknown %s -o - -filetype=obj | spirv-val %} + +; Make sure we don't pattern match to faceforward in OpenCL. + +; CHECK-DAG: %[[#op_ext_glsl:]] = OpExtInstImport "OpenCL.std" + +; CHECK-DAG: %[[#float_32:]] = OpTypeFloat 32 +; CHECK-DAG: %[[#bool:]] = OpTypeBool +; CHECK-DAG: %[[#zero:]] = OpConstantNull %[[#float_32]] + +define internal noundef float @faceforward_no_instcombine_float(float noundef %a, float noundef %b, float noundef %c) { +entry: + ; CHECK: %[[#]] = OpFunction %[[#float_32]] None %[[#]] + ; CHECK: %[[#arg0:]] = OpFunctionParameter %[[#float_32]] + ; CHECK: %[[#arg1:]] = OpFunctionParameter %[[#float_32]] + ; CHECK: %[[#arg2:]] = OpFunctionParameter %[[#float_32]] + ; CHECK: %[[#fmul:]] = OpFMul %[[#float_32]] %[[#arg1]] %[[#arg2]] + ; CHECK: %[[#fcmp:]] = OpFOrdLessThan %[[#bool]] %[[#fmul]] %[[#zero]] + ; CHECK: %[[#fneg:]] = OpFNegate %[[#float_32]] %[[#arg0]] + ; CHECK: %[[#select:]] = OpSelect %[[#float_32]] %[[#fcmp]] %[[#arg0]] %[[#fneg]] + %fmul= fmul float %b, %c + %fcmp = fcmp olt float %fmul, 0.000000e+00 + %fneg = fneg float %a + %select = select i1 %fcmp, float %a, float %fneg + ret float %select + } From fa580b98e4954f5721632d7c615e305dc1f8130a Mon Sep 17 00:00:00 2001 From: kmpeng Date: Wed, 24 Sep 2025 03:17:27 -0700 Subject: [PATCH 7/9] undo frontend changes, change pattern matching to use mi_match, add constant tests --- .../lib/Headers/hlsl/hlsl_intrinsic_helpers.h | 4 + .../CodeGenHLSL/builtins/faceforward.hlsl | 56 ++++-------- llvm/lib/Target/SPIRV/SPIRVCombinerHelper.cpp | 85 ++++++++++--------- ...egalizercombiner-select-to-faceforward.mir | 70 ++++++++++++++- .../SPIRV/hlsl-intrinsics/faceforward.ll | 26 ++++++ 5 files changed, 162 insertions(+), 79 deletions(-) diff --git a/clang/lib/Headers/hlsl/hlsl_intrinsic_helpers.h b/clang/lib/Headers/hlsl/hlsl_intrinsic_helpers.h index 4926e8092223a..c877234479ad1 100644 --- a/clang/lib/Headers/hlsl/hlsl_intrinsic_helpers.h +++ b/clang/lib/Headers/hlsl/hlsl_intrinsic_helpers.h @@ -137,7 +137,11 @@ template constexpr vector lit_impl(T NDotL, T NDotH, T M) { } template constexpr T faceforward_impl(T N, T I, T Ng) { +#if (__has_builtin(__builtin_spirv_faceforward)) + return __builtin_spirv_faceforward(N, I, Ng); +#else return select(dot(I, Ng) < 0, N, -N); +#endif } template constexpr T ldexp_impl(T X, T Exp) { diff --git a/clang/test/CodeGenHLSL/builtins/faceforward.hlsl b/clang/test/CodeGenHLSL/builtins/faceforward.hlsl index f12f75f384964..d2ece57aba4ae 100644 --- a/clang/test/CodeGenHLSL/builtins/faceforward.hlsl +++ b/clang/test/CodeGenHLSL/builtins/faceforward.hlsl @@ -12,11 +12,8 @@ // CHECK: %hlsl.select.i = select reassoc nnan ninf nsz arcp afn i1 %cmp.i, half %{{.*}}, half %fneg.i // CHECK: ret half %hlsl.select.i // SPVCHECK-LABEL: test_faceforward_half -// SPVCHECK: %hlsl.dot.i = fmul reassoc nnan ninf nsz arcp afn half %{{.*}}, %{{.*}} -// SPVCHECK: %cmp.i = fcmp reassoc nnan ninf nsz arcp afn olt half %hlsl.dot.i, 0xH0000 -// SPVCHECK: %fneg.i = fneg reassoc nnan ninf nsz arcp afn half %{{.*}} -// SPVCHECK: %hlsl.select.i = select reassoc nnan ninf nsz arcp afn i1 %cmp.i, half %{{.*}}, half %fneg.i -// SPVCHECK: ret half %hlsl.select.i +// SPVCHECK: %spv.faceforward.i = call reassoc nnan ninf nsz arcp afn noundef half @llvm.spv.faceforward.f16(half %{{.*}}, half %{{.*}}, half %{{.*}}) +// SPVCHECK: ret half %spv.faceforward.i half test_faceforward_half(half N, half I, half Ng) { return faceforward(N, I, Ng); } // CHECK-LABEL: test_faceforward_half2 @@ -26,11 +23,8 @@ half test_faceforward_half(half N, half I, half Ng) { return faceforward(N, I, N // CHECK: %hlsl.select.i = select reassoc nnan ninf nsz arcp afn i1 %cmp.i, <2 x half> %{{.*}}, <2 x half> %fneg.i // CHECK: ret <2 x half> %hlsl.select.i // SPVCHECK-LABEL: test_faceforward_half2 -// SPVCHECK: %hlsl.dot.i = call reassoc nnan ninf nsz arcp afn half @llvm.spv.fdot.v2f16(<2 x half> %{{.*}}, <2 x half> %{{.*}}) -// SPVCHECK: %cmp.i = fcmp reassoc nnan ninf nsz arcp afn olt half %hlsl.dot.i, 0xH0000 -// SPVCHECK: %fneg.i = fneg reassoc nnan ninf nsz arcp afn <2 x half> %{{.*}} -// SPVCHECK: %hlsl.select.i = select reassoc nnan ninf nsz arcp afn i1 %cmp.i, <2 x half> %{{.*}}, <2 x half> %fneg.i -// SPVCHECK: ret <2 x half> %hlsl.select.i +// SPVCHECK: %spv.faceforward.i = call reassoc nnan ninf nsz arcp afn noundef <2 x half> @llvm.spv.faceforward.v2f16(<2 x half> %{{.*}}, <2 x half> %{{.*}}, <2 x half> %{{.*}}) +// SPVCHECK: ret <2 x half> %spv.faceforward.i half2 test_faceforward_half2(half2 N, half2 I, half2 Ng) { return faceforward(N, I, Ng); } // CHECK-LABEL: test_faceforward_half3 @@ -40,11 +34,8 @@ half2 test_faceforward_half2(half2 N, half2 I, half2 Ng) { return faceforward(N, // CHECK: %hlsl.select.i = select reassoc nnan ninf nsz arcp afn i1 %cmp.i, <3 x half> %{{.*}}, <3 x half> %fneg.i // CHECK: ret <3 x half> %hlsl.select.i // SPVCHECK-LABEL: test_faceforward_half3 -// SPVCHECK: %hlsl.dot.i = call reassoc nnan ninf nsz arcp afn half @llvm.spv.fdot.v3f16(<3 x half> %{{.*}}, <3 x half> %{{.*}}) -// SPVCHECK: %cmp.i = fcmp reassoc nnan ninf nsz arcp afn olt half %hlsl.dot.i, 0xH0000 -// SPVCHECK: %fneg.i = fneg reassoc nnan ninf nsz arcp afn <3 x half> %{{.*}} -// SPVCHECK: %hlsl.select.i = select reassoc nnan ninf nsz arcp afn i1 %cmp.i, <3 x half> %{{.*}}, <3 x half> %fneg.i -// SPVCHECK: ret <3 x half> %hlsl.select.i +// SPVCHECK: %spv.faceforward.i = call reassoc nnan ninf nsz arcp afn noundef <3 x half> @llvm.spv.faceforward.v3f16(<3 x half> %{{.*}}, <3 x half> %{{.*}}, <3 x half> %{{.*}}) +// SPVCHECK: ret <3 x half> %spv.faceforward.i half3 test_faceforward_half3(half3 N, half3 I, half3 Ng) { return faceforward(N, I, Ng); } // CHECK-LABEL: test_faceforward_half4 @@ -54,11 +45,8 @@ half3 test_faceforward_half3(half3 N, half3 I, half3 Ng) { return faceforward(N, // CHECK: %hlsl.select.i = select reassoc nnan ninf nsz arcp afn i1 %cmp.i, <4 x half> %{{.*}}, <4 x half> %fneg.i // CHECK: ret <4 x half> %hlsl.select.i // SPVCHECK-LABEL: test_faceforward_half4 -// SPVCHECK: %hlsl.dot.i = call reassoc nnan ninf nsz arcp afn half @llvm.spv.fdot.v4f16(<4 x half> %{{.*}}, <4 x half> %{{.*}}) -// SPVCHECK: %cmp.i = fcmp reassoc nnan ninf nsz arcp afn olt half %hlsl.dot.i, 0xH0000 -// SPVCHECK: %fneg.i = fneg reassoc nnan ninf nsz arcp afn <4 x half> %{{.*}} -// SPVCHECK: %hlsl.select.i = select reassoc nnan ninf nsz arcp afn i1 %cmp.i, <4 x half> %{{.*}}, <4 x half> %fneg.i -// SPVCHECK: ret <4 x half> %hlsl.select.i +// SPVCHECK: %spv.faceforward.i = call reassoc nnan ninf nsz arcp afn noundef <4 x half> @llvm.spv.faceforward.v4f16(<4 x half> %{{.*}}, <4 x half> %{{.*}}, <4 x half> %{{.*}}) +// SPVCHECK: ret <4 x half> %spv.faceforward.i half4 test_faceforward_half4(half4 N, half4 I, half4 Ng) { return faceforward(N, I, Ng); } // CHECK-LABEL: test_faceforward_float @@ -68,11 +56,8 @@ half4 test_faceforward_half4(half4 N, half4 I, half4 Ng) { return faceforward(N, // CHECK: %hlsl.select.i = select reassoc nnan ninf nsz arcp afn i1 %cmp.i, float %{{.*}}, float %fneg.i // CHECK: ret float %hlsl.select.i // SPVCHECK-LABEL: test_faceforward_float -// SPVCHECK: %hlsl.dot.i = fmul reassoc nnan ninf nsz arcp afn float %{{.*}}, %{{.*}} -// SPVCHECK: %cmp.i = fcmp reassoc nnan ninf nsz arcp afn olt float %hlsl.dot.i, 0.000000e+00 -// SPVCHECK: %fneg.i = fneg reassoc nnan ninf nsz arcp afn float %{{.*}} -// SPVCHECK: %hlsl.select.i = select reassoc nnan ninf nsz arcp afn i1 %cmp.i, float %{{.*}}, float %fneg.i -// SPVCHECK: ret float %hlsl.select.i +// SPVCHECK: %spv.faceforward.i = call reassoc nnan ninf nsz arcp afn noundef float @llvm.spv.faceforward.f32(float %{{.*}}, float %{{.*}}, float %{{.*}}) +// SPVCHECK: ret float %spv.faceforward.i float test_faceforward_float(float N, float I, float Ng) { return faceforward(N, I, Ng); } // CHECK-LABEL: test_faceforward_float2 @@ -82,11 +67,8 @@ float test_faceforward_float(float N, float I, float Ng) { return faceforward(N, // CHECK: %hlsl.select.i = select reassoc nnan ninf nsz arcp afn i1 %cmp.i, <2 x float> %{{.*}}, <2 x float> %fneg.i // CHECK: ret <2 x float> %hlsl.select.i // SPVCHECK-LABEL: test_faceforward_float2 -// SPVCHECK: %hlsl.dot.i = call reassoc nnan ninf nsz arcp afn float @llvm.spv.fdot.v2f32(<2 x float> %{{.*}}, <2 x float> %{{.*}}) -// SPVCHECK: %cmp.i = fcmp reassoc nnan ninf nsz arcp afn olt float %hlsl.dot.i, 0.000000e+00 -// SPVCHECK: %fneg.i = fneg reassoc nnan ninf nsz arcp afn <2 x float> %{{.*}} -// SPVCHECK: %hlsl.select.i = select reassoc nnan ninf nsz arcp afn i1 %cmp.i, <2 x float> %{{.*}}, <2 x float> %fneg.i -// SPVCHECK: ret <2 x float> %hlsl.select.i +// SPVCHECK: %spv.faceforward.i = call reassoc nnan ninf nsz arcp afn noundef <2 x float> @llvm.spv.faceforward.v2f32(<2 x float> %{{.*}}, <2 x float> %{{.*}}, <2 x float> %{{.*}}) +// SPVCHECK: ret <2 x float> %spv.faceforward.i float2 test_faceforward_float2(float2 N, float2 I, float2 Ng) { return faceforward(N, I, Ng); } // CHECK-LABEL: test_faceforward_float3 @@ -96,11 +78,8 @@ float2 test_faceforward_float2(float2 N, float2 I, float2 Ng) { return faceforwa // CHECK: %hlsl.select.i = select reassoc nnan ninf nsz arcp afn i1 %cmp.i, <3 x float> %{{.*}}, <3 x float> %fneg.i // CHECK: ret <3 x float> %hlsl.select.i // SPVCHECK-LABEL: test_faceforward_float3 -// SPVCHECK: %hlsl.dot.i = call reassoc nnan ninf nsz arcp afn float @llvm.spv.fdot.v3f32(<3 x float> %{{.*}}, <3 x float> %{{.*}}) -// SPVCHECK: %cmp.i = fcmp reassoc nnan ninf nsz arcp afn olt float %hlsl.dot.i, 0.000000e+00 -// SPVCHECK: %fneg.i = fneg reassoc nnan ninf nsz arcp afn <3 x float> %{{.*}} -// SPVCHECK: %hlsl.select.i = select reassoc nnan ninf nsz arcp afn i1 %cmp.i, <3 x float> %{{.*}}, <3 x float> %fneg.i -// SPVCHECK: ret <3 x float> %hlsl.select.i +// SPVCHECK: %spv.faceforward.i = call reassoc nnan ninf nsz arcp afn noundef <3 x float> @llvm.spv.faceforward.v3f32(<3 x float> %{{.*}}, <3 x float> %{{.*}}, <3 x float> %{{.*}}) +// SPVCHECK: ret <3 x float> %spv.faceforward.i float3 test_faceforward_float3(float3 N, float3 I, float3 Ng) { return faceforward(N, I, Ng); } // CHECK-LABEL: test_faceforward_float4 @@ -110,9 +89,6 @@ float3 test_faceforward_float3(float3 N, float3 I, float3 Ng) { return faceforwa // CHECK: %hlsl.select.i = select reassoc nnan ninf nsz arcp afn i1 %cmp.i, <4 x float> %{{.*}}, <4 x float> %fneg.i // CHECK: ret <4 x float> %hlsl.select.i // SPVCHECK-LABEL: test_faceforward_float4 -// SPVCHECK: %hlsl.dot.i = call reassoc nnan ninf nsz arcp afn float @llvm.spv.fdot.v4f32(<4 x float> %{{.*}}, <4 x float> %{{.*}}) -// SPVCHECK: %cmp.i = fcmp reassoc nnan ninf nsz arcp afn olt float %hlsl.dot.i, 0.000000e+00 -// SPVCHECK: %fneg.i = fneg reassoc nnan ninf nsz arcp afn <4 x float> %{{.*}} -// SPVCHECK: %hlsl.select.i = select reassoc nnan ninf nsz arcp afn i1 %cmp.i, <4 x float> %{{.*}}, <4 x float> %fneg.i -// SPVCHECK: ret <4 x float> %hlsl.select.i +// SPVCHECK: %spv.faceforward.i = call reassoc nnan ninf nsz arcp afn noundef <4 x float> @llvm.spv.faceforward.v4f32(<4 x float> %{{.*}}, <4 x float> %{{.*}}, <4 x float> %{{.*}}) +// SPVCHECK: ret <4 x float> %spv.faceforward.i float4 test_faceforward_float4(float4 N, float4 I, float4 Ng) { return faceforward(N, I, Ng); } diff --git a/llvm/lib/Target/SPIRV/SPIRVCombinerHelper.cpp b/llvm/lib/Target/SPIRV/SPIRVCombinerHelper.cpp index cfde19e5fcb1c..9e4a7ee49e611 100644 --- a/llvm/lib/Target/SPIRV/SPIRVCombinerHelper.cpp +++ b/llvm/lib/Target/SPIRV/SPIRVCombinerHelper.cpp @@ -72,13 +72,13 @@ void SPIRVCombinerHelper::applySPIRVDistance(MachineInstr &MI) const { } /// This match is part of a combine that -/// rewrites select(fcmp(dot(I, Ng), 0), N, 0 - N) to faceforward(N, I, Ng) +/// rewrites select(fcmp(dot(I, Ng), 0), N, -N) to faceforward(N, I, Ng) /// (vXf32 (g_select /// (g_fcmp /// (g_intrinsic dot(vXf32 I) (vXf32 Ng) /// 0) /// (vXf32 N) -/// (vXf32 g_fsub (0) (vXf32 N)))) +/// (vXf32 g_fneg (vXf32 N)))) /// -> /// (vXf32 (g_intrinsic faceforward /// (vXf32 N) (vXf32 I) (vXf32 Ng))) @@ -86,65 +86,70 @@ void SPIRVCombinerHelper::applySPIRVDistance(MachineInstr &MI) const { /// This only works for Vulkan targets. /// bool SPIRVCombinerHelper::matchSelectToFaceForward(MachineInstr &MI) const { - const SPIRVSubtarget &ST = MI.getMF()->getSubtarget(); - if (!ST.isShader()) + if (!MI.getMF()->getSubtarget().isShader()) return false; - if (MI.getOpcode() != TargetOpcode::G_SELECT) + // Match overall select pattern. + Register CondReg, TrueReg, FalseReg; + if (!mi_match(MI.getOperand(0).getReg(), MRI, + m_GISelect(m_Reg(CondReg), m_Reg(TrueReg), m_Reg(FalseReg)))) return false; - // Check if select's condition is a comparison between a dot product and 0. - Register CondReg = MI.getOperand(1).getReg(); - MachineInstr *CondInstr = MRI.getVRegDef(CondReg); - if (!CondInstr || CondInstr->getOpcode() != TargetOpcode::G_FCMP) + // Match the FCMP condition. + Register DotReg, CondZeroReg; + CmpInst::Predicate Pred; + if (!mi_match(CondReg, MRI, + m_GFCmp(m_Pred(Pred), m_Reg(DotReg), m_Reg(CondZeroReg))) || + Pred != CmpInst::FCMP_OLT) return false; - Register DotReg = CondInstr->getOperand(2).getReg(); + // Check if FCMP is a comparison between a dot product and 0. MachineInstr *DotInstr = MRI.getVRegDef(DotReg); if ((DotInstr->getOpcode() != TargetOpcode::G_INTRINSIC || cast(DotInstr)->getIntrinsicID() != Intrinsic::spv_fdot)) { - if (DotInstr->getOpcode() != TargetOpcode::G_FMUL) - return false; - Register DotOperand1 = DotInstr->getOperand(1).getReg(); - Register DotOperand2 = DotInstr->getOperand(2).getReg(); - if (MRI.getType(DotOperand1).isVector() || - MRI.getType(DotOperand2).isVector()) + Register DotOperand1, DotOperand2; + // Check for scalar dot product. + if (!mi_match(DotReg, MRI, + m_GFMul(m_Reg(DotOperand1), m_Reg(DotOperand2))) || + !MRI.getType(DotOperand1).isScalar() || + !MRI.getType(DotOperand2).isScalar()) return false; } - Register CondZeroReg = CondInstr->getOperand(3).getReg(); - MachineInstr *CondZeroInstr = MRI.getVRegDef(CondZeroReg); - if (CondZeroInstr->getOpcode() != TargetOpcode::G_FCONSTANT || - !CondZeroInstr->getOperand(1).getFPImm()->isZero()) + const ConstantFP *ZeroVal; + if (!mi_match(CondZeroReg, MRI, m_GFCst(ZeroVal)) || !ZeroVal->isZero()) return false; // Check if select's false operand is the negation of the true operand. - Register TrueReg = MI.getOperand(2).getReg(); - Register FalseReg = MI.getOperand(3).getReg(); - MachineInstr *FalseInstr = MRI.getVRegDef(FalseReg); - if (FalseInstr->getOpcode() != TargetOpcode::G_FNEG) - return false; - if (TrueReg != FalseInstr->getOperand(1).getReg()) - return false; + if (!mi_match(FalseReg, MRI, m_GFNeg(m_SpecificReg(TrueReg))) && + !mi_match(TrueReg, MRI, m_GFNeg(m_SpecificReg(FalseReg)))) { + // Check if they're constant opposites. + std::optional TrueVal, FalseVal; + if (!mi_match(TrueReg, MRI, m_GFCstOrSplat(TrueVal)) || + !mi_match(FalseReg, MRI, m_GFCstOrSplat(FalseVal))) + return false; + APFloat TrueValNegated = TrueVal->Value; + TrueValNegated.changeSign(); + if (FalseVal->Value.compare(TrueValNegated) != APFloat::cmpEqual) + return false; + } return true; } void SPIRVCombinerHelper::applySPIRVFaceForward(MachineInstr &MI) const { // Extract the operands for N, I, and Ng from the match criteria. - Register CondReg = MI.getOperand(1).getReg(); - MachineInstr *CondInstr = MRI.getVRegDef(CondReg); - Register DotReg = CondInstr->getOperand(2).getReg(); + Register CondReg, TrueReg, DotReg, DotOperand1, DotOperand2; + if (!mi_match(MI.getOperand(0).getReg(), MRI, + m_GISelect(m_Reg(CondReg), m_Reg(TrueReg), m_Reg()))) + return; + if (!mi_match(CondReg, MRI, m_GFCmp(m_Pred(), m_Reg(DotReg), m_Reg()))) + return; MachineInstr *DotInstr = MRI.getVRegDef(DotReg); - Register DotOperand1, DotOperand2; - if (DotInstr->getOpcode() == TargetOpcode::G_FMUL) { - DotOperand1 = DotInstr->getOperand(1).getReg(); - DotOperand2 = DotInstr->getOperand(2).getReg(); - } else { + if (!mi_match(DotReg, MRI, m_GFMul(m_Reg(DotOperand1), m_Reg(DotOperand2)))) { DotOperand1 = DotInstr->getOperand(2).getReg(); DotOperand2 = DotInstr->getOperand(3).getReg(); } - Register TrueReg = MI.getOperand(2).getReg(); // Remove the original `select` instruction. Register ResultReg = MI.getOperand(0).getReg(); @@ -164,7 +169,11 @@ void SPIRVCombinerHelper::applySPIRVFaceForward(MachineInstr &MI) const { SPIRVGlobalRegistry *GR = MI.getMF()->getSubtarget().getSPIRVGlobalRegistry(); - removeAllUses(CondReg, MRI, GR); // remove all uses of FCMP Result + removeAllUses(CondReg, MRI, GR); // Remove all uses of FCMP result + MachineInstr *CondInstr = MRI.getVRegDef(CondReg); GR->invalidateMachineInstr(CondInstr); - CondInstr->eraseFromParent(); // remove FCMP instruction + CondInstr->eraseFromParent(); // Remove FCMP instruction + removeAllUses(DotReg, MRI, GR); // Remove all uses of dot product result + GR->invalidateMachineInstr(DotInstr); + DotInstr->eraseFromParent(); // Remove dot product instruction } diff --git a/llvm/test/CodeGen/SPIRV/GlobalISel/InstCombine/prelegalizercombiner-select-to-faceforward.mir b/llvm/test/CodeGen/SPIRV/GlobalISel/InstCombine/prelegalizercombiner-select-to-faceforward.mir index 7fd29c2d101a6..65f4368fc480e 100644 --- a/llvm/test/CodeGen/SPIRV/GlobalISel/InstCombine/prelegalizercombiner-select-to-faceforward.mir +++ b/llvm/test/CodeGen/SPIRV/GlobalISel/InstCombine/prelegalizercombiner-select-to-faceforward.mir @@ -55,11 +55,79 @@ body: | %1:vfid(<4 x s32>) = OpFunctionParameter %4:type(s64) %2:vfid(<4 x s32>) = OpFunctionParameter %4:type(s64) OpName %5:iid(s64), 1701011814, 2003988326, 1600418401, 1953721961, 1651339107, 1600482921, 1634692198, 13428 - OpDecorate %5:iid(s64), 41, 1701011814, 2003988326, 1600418401, 1953721961, 1651339107, 1600482921, 1634692198, 13428, 0 %10:_(s32) = G_FCONSTANT float 0.000000e+00 %9:_(s32) = G_INTRINSIC intrinsic(@llvm.spv.fdot), %1:vfid(<4 x s32>), %2:vfid(<4 x s32>) %11:_(s1) = G_FCMP floatpred(olt), %9:_(s32), %10:_ %12:_(<4 x s32>) = G_FNEG %0:vfid %13:id(<4 x s32>) = G_SELECT %11:_(s1), %0:vfid, %12:_ OpReturnValue %13:id(<4 x s32>) +--- +name: faceforward_instcombine_float_constants +tracksRegLiveness: true +legalized: true +body: | + bb.1.entry: + ; CHECK-LABEL: name: faceforward_instcombine_float_constants + ; CHECK-NOT: %9:_(s32) = G_FCONSTANT float 0.000000e+00 + ; CHECK-NOT: %13:_(s32) = G_FCONSTANT float -1.000000e+00 + ; CHECK-NOT: %8:_(s32) = G_FMUL %1:fid, %2:fid + ; CHECK-NOT: %10:_(s1) = G_FCMP floatpred(olt), %8:_(s32), %9:_ + ; CHECK-NOT: %11:id(s32) = G_SELECT %10:_(s1), %12:_, %13:_ + ; CHECK: %7:_(s32) = G_FCONSTANT float 1.000000e+00 + ; CHECK: %11:id(s32) = G_INTRINSIC intrinsic(@llvm.spv.faceforward), %7(s32), %3(s32), %4(s32) + ; CHECK: OpReturnValue %11(s32) + %3:type(s64) = OpTypeFloat 32 + %5:type(s64) = OpTypeFunction %3:type(s64), %3:type(s64), %3:type(s64), %3:type(s64) + OpName %0:fid(s32), 97 + OpName %1:fid(s32), 98 + OpName %2:fid(s32), 99 + %4:iid(s64) = OpFunction %3:type(s64), 0, %5:type(s64) + %0:fid(s32) = OpFunctionParameter %3:type(s64) + %1:fid(s32) = OpFunctionParameter %3:type(s64) + %2:fid(s32) = OpFunctionParameter %3:type(s64) + OpName %4:iid(s64), 1701011814, 2003988326, 1600418401, 1953721961, 1651339107, 1600482921, 1634692198, 1868783476, 1635021678, 7566446 + %9:_(s32) = G_FCONSTANT float 0.000000e+00 + %12:_(s32) = G_FCONSTANT float 1.000000e+00 + %13:_(s32) = G_FCONSTANT float -1.000000e+00 + %8:_(s32) = G_FMUL %1:fid, %2:fid + %10:_(s1) = G_FCMP floatpred(olt), %8:_(s32), %9:_ + %11:id(s32) = G_SELECT %10:_(s1), %12:_, %13:_ + OpReturnValue %11:id(s32) +--- +name: faceforward_instcombine_float4_constants +tracksRegLiveness: true +legalized: true +body: | + bb.1.entry: + ; CHECK-LABEL: name: faceforward_instcombine_float4 + ; CHECK-NOT: %10:_(s32) = G_FCONSTANT float 0.000000e+00 + ; CHECK-NOT: %16:_(s32) = G_FCONSTANT float -1.000000e+00 + ; CHECK-NOT: %15:_(<4 x s32>) = G_BUILD_VECTOR %16:_(s32), %16:_(s32), %16:_(s32), %16:_(s32) + ; CHECK-NOT: %9:_(s32) = G_INTRINSIC intrinsic(@llvm.spv.fdot), %1:vfid(<4 x s32>), %2:vfid(<4 x s32>) + ; CHECK-NOT: %11:_(s1) = G_FCMP floatpred(olt), %9:_(s32), %10:_ + ; CHECK-NOT: %12:id(<4 x s32>) = G_SELECT %11:_(s1), %13:_, %15:_ + ; CHECK: %8:_(s32) = G_FCONSTANT float 1.000000e+00 + ; CHECK: %9:_(<4 x s32>) = G_BUILD_VECTOR %8(s32), %8(s32), %8(s32), %8(s32) + ; CHECK: %14:id(<4 x s32>) = G_INTRINSIC intrinsic(@llvm.spv.faceforward), %9(<4 x s32>), %4(<4 x s32>), %5(<4 x s32>) + ; CHECK: OpReturnValue %14(<4 x s32>) + %4:type(s64) = OpTypeVector %3:type(s64), 4 + %6:type(s64) = OpTypeFunction %4:type(s64), %4:type(s64), %4:type(s64), %4:type(s64) + %3:type(s64) = OpTypeFloat 32 + OpName %0:vfid(<4 x s32>), 97 + OpName %1:vfid(<4 x s32>), 98 + OpName %2:vfid(<4 x s32>), 99 + %5:iid(s64) = OpFunction %4:type(s64), 0, %6:type(s64) + %0:vfid(<4 x s32>) = OpFunctionParameter %4:type(s64) + %1:vfid(<4 x s32>) = OpFunctionParameter %4:type(s64) + %2:vfid(<4 x s32>) = OpFunctionParameter %4:type(s64) + OpName %5:iid(s64), 1701011814, 2003988326, 1600418401, 1953721961, 1651339107, 1600482921, 1634692198, 1667183732, 1953721967, 1937010273, 0 + %10:_(s32) = G_FCONSTANT float 0.000000e+00 + %14:_(s32) = G_FCONSTANT float 1.000000e+00 + %13:_(<4 x s32>) = G_BUILD_VECTOR %14:_(s32), %14:_(s32), %14:_(s32), %14:_(s32) + %16:_(s32) = G_FCONSTANT float -1.000000e+00 + %15:_(<4 x s32>) = G_BUILD_VECTOR %16:_(s32), %16:_(s32), %16:_(s32), %16:_(s32) + %9:_(s32) = G_INTRINSIC intrinsic(@llvm.spv.fdot), %1:vfid(<4 x s32>), %2:vfid(<4 x s32>) + %11:_(s1) = G_FCMP floatpred(olt), %9:_(s32), %10:_ + %12:id(<4 x s32>) = G_SELECT %11:_(s1), %13:_, %15:_ + OpReturnValue %12:id(<4 x s32>) diff --git a/llvm/test/CodeGen/SPIRV/hlsl-intrinsics/faceforward.ll b/llvm/test/CodeGen/SPIRV/hlsl-intrinsics/faceforward.ll index 6811f95bbab17..d70fa329ddecd 100644 --- a/llvm/test/CodeGen/SPIRV/hlsl-intrinsics/faceforward.ll +++ b/llvm/test/CodeGen/SPIRV/hlsl-intrinsics/faceforward.ll @@ -81,6 +81,32 @@ entry: ret <4 x float> %select } +define internal noundef float @faceforward_instcombine_float_constants(float noundef %a, float noundef %b, float noundef %c) { +entry: + ; CHECK: %[[#]] = OpFunction %[[#float_32]] None %[[#]] + ; CHECK: %[[#arg0:]] = OpFunctionParameter %[[#float_32]] + ; CHECK: %[[#arg1:]] = OpFunctionParameter %[[#float_32]] + ; CHECK: %[[#arg2:]] = OpFunctionParameter %[[#float_32]] + ; CHECK: %[[#]] = OpExtInst %[[#float_32]] %[[#op_ext_glsl]] FaceForward %[[#]] %[[#arg1]] %[[#arg2]] + %fmul = fmul float %b, %c + %fcmp = fcmp olt float %fmul, 0.000000e+00 + %select = select i1 %fcmp, float 1.000000e+00, float -1.000000e+00 + ret float %select +} + +define internal noundef <4 x float> @faceforward_instcombine_float4_constants(<4 x float> noundef %a, <4 x float> noundef %b, <4 x float> noundef %c) { +entry: + ; CHECK: %[[#]] = OpFunction %[[#vec4_float_32]] None %[[#]] + ; CHECK: %[[#arg0:]] = OpFunctionParameter %[[#vec4_float_32]] + ; CHECK: %[[#arg1:]] = OpFunctionParameter %[[#vec4_float_32]] + ; CHECK: %[[#arg2:]] = OpFunctionParameter %[[#vec4_float_32]] + ; CHECK: %[[#]] = OpExtInst %[[#vec4_float_32]] %[[#op_ext_glsl]] FaceForward %[[#]] %[[#arg1]] %[[#arg2]] + %spv.fdot = call float @llvm.spv.fdot.v4f32(<4 x float> %b, <4 x float> %c) + %fcmp = fcmp olt float %spv.fdot, 0.000000e+00 + %select = select i1 %fcmp, <4 x float> , <4 x float> + ret <4 x float> %select +} + ; The other fucntions are the test, but a entry point is required to have a valid SPIR-V module. define void @main() #1 { entry: From 900b7bb2c1561ef1b0f5e29adb9acd045842426f Mon Sep 17 00:00:00 2001 From: kmpeng Date: Wed, 24 Sep 2025 15:07:32 -0700 Subject: [PATCH 8/9] edit true/false operand matching to work more robustly with constant vectors --- llvm/lib/Target/SPIRV/SPIRVCombinerHelper.cpp | 31 +++++++++++++------ .../SPIRV/hlsl-intrinsics/faceforward.ll | 2 +- 2 files changed, 23 insertions(+), 10 deletions(-) diff --git a/llvm/lib/Target/SPIRV/SPIRVCombinerHelper.cpp b/llvm/lib/Target/SPIRV/SPIRVCombinerHelper.cpp index 9e4a7ee49e611..a807677de8773 100644 --- a/llvm/lib/Target/SPIRV/SPIRVCombinerHelper.cpp +++ b/llvm/lib/Target/SPIRV/SPIRVCombinerHelper.cpp @@ -105,8 +105,8 @@ bool SPIRVCombinerHelper::matchSelectToFaceForward(MachineInstr &MI) const { // Check if FCMP is a comparison between a dot product and 0. MachineInstr *DotInstr = MRI.getVRegDef(DotReg); - if ((DotInstr->getOpcode() != TargetOpcode::G_INTRINSIC || - cast(DotInstr)->getIntrinsicID() != Intrinsic::spv_fdot)) { + if (DotInstr->getOpcode() != TargetOpcode::G_INTRINSIC || + cast(DotInstr)->getIntrinsicID() != Intrinsic::spv_fdot) { Register DotOperand1, DotOperand2; // Check for scalar dot product. if (!mi_match(DotReg, MRI, @@ -121,16 +121,29 @@ bool SPIRVCombinerHelper::matchSelectToFaceForward(MachineInstr &MI) const { return false; // Check if select's false operand is the negation of the true operand. + auto AreNegatedConstants = [&](Register TrueReg, Register FalseReg) { + const ConstantFP *TrueVal, *FalseVal; + if (!mi_match(TrueReg, MRI, m_GFCst(TrueVal)) || + !mi_match(FalseReg, MRI, m_GFCst(FalseVal))) + return false; + APFloat TrueValNegated = TrueVal->getValue(); + TrueValNegated.changeSign(); + return FalseVal->getValue().compare(TrueValNegated) == APFloat::cmpEqual; + }; + if (!mi_match(FalseReg, MRI, m_GFNeg(m_SpecificReg(TrueReg))) && !mi_match(TrueReg, MRI, m_GFNeg(m_SpecificReg(FalseReg)))) { // Check if they're constant opposites. - std::optional TrueVal, FalseVal; - if (!mi_match(TrueReg, MRI, m_GFCstOrSplat(TrueVal)) || - !mi_match(FalseReg, MRI, m_GFCstOrSplat(FalseVal))) - return false; - APFloat TrueValNegated = TrueVal->Value; - TrueValNegated.changeSign(); - if (FalseVal->Value.compare(TrueValNegated) != APFloat::cmpEqual) + MachineInstr *TrueInstr = MRI.getVRegDef(TrueReg); + MachineInstr *FalseInstr = MRI.getVRegDef(FalseReg); + if (TrueInstr->getOpcode() == TargetOpcode::G_BUILD_VECTOR && + FalseInstr->getOpcode() == TargetOpcode::G_BUILD_VECTOR && + TrueInstr->getNumOperands() == FalseInstr->getNumOperands()) { + for (unsigned I = 1; I < TrueInstr->getNumOperands(); ++I) + if (!AreNegatedConstants(TrueInstr->getOperand(I).getReg(), + FalseInstr->getOperand(I).getReg())) + return false; + } else if (!AreNegatedConstants(TrueReg, FalseReg)) return false; } diff --git a/llvm/test/CodeGen/SPIRV/hlsl-intrinsics/faceforward.ll b/llvm/test/CodeGen/SPIRV/hlsl-intrinsics/faceforward.ll index d70fa329ddecd..ef6057f52c413 100644 --- a/llvm/test/CodeGen/SPIRV/hlsl-intrinsics/faceforward.ll +++ b/llvm/test/CodeGen/SPIRV/hlsl-intrinsics/faceforward.ll @@ -103,7 +103,7 @@ entry: ; CHECK: %[[#]] = OpExtInst %[[#vec4_float_32]] %[[#op_ext_glsl]] FaceForward %[[#]] %[[#arg1]] %[[#arg2]] %spv.fdot = call float @llvm.spv.fdot.v4f32(<4 x float> %b, <4 x float> %c) %fcmp = fcmp olt float %spv.fdot, 0.000000e+00 - %select = select i1 %fcmp, <4 x float> , <4 x float> + %select = select i1 %fcmp, <4 x float> , <4 x float> ret <4 x float> %select } From 04528a684269ef7e4fc8b5bd9c06f8406578cbac Mon Sep 17 00:00:00 2001 From: kmpeng Date: Mon, 13 Oct 2025 11:59:05 -0700 Subject: [PATCH 9/9] change vulkan target check, use MIRBuilder --- llvm/lib/Target/SPIRV/SPIRVCombinerHelper.cpp | 45 ++++--------------- 1 file changed, 8 insertions(+), 37 deletions(-) diff --git a/llvm/lib/Target/SPIRV/SPIRVCombinerHelper.cpp b/llvm/lib/Target/SPIRV/SPIRVCombinerHelper.cpp index a807677de8773..3315c7b2b7bc3 100644 --- a/llvm/lib/Target/SPIRV/SPIRVCombinerHelper.cpp +++ b/llvm/lib/Target/SPIRV/SPIRVCombinerHelper.cpp @@ -21,18 +21,6 @@ SPIRVCombinerHelper::SPIRVCombinerHelper( const SPIRVSubtarget &STI) : CombinerHelper(Observer, B, IsPreLegalize, VT, MDT, LI), STI(STI) {} -static void removeAllUses(Register Reg, MachineRegisterInfo &MRI, - SPIRVGlobalRegistry *GR) { - SmallVector UsesToErase( - llvm::make_pointer_range(MRI.use_instructions(Reg))); - - // calling eraseFromParent too early invalidates the iterator. - for (auto *MIToErase : UsesToErase) { - GR->invalidateMachineInstr(MIToErase); - MIToErase->eraseFromParent(); - } -} - /// This match is part of a combine that /// rewrites length(X - Y) to distance(X, Y) /// (f32 (g_intrinsic length @@ -86,7 +74,7 @@ void SPIRVCombinerHelper::applySPIRVDistance(MachineInstr &MI) const { /// This only works for Vulkan targets. /// bool SPIRVCombinerHelper::matchSelectToFaceForward(MachineInstr &MI) const { - if (!MI.getMF()->getSubtarget().isShader()) + if (!STI.isShader()) return false; // Match overall select pattern. @@ -164,29 +152,12 @@ void SPIRVCombinerHelper::applySPIRVFaceForward(MachineInstr &MI) const { DotOperand2 = DotInstr->getOperand(3).getReg(); } - // Remove the original `select` instruction. Register ResultReg = MI.getOperand(0).getReg(); - DebugLoc DL = MI.getDebugLoc(); - MachineBasicBlock &MBB = *MI.getParent(); - MachineBasicBlock::iterator InsertPt = MI.getIterator(); - - // Build the `spv_faceforward` intrinsic. - MachineInstrBuilder NewInstr = BuildMI( - MBB, InsertPt, DL, Builder.getTII().get(TargetOpcode::G_INTRINSIC)); - NewInstr - .addDef(ResultReg) // Result register - .addIntrinsicID(Intrinsic::spv_faceforward) // Intrinsic ID - .addUse(TrueReg) // Operand N - .addUse(DotOperand1) // Operand I - .addUse(DotOperand2); // Operand Ng - - SPIRVGlobalRegistry *GR = - MI.getMF()->getSubtarget().getSPIRVGlobalRegistry(); - removeAllUses(CondReg, MRI, GR); // Remove all uses of FCMP result - MachineInstr *CondInstr = MRI.getVRegDef(CondReg); - GR->invalidateMachineInstr(CondInstr); - CondInstr->eraseFromParent(); // Remove FCMP instruction - removeAllUses(DotReg, MRI, GR); // Remove all uses of dot product result - GR->invalidateMachineInstr(DotInstr); - DotInstr->eraseFromParent(); // Remove dot product instruction + Builder.setInstrAndDebugLoc(MI); + Builder.buildIntrinsic(Intrinsic::spv_faceforward, ResultReg) + .addUse(TrueReg) // N + .addUse(DotOperand1) // I + .addUse(DotOperand2); // Ng + + MI.eraseFromParent(); }