Skip to content

Commit 5810f15

Browse files
[SPIR-V] Fix SPIRVEmitIntrinsics undefined behavior (#123625)
Before this change InstrSet in SPIRVEmitIntrinsics was uninitialized before running runOnFunction. This change adds a new function getPreferredInstructionSet in SPIRVSubtarget.
1 parent 7084110 commit 5810f15

File tree

4 files changed

+18
-18
lines changed

4 files changed

+18
-18
lines changed

llvm/lib/Target/SPIRV/SPIRVCallLowering.cpp

Lines changed: 3 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -544,13 +544,7 @@ bool SPIRVCallLowering::lowerCall(MachineIRBuilder &MIRBuilder,
544544
const auto *ST = static_cast<const SPIRVSubtarget *>(&MF.getSubtarget());
545545

546546
bool isFunctionDecl = CF && CF->isDeclaration();
547-
bool canUseOpenCL = ST->canUseExtInstSet(SPIRV::InstructionSet::OpenCL_std);
548-
bool canUseGLSL = ST->canUseExtInstSet(SPIRV::InstructionSet::GLSL_std_450);
549-
assert(canUseGLSL != canUseOpenCL &&
550-
"Scenario where both sets are enabled is not supported.");
551-
552-
if (isFunctionDecl && !DemangledName.empty() &&
553-
(canUseGLSL || canUseOpenCL)) {
547+
if (isFunctionDecl && !DemangledName.empty()) {
554548
if (ResVReg.isValid()) {
555549
if (!GR->getSPIRVTypeForVReg(ResVReg)) {
556550
const Type *RetTy = OrigRetTy;
@@ -607,11 +601,9 @@ bool SPIRVCallLowering::lowerCall(MachineIRBuilder &MIRBuilder,
607601
GR->getPointerSize()));
608602
}
609603
}
610-
auto instructionSet = canUseOpenCL ? SPIRV::InstructionSet::OpenCL_std
611-
: SPIRV::InstructionSet::GLSL_std_450;
612604
if (auto Res =
613-
SPIRV::lowerBuiltin(DemangledName, instructionSet, MIRBuilder,
614-
ResVReg, OrigRetTy, ArgVRegs, GR))
605+
SPIRV::lowerBuiltin(DemangledName, ST->getPreferredInstructionSet(),
606+
MIRBuilder, ResVReg, OrigRetTy, ArgVRegs, GR))
615607
return *Res;
616608
}
617609

llvm/lib/Target/SPIRV/SPIRVEmitIntrinsics.cpp

Lines changed: 6 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -74,7 +74,6 @@ class SPIRVEmitIntrinsics
7474
DenseMap<Instruction *, Constant *> AggrConsts;
7575
DenseMap<Instruction *, Type *> AggrConstTypes;
7676
DenseSet<Instruction *> AggrStores;
77-
SPIRV::InstructionSet::InstructionSet InstrSet;
7877

7978
// map of function declarations to <pointer arg index => element type>
8079
DenseMap<Function *, SmallVector<std::pair<unsigned, Type *>>> FDeclPtrTys;
@@ -896,8 +895,9 @@ bool SPIRVEmitIntrinsics::deduceOperandElementTypeCalledFunction(
896895
getOclOrSpirvBuiltinDemangledName(CalledF->getName());
897896
if (DemangledName.length() > 0 &&
898897
!StringRef(DemangledName).starts_with("llvm.")) {
899-
auto [Grp, Opcode, ExtNo] =
900-
SPIRV::mapBuiltinToOpcode(DemangledName, InstrSet);
898+
const SPIRVSubtarget &ST = TM->getSubtarget<SPIRVSubtarget>(*CalledF);
899+
auto [Grp, Opcode, ExtNo] = SPIRV::mapBuiltinToOpcode(
900+
DemangledName, ST.getPreferredInstructionSet());
901901
if (Opcode == SPIRV::OpGroupAsyncCopy) {
902902
for (unsigned i = 0, PtrCnt = 0; i < CI->arg_size() && PtrCnt < 2; ++i) {
903903
Value *Op = CI->getArgOperand(i);
@@ -2317,8 +2317,6 @@ bool SPIRVEmitIntrinsics::runOnFunction(Function &Func) {
23172317

23182318
const SPIRVSubtarget &ST = TM->getSubtarget<SPIRVSubtarget>(Func);
23192319
GR = ST.getSPIRVGlobalRegistry();
2320-
InstrSet = ST.isOpenCLEnv() ? SPIRV::InstructionSet::OpenCL_std
2321-
: SPIRV::InstructionSet::GLSL_std_450;
23222320

23232321
if (!CurrF)
23242322
HaveFunPtrs =
@@ -2475,8 +2473,9 @@ void SPIRVEmitIntrinsics::parseFunDeclarations(Module &M) {
24752473
if (DemangledName.empty())
24762474
continue;
24772475
// allow only OpGroupAsyncCopy use case at the moment
2478-
auto [Grp, Opcode, ExtNo] =
2479-
SPIRV::mapBuiltinToOpcode(DemangledName, InstrSet);
2476+
const SPIRVSubtarget &ST = TM->getSubtarget<SPIRVSubtarget>(F);
2477+
auto [Grp, Opcode, ExtNo] = SPIRV::mapBuiltinToOpcode(
2478+
DemangledName, ST.getPreferredInstructionSet());
24802479
if (Opcode != SPIRV::OpGroupAsyncCopy)
24812480
continue;
24822481
// find pointer arguments

llvm/lib/Target/SPIRV/SPIRVSubtarget.cpp

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -111,6 +111,14 @@ bool SPIRVSubtarget::canUseExtInstSet(
111111
return AvailableExtInstSets.contains(E);
112112
}
113113

114+
SPIRV::InstructionSet::InstructionSet
115+
SPIRVSubtarget::getPreferredInstructionSet() const {
116+
if (isOpenCLEnv())
117+
return SPIRV::InstructionSet::OpenCL_std;
118+
else
119+
return SPIRV::InstructionSet::GLSL_std_450;
120+
}
121+
114122
bool SPIRVSubtarget::isAtLeastSPIRVVer(VersionTuple VerToCompareTo) const {
115123
return isAtLeastVer(SPIRVVersion, VerToCompareTo);
116124
}

llvm/lib/Target/SPIRV/SPIRVSubtarget.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -96,6 +96,7 @@ class SPIRVSubtarget : public SPIRVGenSubtargetInfo {
9696
}
9797
bool canUseExtension(SPIRV::Extension::Extension E) const;
9898
bool canUseExtInstSet(SPIRV::InstructionSet::InstructionSet E) const;
99+
SPIRV::InstructionSet::InstructionSet getPreferredInstructionSet() const;
99100

100101
SPIRVGlobalRegistry *getSPIRVGlobalRegistry() const { return GR.get(); }
101102

0 commit comments

Comments
 (0)