[llvm] [SPIRV] Audit `select` Result in SPIRVInstructionSelector (PR #115193)
via llvm-commits
llvm-commits at lists.llvm.org
Wed Nov 6 11:04:56 PST 2024
llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-backend-spir-v
Author: Finn Plummer (inbelic)
<details>
<summary>Changes</summary>
- as per the definition of `select` in GlobalISel/InstructionSelector.h the return value is a boolean denoting if the select was successful
- doing `Result |=` is incorrect as all inserted instructions should be succesful, hence we change to using `Result &=`
---
Full diff: https://github.com/llvm/llvm-project/pull/115193.diff
1 Files Affected:
- (modified) llvm/lib/Target/SPIRV/SPIRVInstructionSelector.cpp (+13-13)
``````````diff
diff --git a/llvm/lib/Target/SPIRV/SPIRVInstructionSelector.cpp b/llvm/lib/Target/SPIRV/SPIRVInstructionSelector.cpp
index be38b22f70c583..f5b9ca939ba898 100644
--- a/llvm/lib/Target/SPIRV/SPIRVInstructionSelector.cpp
+++ b/llvm/lib/Target/SPIRV/SPIRVInstructionSelector.cpp
@@ -1082,16 +1082,16 @@ bool SPIRVInstructionSelector::selectAtomicRMW(Register ResVReg,
uint32_t MemSem = static_cast<uint32_t>(getMemSemantics(AO));
Register MemSemReg = buildI32Constant(MemSem /*| ScSem*/, I);
- bool Result = false;
+ bool Result = true;
Register ValueReg = I.getOperand(2).getReg();
if (NegateOpcode != 0) {
// Translation with negative value operand is requested
Register TmpReg = MRI->createVirtualRegister(&SPIRV::iIDRegClass);
- Result |= selectUnOpWithSrc(TmpReg, ResType, I, ValueReg, NegateOpcode);
+ Result &= selectUnOpWithSrc(TmpReg, ResType, I, ValueReg, NegateOpcode);
ValueReg = TmpReg;
}
- Result |= BuildMI(*I.getParent(), I, I.getDebugLoc(), TII.get(NewOpcode))
+ Result &= BuildMI(*I.getParent(), I, I.getDebugLoc(), TII.get(NewOpcode))
.addDef(ResVReg)
.addUse(GR.getSPIRVTypeID(ResType))
.addUse(Ptr)
@@ -1264,21 +1264,21 @@ bool SPIRVInstructionSelector::selectAtomicCmpXchg(Register ResVReg,
.constrainAllUses(TII, TRI, RBI);
Register CmpSuccReg = MRI->createVirtualRegister(&SPIRV::iIDRegClass);
SPIRVType *BoolTy = GR.getOrCreateSPIRVBoolType(I, TII);
- Result |= BuildMI(*I.getParent(), I, DL, TII.get(SPIRV::OpIEqual))
+ Result &= BuildMI(*I.getParent(), I, DL, TII.get(SPIRV::OpIEqual))
.addDef(CmpSuccReg)
.addUse(GR.getSPIRVTypeID(BoolTy))
.addUse(ACmpRes)
.addUse(Cmp)
.constrainAllUses(TII, TRI, RBI);
Register TmpReg = MRI->createVirtualRegister(&SPIRV::iIDRegClass);
- Result |= BuildMI(*I.getParent(), I, DL, TII.get(SPIRV::OpCompositeInsert))
+ Result &= BuildMI(*I.getParent(), I, DL, TII.get(SPIRV::OpCompositeInsert))
.addDef(TmpReg)
.addUse(GR.getSPIRVTypeID(ResType))
.addUse(ACmpRes)
.addUse(GR.getOrCreateUndef(I, ResType, TII))
.addImm(0)
.constrainAllUses(TII, TRI, RBI);
- Result |= BuildMI(*I.getParent(), I, DL, TII.get(SPIRV::OpCompositeInsert))
+ Result &= BuildMI(*I.getParent(), I, DL, TII.get(SPIRV::OpCompositeInsert))
.addDef(ResVReg)
.addUse(GR.getSPIRVTypeID(ResType))
.addUse(CmpSuccReg)
@@ -1784,7 +1784,7 @@ bool SPIRVInstructionSelector::selectDot4AddPackedExpansion(
assert(I.getOperand(4).isReg());
MachineBasicBlock &BB = *I.getParent();
- bool Result = false;
+ bool Result = true;
// Acc = C
Register Acc = I.getOperand(4).getReg();
@@ -1796,7 +1796,7 @@ bool SPIRVInstructionSelector::selectDot4AddPackedExpansion(
for (unsigned i = 0; i < 4; i++) {
// A[i]
Register AElt = MRI->createVirtualRegister(&SPIRV::IDRegClass);
- Result |= BuildMI(BB, I, I.getDebugLoc(), TII.get(ExtractOp))
+ Result &= BuildMI(BB, I, I.getDebugLoc(), TII.get(ExtractOp))
.addDef(AElt)
.addUse(GR.getSPIRVTypeID(ResType))
.addUse(I.getOperand(2).getReg())
@@ -1806,7 +1806,7 @@ bool SPIRVInstructionSelector::selectDot4AddPackedExpansion(
// B[i]
Register BElt = MRI->createVirtualRegister(&SPIRV::IDRegClass);
- Result |= BuildMI(BB, I, I.getDebugLoc(), TII.get(ExtractOp))
+ Result &= BuildMI(BB, I, I.getDebugLoc(), TII.get(ExtractOp))
.addDef(BElt)
.addUse(GR.getSPIRVTypeID(ResType))
.addUse(I.getOperand(3).getReg())
@@ -1816,7 +1816,7 @@ bool SPIRVInstructionSelector::selectDot4AddPackedExpansion(
// A[i] * B[i]
Register Mul = MRI->createVirtualRegister(&SPIRV::IDRegClass);
- Result |= BuildMI(BB, I, I.getDebugLoc(), TII.get(SPIRV::OpIMulS))
+ Result &= BuildMI(BB, I, I.getDebugLoc(), TII.get(SPIRV::OpIMulS))
.addDef(Mul)
.addUse(GR.getSPIRVTypeID(ResType))
.addUse(AElt)
@@ -1825,7 +1825,7 @@ bool SPIRVInstructionSelector::selectDot4AddPackedExpansion(
// Discard 24 highest-bits so that stored i32 register is i8 equivalent
Register MaskMul = MRI->createVirtualRegister(&SPIRV::IDRegClass);
- Result |= BuildMI(BB, I, I.getDebugLoc(), TII.get(ExtractOp))
+ Result &= BuildMI(BB, I, I.getDebugLoc(), TII.get(ExtractOp))
.addDef(MaskMul)
.addUse(GR.getSPIRVTypeID(ResType))
.addUse(Mul)
@@ -1836,7 +1836,7 @@ bool SPIRVInstructionSelector::selectDot4AddPackedExpansion(
// Acc = Acc + A[i] * B[i]
Register Sum =
i < 3 ? MRI->createVirtualRegister(&SPIRV::IDRegClass) : ResVReg;
- Result |= BuildMI(BB, I, I.getDebugLoc(), TII.get(SPIRV::OpIAddS))
+ Result &= BuildMI(BB, I, I.getDebugLoc(), TII.get(SPIRV::OpIAddS))
.addDef(Sum)
.addUse(GR.getSPIRVTypeID(ResType))
.addUse(Acc)
@@ -1907,7 +1907,7 @@ bool SPIRVInstructionSelector::selectSign(Register ResVReg,
if (NeedsConversion) {
auto ConvertOpcode = IsFloatTy ? SPIRV::OpConvertFToS : SPIRV::OpSConvert;
- Result |= BuildMI(*I.getParent(), I, DL, TII.get(ConvertOpcode))
+ Result &= BuildMI(*I.getParent(), I, DL, TII.get(ConvertOpcode))
.addDef(ResVReg)
.addUse(GR.getSPIRVTypeID(ResType))
.addUse(SignReg)
``````````
</details>
https://github.com/llvm/llvm-project/pull/115193
More information about the llvm-commits
mailing list