Skip to content

Commit 7c917e8

Browse files
[SPIR-V] Implement correct zeroinitializer for extension types in SPIR-V Backend (#93607)
This PR implements correct zeroinitializer for extension types in SPIR-V Backend. Previous version has just created 0 of 32/64 integer type (depending on target machine word size), that caused re-use and type re-write of the corresponding integer constant 0 with a potential crash on wrong usage of the constant (i.e., 0 of integer type expected but extension type found). E.g., the following code would crash without the PR: ``` %r1 = icmp ne i64 %_arg_i, 0 %e1 = tail call spir_func target("spirv.Event") @__spirv_GroupAsyncCopy(i32 2, ptr addrspace(3) %_arg_local, ptr addrspace(1) %_arg_ptr, i64 1, i64 1, target("spirv.Event") zeroinitializer) ``` because 0 in icmp would eventually be of `Event` type.
1 parent f63adf3 commit 7c917e8

File tree

3 files changed

+44
-11
lines changed

3 files changed

+44
-11
lines changed

llvm/lib/Target/SPIRV/SPIRVEmitIntrinsics.cpp

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1212,8 +1212,7 @@ void SPIRVEmitIntrinsics::processInstrAfterVisit(Instruction *I,
12121212
}
12131213
Value *OpTyVal = Op;
12141214
if (Op->getType()->isTargetExtTy())
1215-
OpTyVal = Constant::getNullValue(
1216-
IntegerType::get(I->getContext(), GR->getPointerSize()));
1215+
OpTyVal = PoisonValue::get(Op->getType());
12171216
auto *NewOp = buildIntrWithMD(Intrinsic::spv_track_constant,
12181217
{Op->getType(), OpTyVal->getType()}, Op,
12191218
OpTyVal, {}, B);

llvm/lib/Target/SPIRV/SPIRVPreLegalizer.cpp

Lines changed: 20 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,7 @@ class SPIRVPreLegalizer : public MachineFunctionPass {
4040

4141
static void
4242
addConstantsToTrack(MachineFunction &MF, SPIRVGlobalRegistry *GR,
43+
const SPIRVSubtarget &STI,
4344
DenseMap<MachineInstr *, Type *> &TargetExtConstTypes) {
4445
MachineRegisterInfo &MRI = MF.getRegInfo();
4546
DenseMap<MachineInstr *, Register> RegsAlreadyAddedToDT;
@@ -82,8 +83,17 @@ addConstantsToTrack(MachineFunction &MF, SPIRVGlobalRegistry *GR,
8283
if (Const->getType()->isTargetExtTy()) {
8384
// remember association so that we can restore it when assign types
8485
MachineInstr *SrcMI = MRI.getVRegDef(SrcReg);
85-
if (SrcMI && SrcMI->getOpcode() == TargetOpcode::G_CONSTANT)
86+
if (SrcMI && (SrcMI->getOpcode() == TargetOpcode::G_CONSTANT ||
87+
SrcMI->getOpcode() == TargetOpcode::G_IMPLICIT_DEF))
8688
TargetExtConstTypes[SrcMI] = Const->getType();
89+
if (Const->isNullValue()) {
90+
MachineIRBuilder MIB(MF);
91+
SPIRVType *ExtType =
92+
GR->getOrCreateSPIRVType(Const->getType(), MIB);
93+
SrcMI->setDesc(STI.getInstrInfo()->get(SPIRV::OpConstantNull));
94+
SrcMI->addOperand(MachineOperand::CreateReg(
95+
GR->getSPIRVTypeID(ExtType), false));
96+
}
8797
}
8898
} else {
8999
RegsAlreadyAddedToDT[&MI] = Reg;
@@ -394,6 +404,7 @@ generateAssignInstrs(MachineFunction &MF, SPIRVGlobalRegistry *GR,
394404
for (auto MII = std::prev(MBB->end()), Begin = MBB->begin();
395405
!ReachedBegin;) {
396406
MachineInstr &MI = *MII;
407+
unsigned MIOp = MI.getOpcode();
397408

398409
if (isSpvIntrinsic(MI, Intrinsic::spv_assign_ptr_type)) {
399410
Register Reg = MI.getOperand(1).getReg();
@@ -419,9 +430,9 @@ generateAssignInstrs(MachineFunction &MF, SPIRVGlobalRegistry *GR,
419430
if (Def->getOpcode() != TargetOpcode::G_GLOBAL_VALUE)
420431
insertAssignInstr(Reg, Ty, nullptr, GR, MIB, MF.getRegInfo());
421432
ToErase.push_back(&MI);
422-
} else if (MI.getOpcode() == TargetOpcode::G_CONSTANT ||
423-
MI.getOpcode() == TargetOpcode::G_FCONSTANT ||
424-
MI.getOpcode() == TargetOpcode::G_BUILD_VECTOR) {
433+
} else if (MIOp == TargetOpcode::G_CONSTANT ||
434+
MIOp == TargetOpcode::G_FCONSTANT ||
435+
MIOp == TargetOpcode::G_BUILD_VECTOR) {
425436
// %rc = G_CONSTANT ty Val
426437
// ===>
427438
// %cty = OpType* ty
@@ -435,15 +446,15 @@ generateAssignInstrs(MachineFunction &MF, SPIRVGlobalRegistry *GR,
435446
continue;
436447
}
437448
Type *Ty = nullptr;
438-
if (MI.getOpcode() == TargetOpcode::G_CONSTANT) {
449+
if (MIOp == TargetOpcode::G_CONSTANT) {
439450
auto TargetExtIt = TargetExtConstTypes.find(&MI);
440451
Ty = TargetExtIt == TargetExtConstTypes.end()
441452
? MI.getOperand(1).getCImm()->getType()
442453
: TargetExtIt->second;
443-
} else if (MI.getOpcode() == TargetOpcode::G_FCONSTANT) {
454+
} else if (MIOp == TargetOpcode::G_FCONSTANT) {
444455
Ty = MI.getOperand(1).getFPImm()->getType();
445456
} else {
446-
assert(MI.getOpcode() == TargetOpcode::G_BUILD_VECTOR);
457+
assert(MIOp == TargetOpcode::G_BUILD_VECTOR);
447458
Type *ElemTy = nullptr;
448459
MachineInstr *ElemMI = MRI.getVRegDef(MI.getOperand(1).getReg());
449460
assert(ElemMI);
@@ -459,7 +470,7 @@ generateAssignInstrs(MachineFunction &MF, SPIRVGlobalRegistry *GR,
459470
Ty = VectorType::get(ElemTy, NumElts, false);
460471
}
461472
insertAssignInstr(Reg, Ty, nullptr, GR, MIB, MRI);
462-
} else if (MI.getOpcode() == TargetOpcode::G_GLOBAL_VALUE) {
473+
} else if (MIOp == TargetOpcode::G_GLOBAL_VALUE) {
463474
propagateSPIRVType(&MI, GR, MRI, MIB);
464475
}
465476

@@ -802,7 +813,7 @@ bool SPIRVPreLegalizer::runOnMachineFunction(MachineFunction &MF) {
802813
MachineIRBuilder MIB(MF);
803814
// a registry of target extension constants
804815
DenseMap<MachineInstr *, Type *> TargetExtConstTypes;
805-
addConstantsToTrack(MF, GR, TargetExtConstTypes);
816+
addConstantsToTrack(MF, GR, ST, TargetExtConstTypes);
806817
foldConstantsIntoIntrinsics(MF);
807818
insertBitcasts(MF, GR, MIB);
808819
generateAssignInstrs(MF, GR, MIB, TargetExtConstTypes);
Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
; RUN: llc -O0 -mtriple=spirv64-unknown-unknown %s -o - | FileCheck %s
2+
; RUN: %if spirv-tools %{ llc -O0 -mtriple=spirv64-unknown-unknown %s -o - -filetype=obj | spirv-val %}
3+
4+
; RUN: llc -O0 -mtriple=spirv32-unknown-unknown %s -o - | FileCheck %s
5+
; RUN: %if spirv-tools %{ llc -O0 -mtriple=spirv32-unknown-unknown %s -o - -filetype=obj | spirv-val %}
6+
7+
; CHECK: %[[#LongTy:]] = OpTypeInt 64 0
8+
; CHECK: %[[#EventTy:]] = OpTypeEvent
9+
; CHECK: %[[#LongNull:]] = OpConstantNull %[[#LongTy]]
10+
; CHECK: %[[#EventNull:]] = OpConstantNull %[[#EventTy]]
11+
; CHECK: OpFunction
12+
; CHECK: OpINotEqual %[[#]] %[[#]] %[[#LongNull]]
13+
; CHECK: OpGroupAsyncCopy %[[#EventTy]] %[[#]] %[[#]] %[[#]] %[[#]] %[[#]] %[[#EventNull]]
14+
15+
16+
define weak_odr dso_local spir_kernel void @foo(i64 %_arg_i, ptr addrspace(1) %_arg_ptr, ptr addrspace(3) %_arg_local) {
17+
entry:
18+
%r1 = icmp ne i64 %_arg_i, 0
19+
%e1 = tail call spir_func target("spirv.Event") @__spirv_GroupAsyncCopy(i32 2, ptr addrspace(3) %_arg_local, ptr addrspace(1) %_arg_ptr, i64 1, i64 1, target("spirv.Event") zeroinitializer)
20+
ret void
21+
}
22+
23+
declare dso_local spir_func target("spirv.Event") @__spirv_GroupAsyncCopy(i32, ptr addrspace(3), ptr addrspace(1), i64, i64, target("spirv.Event"))

0 commit comments

Comments
 (0)