From 7c917e8268225735bf6fe0f7d8491fc944358e47 Mon Sep 17 00:00:00 2001 From: Vyacheslav Levytskyy Date: Wed, 29 May 2024 12:53:37 +0200 Subject: [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. --- 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 e4bbeb5..ffbd1e1 100644 --- a/llvm/lib/Target/SPIRV/SPIRVEmitIntrinsics.cpp +++ b/llvm/lib/Target/SPIRV/SPIRVEmitIntrinsics.cpp @@ -1212,8 +1212,7 @@ void SPIRVEmitIntrinsics::processInstrAfterVisit(Instruction *I, } Value *OpTyVal = Op; if (Op->getType()->isTargetExtTy()) - OpTyVal = Constant::getNullValue( - IntegerType::get(I->getContext(), GR->getPointerSize())); + OpTyVal = PoisonValue::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 85299a4..62489960 100644 --- a/llvm/lib/Target/SPIRV/SPIRVPreLegalizer.cpp +++ b/llvm/lib/Target/SPIRV/SPIRVPreLegalizer.cpp @@ -40,6 +40,7 @@ public: static void addConstantsToTrack(MachineFunction &MF, SPIRVGlobalRegistry *GR, + const SPIRVSubtarget &STI, DenseMap &TargetExtConstTypes) { MachineRegisterInfo &MRI = MF.getRegInfo(); DenseMap 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 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 0000000..b40456d --- /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")) -- cgit v1.1