[llvm] [SPIRV] Audit `select` Result in SPIRVInstructionSelector (PR #115193)
Finn Plummer via llvm-commits
llvm-commits at lists.llvm.org
Tue Nov 12 10:28:14 PST 2024
================
@@ -1229,17 +1244,26 @@ bool SPIRVInstructionSelector::selectAtomicCmpXchg(Register ResVReg,
const MachineMemOperand *MemOp = *I.memoperands_begin();
unsigned Scope = static_cast<uint32_t>(getMemScope(
GR.CurMF->getFunction().getContext(), MemOp->getSyncScopeID()));
- ScopeReg = buildI32Constant(Scope, I);
+ auto ScopeConstant = buildI32Constant(Scope, I);
+ ScopeReg = ScopeConstant.first;
+ Result &= ScopeConstant.second;
unsigned ScSem = static_cast<uint32_t>(
getMemSemanticsForStorageClass(GR.getPointerStorageClass(Ptr)));
AtomicOrdering AO = MemOp->getSuccessOrdering();
unsigned MemSemEq = static_cast<uint32_t>(getMemSemantics(AO)) | ScSem;
- MemSemEqReg = buildI32Constant(MemSemEq, I);
+ auto MemSemEqConstant = buildI32Constant(MemSemEq, I);
+ MemSemEqReg = MemSemEqConstant.first;
+ Result &= MemSemEqConstant.second;
AtomicOrdering FO = MemOp->getFailureOrdering();
unsigned MemSemNeq = static_cast<uint32_t>(getMemSemantics(FO)) | ScSem;
- MemSemNeqReg =
- MemSemEq == MemSemNeq ? MemSemEqReg : buildI32Constant(MemSemNeq, I);
+ if (MemSemEq == MemSemNeq)
+ MemSemNeqReg = MemSemEqReg;
+ else {
+ auto MemSemNeqConstant = buildI32Constant(MemSemEq, I);
+ MemSemNeqReg = MemSemNeqConstant.first;
+ Result &= MemSemNeqConstant.second;
----------------
inbelic wrote:
I agree it would be more elegant.
But from scanning the file, there doesn't seem to be a consistent precedent to checking a register's validity before use. In particular the returns of similar functions `buildZerosVal` and `BuildOnesVal`. I think to differentiate from those functions, and to ensure the result is checked in future calls we should explicitly return the type.
This might be a good reason to also audit the use of returned Registers, which I think is out of scope for this pr. Then we could switch to returning an invalid Register.
So I will merge as-is.
https://github.com/llvm/llvm-project/pull/115193
More information about the llvm-commits
mailing list