[llvm] [SPIR-V] Implement correct zeroinitializer for extension types in SPIR-V Backend (PR #93607)
Vyacheslav Levytskyy via llvm-commits
llvm-commits at lists.llvm.org
Tue May 28 13:09:54 PDT 2024
https://github.com/VyacheslavLevytskyy created https://github.com/llvm/llvm-project/pull/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.
>From 403948e1613aeaaba4586049e33f1c4c0e5cc904 Mon Sep 17 00:00:00 2001
From: "Levytskyy, Vyacheslav" <vyacheslav.levytskyy at intel.com>
Date: Tue, 28 May 2024 13:03:27 -0700
Subject: [PATCH] Fix ext type zeroinitialyzer
---
llvm/lib/Target/SPIRV/SPIRVEmitIntrinsics.cpp | 3 +-
llvm/lib/Target/SPIRV/SPIRVPreLegalizer.cpp | 29 +++++++++++++------
llvm/test/CodeGen/SPIRV/event-zero-const.ll | 23 +++++++++++++++
3 files changed, 44 insertions(+), 11 deletions(-)
create mode 100644 llvm/test/CodeGen/SPIRV/event-zero-const.ll
diff --git a/llvm/lib/Target/SPIRV/SPIRVEmitIntrinsics.cpp b/llvm/lib/Target/SPIRV/SPIRVEmitIntrinsics.cpp
index ea53fe55e7ab5..46ef287b741c0 100644
--- a/llvm/lib/Target/SPIRV/SPIRVEmitIntrinsics.cpp
+++ b/llvm/lib/Target/SPIRV/SPIRVEmitIntrinsics.cpp
@@ -1191,8 +1191,7 @@ void SPIRVEmitIntrinsics::processInstrAfterVisit(Instruction *I,
B.SetInsertPoint(I);
Value *OpTyVal = Op;
if (Op->getType()->isTargetExtTy())
- OpTyVal = Constant::getNullValue(
- IntegerType::get(I->getContext(), GR->getPointerSize()));
+ OpTyVal = UndefValue::get(Op->getType());
auto *NewOp = buildIntrWithMD(Intrinsic::spv_track_constant,
{Op->getType(), OpTyVal->getType()}, Op,
OpTyVal, {}, B);
diff --git a/llvm/lib/Target/SPIRV/SPIRVPreLegalizer.cpp b/llvm/lib/Target/SPIRV/SPIRVPreLegalizer.cpp
index 85299a49a6b94..624899600693a 100644
--- a/llvm/lib/Target/SPIRV/SPIRVPreLegalizer.cpp
+++ b/llvm/lib/Target/SPIRV/SPIRVPreLegalizer.cpp
@@ -40,6 +40,7 @@ class SPIRVPreLegalizer : public MachineFunctionPass {
static void
addConstantsToTrack(MachineFunction &MF, SPIRVGlobalRegistry *GR,
+ const SPIRVSubtarget &STI,
DenseMap<MachineInstr *, Type *> &TargetExtConstTypes) {
MachineRegisterInfo &MRI = MF.getRegInfo();
DenseMap<MachineInstr *, Register> RegsAlreadyAddedToDT;
@@ -82,8 +83,17 @@ addConstantsToTrack(MachineFunction &MF, SPIRVGlobalRegistry *GR,
if (Const->getType()->isTargetExtTy()) {
// remember association so that we can restore it when assign types
MachineInstr *SrcMI = MRI.getVRegDef(SrcReg);
- if (SrcMI && SrcMI->getOpcode() == TargetOpcode::G_CONSTANT)
+ if (SrcMI && (SrcMI->getOpcode() == TargetOpcode::G_CONSTANT ||
+ SrcMI->getOpcode() == TargetOpcode::G_IMPLICIT_DEF))
TargetExtConstTypes[SrcMI] = Const->getType();
+ if (Const->isNullValue()) {
+ MachineIRBuilder MIB(MF);
+ SPIRVType *ExtType =
+ GR->getOrCreateSPIRVType(Const->getType(), MIB);
+ SrcMI->setDesc(STI.getInstrInfo()->get(SPIRV::OpConstantNull));
+ SrcMI->addOperand(MachineOperand::CreateReg(
+ GR->getSPIRVTypeID(ExtType), false));
+ }
}
} else {
RegsAlreadyAddedToDT[&MI] = Reg;
@@ -394,6 +404,7 @@ generateAssignInstrs(MachineFunction &MF, SPIRVGlobalRegistry *GR,
for (auto MII = std::prev(MBB->end()), Begin = MBB->begin();
!ReachedBegin;) {
MachineInstr &MI = *MII;
+ unsigned MIOp = MI.getOpcode();
if (isSpvIntrinsic(MI, Intrinsic::spv_assign_ptr_type)) {
Register Reg = MI.getOperand(1).getReg();
@@ -419,9 +430,9 @@ generateAssignInstrs(MachineFunction &MF, SPIRVGlobalRegistry *GR,
if (Def->getOpcode() != TargetOpcode::G_GLOBAL_VALUE)
insertAssignInstr(Reg, Ty, nullptr, GR, MIB, MF.getRegInfo());
ToErase.push_back(&MI);
- } else if (MI.getOpcode() == TargetOpcode::G_CONSTANT ||
- MI.getOpcode() == TargetOpcode::G_FCONSTANT ||
- MI.getOpcode() == TargetOpcode::G_BUILD_VECTOR) {
+ } else if (MIOp == TargetOpcode::G_CONSTANT ||
+ MIOp == TargetOpcode::G_FCONSTANT ||
+ MIOp == TargetOpcode::G_BUILD_VECTOR) {
// %rc = G_CONSTANT ty Val
// ===>
// %cty = OpType* ty
@@ -435,15 +446,15 @@ generateAssignInstrs(MachineFunction &MF, SPIRVGlobalRegistry *GR,
continue;
}
Type *Ty = nullptr;
- if (MI.getOpcode() == TargetOpcode::G_CONSTANT) {
+ if (MIOp == TargetOpcode::G_CONSTANT) {
auto TargetExtIt = TargetExtConstTypes.find(&MI);
Ty = TargetExtIt == TargetExtConstTypes.end()
? MI.getOperand(1).getCImm()->getType()
: TargetExtIt->second;
- } else if (MI.getOpcode() == TargetOpcode::G_FCONSTANT) {
+ } else if (MIOp == TargetOpcode::G_FCONSTANT) {
Ty = MI.getOperand(1).getFPImm()->getType();
} else {
- assert(MI.getOpcode() == TargetOpcode::G_BUILD_VECTOR);
+ assert(MIOp == TargetOpcode::G_BUILD_VECTOR);
Type *ElemTy = nullptr;
MachineInstr *ElemMI = MRI.getVRegDef(MI.getOperand(1).getReg());
assert(ElemMI);
@@ -459,7 +470,7 @@ generateAssignInstrs(MachineFunction &MF, SPIRVGlobalRegistry *GR,
Ty = VectorType::get(ElemTy, NumElts, false);
}
insertAssignInstr(Reg, Ty, nullptr, GR, MIB, MRI);
- } else if (MI.getOpcode() == TargetOpcode::G_GLOBAL_VALUE) {
+ } else if (MIOp == TargetOpcode::G_GLOBAL_VALUE) {
propagateSPIRVType(&MI, GR, MRI, MIB);
}
@@ -802,7 +813,7 @@ bool SPIRVPreLegalizer::runOnMachineFunction(MachineFunction &MF) {
MachineIRBuilder MIB(MF);
// a registry of target extension constants
DenseMap<MachineInstr *, Type *> TargetExtConstTypes;
- addConstantsToTrack(MF, GR, TargetExtConstTypes);
+ addConstantsToTrack(MF, GR, ST, TargetExtConstTypes);
foldConstantsIntoIntrinsics(MF);
insertBitcasts(MF, GR, MIB);
generateAssignInstrs(MF, GR, MIB, TargetExtConstTypes);
diff --git a/llvm/test/CodeGen/SPIRV/event-zero-const.ll b/llvm/test/CodeGen/SPIRV/event-zero-const.ll
new file mode 100644
index 0000000000000..b40456d233f12
--- /dev/null
+++ b/llvm/test/CodeGen/SPIRV/event-zero-const.ll
@@ -0,0 +1,23 @@
+; RUN: llc -O0 -mtriple=spirv64-unknown-unknown %s -o - | FileCheck %s
+; RUN: %if spirv-tools %{ llc -O0 -mtriple=spirv64-unknown-unknown %s -o - -filetype=obj | spirv-val %}
+
+; RUN: llc -O0 -mtriple=spirv32-unknown-unknown %s -o - | FileCheck %s
+; RUN: %if spirv-tools %{ llc -O0 -mtriple=spirv32-unknown-unknown %s -o - -filetype=obj | spirv-val %}
+
+; CHECK: %[[#LongTy:]] = OpTypeInt 64 0
+; CHECK: %[[#EventTy:]] = OpTypeEvent
+; CHECK: %[[#LongNull:]] = OpConstantNull %[[#LongTy]]
+; CHECK: %[[#EventNull:]] = OpConstantNull %[[#EventTy]]
+; CHECK: OpFunction
+; CHECK: OpINotEqual %[[#]] %[[#]] %[[#LongNull]]
+; CHECK: OpGroupAsyncCopy %[[#EventTy]] %[[#]] %[[#]] %[[#]] %[[#]] %[[#]] %[[#EventNull]]
+
+
+define weak_odr dso_local spir_kernel void @foo(i64 %_arg_i, ptr addrspace(1) %_arg_ptr, ptr addrspace(3) %_arg_local) {
+entry:
+ %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)
+ ret void
+}
+
+declare dso_local spir_func target("spirv.Event") @__spirv_GroupAsyncCopy(i32, ptr addrspace(3), ptr addrspace(1), i64, i64, target("spirv.Event"))
More information about the llvm-commits
mailing list