Skip to content

Commit 1ca8ad2

Browse files
farzonlfarzonl
authored
[SPIRV] Create a new OpSelect selector and fix register types. (#152311)
fixes #135572 There are two problems that are causing problems first register types are copied from older registers instead of evaluating the spirv types. Second the way OpSelect is defined in SPIRVInstrInfo.td we always default to integer for TernOpTyped. There seems to be a problem of multiple matches in the getMatchTable so when executeMatchTable runs we aren't getting the right opSelect. Correcting the tablegen wasn't very easy so instead created an emitter for Select that evaluated the register types. this passes the original llvm/test/CodeGen/SPIRV/instructions/select.ll tests and the new float ones I'm adding in issue-135572-emit-float-opselect.ll
1 parent 116c318 commit 1ca8ad2

File tree

3 files changed

+121
-14
lines changed

3 files changed

+121
-14
lines changed

‎llvm/lib/Target/SPIRV/SPIRVInstructionSelector.cpp

Lines changed: 64 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -220,8 +220,10 @@ class SPIRVInstructionSelector : public InstructionSelector {
220220
bool selectConst(Register ResVReg, const SPIRVType *ResType,
221221
MachineInstr &I) const;
222222

223-
bool selectSelect(Register ResVReg, const SPIRVType *ResType, MachineInstr &I,
224-
bool IsSigned) const;
223+
bool selectSelect(Register ResVReg, const SPIRVType *ResType,
224+
MachineInstr &I) const;
225+
bool selectSelectDefaultArgs(Register ResVReg, const SPIRVType *ResType,
226+
MachineInstr &I, bool IsSigned) const;
225227
bool selectIToF(Register ResVReg, const SPIRVType *ResType, MachineInstr &I,
226228
bool IsSigned, unsigned Opcode) const;
227229
bool selectExt(Register ResVReg, const SPIRVType *ResType, MachineInstr &I,
@@ -510,7 +512,18 @@ bool SPIRVInstructionSelector::select(MachineInstr &I) {
510512
if (isTypeFoldingSupported(Def->getOpcode()) &&
511513
Def->getOpcode() != TargetOpcode::G_CONSTANT &&
512514
Def->getOpcode() != TargetOpcode::G_FCONSTANT) {
513-
bool Res = selectImpl(I, *CoverageInfo);
515+
bool Res = false;
516+
if (Def->getOpcode() == TargetOpcode::G_SELECT) {
517+
Register SelectDstReg = Def->getOperand(0).getReg();
518+
Res = selectSelect(SelectDstReg, GR.getSPIRVTypeForVReg(SelectDstReg),
519+
*Def);
520+
GR.invalidateMachineInstr(Def);
521+
Def->removeFromParent();
522+
MRI->replaceRegWith(DstReg, SelectDstReg);
523+
GR.invalidateMachineInstr(&I);
524+
I.removeFromParent();
525+
} else
526+
Res = selectImpl(I, *CoverageInfo);
514527
LLVM_DEBUG({
515528
if (!Res && Def->getOpcode() != TargetOpcode::G_CONSTANT) {
516529
dbgs() << "Unexpected pattern in ASSIGN_TYPE.\nInstruction: ";
@@ -2565,8 +2578,52 @@ Register SPIRVInstructionSelector::buildOnesVal(bool AllOnes,
25652578

25662579
bool SPIRVInstructionSelector::selectSelect(Register ResVReg,
25672580
const SPIRVType *ResType,
2568-
MachineInstr &I,
2569-
bool IsSigned) const {
2581+
MachineInstr &I) const {
2582+
Register SelectFirstArg = I.getOperand(2).getReg();
2583+
Register SelectSecondArg = I.getOperand(3).getReg();
2584+
assert(ResType == GR.getSPIRVTypeForVReg(SelectFirstArg) &&
2585+
ResType == GR.getSPIRVTypeForVReg(SelectSecondArg));
2586+
2587+
bool IsFloatTy =
2588+
GR.isScalarOrVectorOfType(SelectFirstArg, SPIRV::OpTypeFloat);
2589+
bool IsPtrTy =
2590+
GR.isScalarOrVectorOfType(SelectFirstArg, SPIRV::OpTypePointer);
2591+
bool IsVectorTy = GR.getSPIRVTypeForVReg(SelectFirstArg)->getOpcode() ==
2592+
SPIRV::OpTypeVector;
2593+
2594+
bool IsScalarBool =
2595+
GR.isScalarOfType(I.getOperand(1).getReg(), SPIRV::OpTypeBool);
2596+
unsigned Opcode;
2597+
if (IsVectorTy) {
2598+
if (IsFloatTy) {
2599+
Opcode = IsScalarBool ? SPIRV::OpSelectVFSCond : SPIRV::OpSelectVFVCond;
2600+
} else if (IsPtrTy) {
2601+
Opcode = IsScalarBool ? SPIRV::OpSelectVPSCond : SPIRV::OpSelectVPVCond;
2602+
} else {
2603+
Opcode = IsScalarBool ? SPIRV::OpSelectVISCond : SPIRV::OpSelectVIVCond;
2604+
}
2605+
} else {
2606+
if (IsFloatTy) {
2607+
Opcode = IsScalarBool ? SPIRV::OpSelectSFSCond : SPIRV::OpSelectVFVCond;
2608+
} else if (IsPtrTy) {
2609+
Opcode = IsScalarBool ? SPIRV::OpSelectSPSCond : SPIRV::OpSelectVPVCond;
2610+
} else {
2611+
Opcode = IsScalarBool ? SPIRV::OpSelectSISCond : SPIRV::OpSelectVIVCond;
2612+
}
2613+
}
2614+
return BuildMI(*I.getParent(), I, I.getDebugLoc(), TII.get(Opcode))
2615+
.addDef(ResVReg)
2616+
.addUse(GR.getSPIRVTypeID(ResType))
2617+
.addUse(I.getOperand(1).getReg())
2618+
.addUse(SelectFirstArg)
2619+
.addUse(SelectSecondArg)
2620+
.constrainAllUses(TII, TRI, RBI);
2621+
}
2622+
2623+
bool SPIRVInstructionSelector::selectSelectDefaultArgs(Register ResVReg,
2624+
const SPIRVType *ResType,
2625+
MachineInstr &I,
2626+
bool IsSigned) const {
25702627
// To extend a bool, we need to use OpSelect between constants.
25712628
Register ZeroReg = buildZerosVal(ResType, I);
25722629
Register OneReg = buildOnesVal(IsSigned, ResType, I);
@@ -2598,7 +2655,7 @@ bool SPIRVInstructionSelector::selectIToF(Register ResVReg,
25982655
TmpType = GR.getOrCreateSPIRVVectorType(TmpType, NumElts, I, TII);
25992656
}
26002657
SrcReg = createVirtualRegister(TmpType, &GR, MRI, MRI->getMF());
2601-
selectSelect(SrcReg, TmpType, I, false);
2658+
selectSelectDefaultArgs(SrcReg, TmpType, I, false);
26022659
}
26032660
return selectOpWithSrcs(ResVReg, ResType, I, {SrcReg}, Opcode);
26042661
}
@@ -2608,7 +2665,7 @@ bool SPIRVInstructionSelector::selectExt(Register ResVReg,
26082665
MachineInstr &I, bool IsSigned) const {
26092666
Register SrcReg = I.getOperand(1).getReg();
26102667
if (GR.isScalarOrVectorOfType(SrcReg, SPIRV::OpTypeBool))
2611-
return selectSelect(ResVReg, ResType, I, IsSigned);
2668+
return selectSelectDefaultArgs(ResVReg, ResType, I, IsSigned);
26122669

26132670
SPIRVType *SrcType = GR.getSPIRVTypeForVReg(SrcReg);
26142671
if (SrcType == ResType)

‎llvm/lib/Target/SPIRV/SPIRVPreLegalizer.cpp

Lines changed: 4 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -441,13 +441,10 @@ void insertAssignInstr(Register Reg, Type *Ty, SPIRVType *SpvType,
441441
// Tablegen definition assumes SPIRV::ASSIGN_TYPE pseudo-instruction is
442442
// present after each auto-folded instruction to take a type reference from.
443443
Register NewReg = MRI.createGenericVirtualRegister(MRI.getType(Reg));
444-
if (auto *RC = MRI.getRegClassOrNull(Reg)) {
445-
MRI.setRegClass(NewReg, RC);
446-
} else {
447-
auto RegClass = GR->getRegClass(SpvType);
448-
MRI.setRegClass(NewReg, RegClass);
449-
MRI.setRegClass(Reg, RegClass);
450-
}
444+
const auto *RegClass = GR->getRegClass(SpvType);
445+
MRI.setRegClass(NewReg, RegClass);
446+
MRI.setRegClass(Reg, RegClass);
447+
451448
GR->assignSPIRVTypeToVReg(SpvType, Reg, MIB.getMF());
452449
// This is to make it convenient for Legalizer to get the SPIRVType
453450
// when processing the actual MI (i.e. not pseudo one).

‎llvm/test/CodeGen/SPIRV/instructions/issue-135572-emit-float-opselect.ll

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,53 @@
1+
; RUN: llc -verify-machineinstrs -O0 -mtriple=spirv-unknown-unknown %s -o - | FileCheck %s
2+
; RUN: llc -verify-machineinstrs -O0 -mtriple=spirv64-unknown-unknown %s -o - | FileCheck %s
3+
; RUN: llc -verify-machineinstrs -O0 -mtriple=spirv32-unknown-unknown %s -o - | FileCheck %s
4+
; RUN: %if spirv-tools %{ llc -O0 -mtriple=spirv64-unknown-unknown %s -o - -filetype=obj | spirv-val --target-env spv1.6 %}
5+
; RUN: %if spirv-tools %{ llc -O0 -mtriple=spirv32-unknown-unknown %s -o - -filetype=obj | spirv-val --target-env spv1.6 %}
6+
7+
; CHECK-DAG: %[[#float_32:]] = OpTypeFloat 32
8+
; CHECK-DAG: %[[#func_type:]] = OpTypeFunction %[[#float_32]] %[[#float_32]] %[[#float_32]]
9+
; CHECK-DAG: %[[#bool:]] = OpTypeBool
10+
; CHECK-DAG: %[[#vec4_float_32:]] = OpTypeVector %[[#float_32]] 4
11+
; CHECK-DAG: %[[#vec_func_type:]] = OpTypeFunction %[[#vec4_float_32]] %[[#vec4_float_32]] %[[#vec4_float_32]]
12+
; CHECK-DAG: %[[#vec_4_bool:]] = OpTypeVector %[[#bool]] 4
13+
14+
define spir_func float @opselect_float_scalar_test(float %x, float %y) {
15+
entry:
16+
; CHECK: %[[#]] = OpFunction %[[#float_32]] None %[[#func_type]]
17+
; CHECK: %[[#arg0:]] = OpFunctionParameter %[[#float_32]]
18+
; CHECK: %[[#arg1:]] = OpFunctionParameter %[[#float_32]]
19+
; CHECK: %[[#fcmp:]] = OpFOrdGreaterThan %[[#bool]] %[[#arg0]] %[[#arg1]]
20+
; CHECK: %[[#fselect:]] = OpSelect %[[#float_32]] %[[#fcmp]] %[[#arg0]] %[[#arg1]]
21+
; CHECK: OpReturnValue %[[#fselect]]
22+
%0 = fcmp ogt float %x, %y
23+
%1 = select i1 %0, float %x, float %y
24+
ret float %1
25+
}
26+
27+
define spir_func <4 x float> @opselect_float4_vec_test(<4 x float> %x, <4 x float> %y) {
28+
entry:
29+
; CHECK: %[[#]] = OpFunction %[[#vec4_float_32]] None %[[#vec_func_type]]
30+
; CHECK: %[[#arg0:]] = OpFunctionParameter %[[#vec4_float_32]]
31+
; CHECK: %[[#arg1:]] = OpFunctionParameter %[[#vec4_float_32]]
32+
; CHECK: %[[#fcmp:]] = OpFOrdGreaterThan %[[#vec_4_bool]] %[[#arg0]] %[[#arg1]]
33+
; CHECK: %[[#fselect:]] = OpSelect %[[#vec4_float_32]] %[[#fcmp]] %[[#arg0]] %[[#arg1]]
34+
; CHECK: OpReturnValue %[[#fselect]]
35+
%0 = fcmp ogt <4 x float> %x, %y
36+
%1 = select <4 x i1> %0, <4 x float> %x, <4 x float> %y
37+
ret <4 x float> %1
38+
}
39+
40+
define spir_func <4 x float> @opselect_scalar_bool_float4_vec_test(float %a, float %b, <4 x float> %x, <4 x float> %y) {
41+
entry:
42+
; CHECK: %[[#]] = OpFunction %[[#vec4_float_32]] None %[[#]]
43+
; CHECK: %[[#arg0:]] = OpFunctionParameter %[[#float_32]]
44+
; CHECK: %[[#arg1:]] = OpFunctionParameter %[[#float_32]]
45+
; CHECK: %[[#arg2:]] = OpFunctionParameter %[[#vec4_float_32]]
46+
; CHECK: %[[#arg3:]] = OpFunctionParameter %[[#vec4_float_32]]
47+
; CHECK: %[[#fcmp:]] = OpFOrdGreaterThan %[[#bool]] %[[#arg0]] %[[#arg1]]
48+
; CHECK: %[[#fselect:]] = OpSelect %[[#vec4_float_32]] %[[#fcmp]] %[[#arg2]] %[[#arg3]]
49+
; CHECK: OpReturnValue %[[#fselect]]
50+
%0 = fcmp ogt float %a, %b
51+
%1 = select i1 %0, <4 x float> %x, <4 x float> %y
52+
ret <4 x float> %1
53+
}

0 commit comments

Comments
 (0)