[llvm] ed22029 - [SPIR-V] Address the case when optimization uses GEP operator and GenCode creates G_PTR_ADD to convey the semantics (#107880)
via llvm-commits
llvm-commits at lists.llvm.org
Wed Sep 11 05:18:17 PDT 2024
Author: Vyacheslav Levytskyy
Date: 2024-09-11T14:18:14+02:00
New Revision: ed22029eea12b37c2a58f2c6b8d67f12009102a0
URL: https://github.com/llvm/llvm-project/commit/ed22029eea12b37c2a58f2c6b8d67f12009102a0
DIFF: https://github.com/llvm/llvm-project/commit/ed22029eea12b37c2a58f2c6b8d67f12009102a0.diff
LOG: [SPIR-V] Address the case when optimization uses GEP operator and GenCode creates G_PTR_ADD to convey the semantics (#107880)
When running SPIR-V Backend with optimization levels higher than 0, we
observe GEP Operator's as a new factor, massively used to convey the
semantics of the original LLVM IR. Previously, an issue related to GEP
Operator was mentioned and fixed on the consumer side of toolchains
(see, for example, Khronos Trandslator Issue
https://github.com/KhronosGroup/SPIRV-LLVM-Translator/issues/2486 and PR
https://github.com/KhronosGroup/SPIRV-LLVM-Translator/pull/2487).
However, there is a case when GenCode creates G_PTR_ADD to convey the
original semantics under optimization levels higher than 0 where it's
SPIR-V Backend that fails to translate source LLVM IR correctly.
Consider the following reproducer:
```
%struct = type { i32, [257 x i8], [257 x i8], [129 x i8], i32, i64, i64, i64, i64, i64, i64 }
@Mem = linkonce_odr dso_local addrspace(1) global %struct zeroinitializer, align 8
define weak dso_local spir_func void @__devicelib_assert_fail(ptr addrspace(4) noundef %expr, i32 noundef %line, i1 %fl) {
entry:
%cmp = icmp eq i32 %line, 0
br i1 %cmp, label %lbl, label %exit
lbl:
store i32 %line, ptr addrspace(1) getelementptr inbounds (i8, ptr addrspace(1) @Mem, i64 648), align 8
br i1 %fl, label %lbl, label %exit
exit:
ret void
}
```
converted to the following machine instructions by SPIR-V Backend:
```
%4:type(s64) = OpTypeInt 32, 0
%22:type(s64) = OpTypePointer 5, %4:type(s64)
%2:type(s64) = OpTypeInt 8, 0
%28:type(s64) = OpTypePointer 5, %2:type(s64)
%10:pid(p1) = G_GLOBAL_VALUE @Mem
%36:type(s64) = OpTypeStruct %4:type(s64), %32:type(s64), %32:type(s64), %34:type(s64), %4:type(s64), %35:type(s64), %35:type(s64), %35:type(s64), %35:type(s64), %35:type(s64), %35:type(s64)
%37:iid(s32) = G_INTRINSIC_W_SIDE_EFFECTS intrinsic(@llvm.spv.const.composite)
%8:iid(s32) = ASSIGN_TYPE %37:iid(s32), %36:type(s64)
G_INTRINSIC_W_SIDE_EFFECTS intrinsic(@llvm.spv.init.global), %10:pid(p1), %8:iid(s32)
%29:pid(p1) = nuw G_PTR_ADD %10:pid, %16:iid(s64)
%15:pid(p1) = nuw ASSIGN_TYPE %29:pid(p1), %28:type(s64)
%27:pid(p2) = G_BITCAST %15:pid(p1)
%17:pid(p2) = ASSIGN_TYPE %27:pid(p2), %22:type(s64)
G_STORE %1:iid(s32), %17:pid(p2) :: (store (s32) into %ir.3, align 8, addrspace 1)
```
On the next stage of instruction selection this `G_PTR_ADD`-related
pattern would be interpreted as an initialization of a global variable
and converted to an invalid constant GEP pattern that, in its turn,
would fail to be verified by LLVM during back translation from SPIR-V to
LLVM IR.
This PR introduces a fix for the problem by adding one more case of
`G_PTR_ADD` translation, when we use a non-const GEP to convey the
meaning. The reproducer is attached as a new test case.
Added:
llvm/test/CodeGen/SPIRV/opt-gepoperator-of-gvar.ll
Modified:
llvm/lib/Target/SPIRV/SPIRVDuplicatesTracker.cpp
llvm/lib/Target/SPIRV/SPIRVInstructionSelector.cpp
llvm/lib/Target/SPIRV/SPIRVSymbolicOperands.td
Removed:
################################################################################
diff --git a/llvm/lib/Target/SPIRV/SPIRVDuplicatesTracker.cpp b/llvm/lib/Target/SPIRV/SPIRVDuplicatesTracker.cpp
index 7c32bb1968ef58..832ca0ba5a82d7 100644
--- a/llvm/lib/Target/SPIRV/SPIRVDuplicatesTracker.cpp
+++ b/llvm/lib/Target/SPIRV/SPIRVDuplicatesTracker.cpp
@@ -13,6 +13,8 @@
#include "SPIRVDuplicatesTracker.h"
+#define DEBUG_TYPE "build-dep-graph"
+
using namespace llvm;
template <typename T>
@@ -63,6 +65,18 @@ void SPIRVGeneralDuplicatesTracker::buildDepsGraph(
if (MI->getOpcode() == SPIRV::OpConstantFunctionPointerINTEL && i == 2)
continue;
MachineOperand *RegOp = &VRegDef->getOperand(0);
+ LLVM_DEBUG({
+ if (Reg2Entry.count(RegOp) == 0 &&
+ (MI->getOpcode() != SPIRV::OpVariable || i != 3)) {
+ dbgs() << "Unexpected pattern while building a dependency "
+ "graph.\nInstruction: ";
+ MI->print(dbgs());
+ dbgs() << "Operand: ";
+ Op.print(dbgs());
+ dbgs() << "\nOperand definition: ";
+ VRegDef->print(dbgs());
+ }
+ });
assert((MI->getOpcode() == SPIRV::OpVariable && i == 3) ||
Reg2Entry.count(RegOp));
if (Reg2Entry.count(RegOp))
diff --git a/llvm/lib/Target/SPIRV/SPIRVInstructionSelector.cpp b/llvm/lib/Target/SPIRV/SPIRVInstructionSelector.cpp
index 1e861da35aaac9..831d7f76ac14c9 100644
--- a/llvm/lib/Target/SPIRV/SPIRVInstructionSelector.cpp
+++ b/llvm/lib/Target/SPIRV/SPIRVInstructionSelector.cpp
@@ -607,10 +607,7 @@ bool SPIRVInstructionSelector::spvSelect(Register ResVReg,
case TargetOpcode::G_ADDRSPACE_CAST:
return selectAddrSpaceCast(ResVReg, ResType, I);
case TargetOpcode::G_PTR_ADD: {
- // Currently, we get G_PTR_ADD only as a result of translating
- // global variables, initialized with constant expressions like GV + Const
- // (see test opencl/basic/progvar_prog_scope_init.ll).
- // TODO: extend the handler once we have other cases.
+ // Currently, we get G_PTR_ADD only applied to global variables.
assert(I.getOperand(1).isReg() && I.getOperand(2).isReg());
Register GV = I.getOperand(1).getReg();
MachineRegisterInfo::def_instr_iterator II = MRI->def_instr_begin(GV);
@@ -619,8 +616,68 @@ bool SPIRVInstructionSelector::spvSelect(Register ResVReg,
(*II).getOpcode() == TargetOpcode::COPY ||
(*II).getOpcode() == SPIRV::OpVariable) &&
isImm(I.getOperand(2), MRI));
- Register Idx = buildZerosVal(GR.getOrCreateSPIRVIntegerType(32, I, TII), I);
+ // It may be the initialization of a global variable.
+ bool IsGVInit = false;
+ for (MachineRegisterInfo::use_instr_iterator
+ UseIt = MRI->use_instr_begin(I.getOperand(0).getReg()),
+ UseEnd = MRI->use_instr_end();
+ UseIt != UseEnd; UseIt = std::next(UseIt)) {
+ if ((*UseIt).getOpcode() == TargetOpcode::G_GLOBAL_VALUE ||
+ (*UseIt).getOpcode() == SPIRV::OpVariable) {
+ IsGVInit = true;
+ break;
+ }
+ }
MachineBasicBlock &BB = *I.getParent();
+ if (!IsGVInit) {
+ SPIRVType *GVType = GR.getSPIRVTypeForVReg(GV);
+ SPIRVType *GVPointeeType = GR.getPointeeType(GVType);
+ SPIRVType *ResPointeeType = GR.getPointeeType(ResType);
+ if (GVPointeeType && ResPointeeType && GVPointeeType != ResPointeeType) {
+ // Build a new virtual register that is associated with the required
+ // data type.
+ Register NewVReg = MRI->createGenericVirtualRegister(MRI->getType(GV));
+ MRI->setRegClass(NewVReg, MRI->getRegClass(GV));
+ // Having a correctly typed base we are ready to build the actually
+ // required GEP. It may not be a constant though, because all Operands
+ // of OpSpecConstantOp is to originate from other const instructions,
+ // and only the AccessChain named opcodes accept a global OpVariable
+ // instruction. We can't use an AccessChain opcode because of the type
+ // mismatch between result and base types.
+ if (!GR.isBitcastCompatible(ResType, GVType))
+ report_fatal_error(
+ "incompatible result and operand types in a bitcast");
+ Register ResTypeReg = GR.getSPIRVTypeID(ResType);
+ MachineInstrBuilder MIB =
+ BuildMI(BB, I, I.getDebugLoc(), TII.get(SPIRV::OpBitcast))
+ .addDef(NewVReg)
+ .addUse(ResTypeReg)
+ .addUse(GV);
+ return MIB.constrainAllUses(TII, TRI, RBI) &&
+ BuildMI(BB, I, I.getDebugLoc(),
+ TII.get(STI.isVulkanEnv()
+ ? SPIRV::OpInBoundsAccessChain
+ : SPIRV::OpInBoundsPtrAccessChain))
+ .addDef(ResVReg)
+ .addUse(ResTypeReg)
+ .addUse(NewVReg)
+ .addUse(I.getOperand(2).getReg())
+ .constrainAllUses(TII, TRI, RBI);
+ } else {
+ return BuildMI(BB, I, I.getDebugLoc(), TII.get(SPIRV::OpSpecConstantOp))
+ .addDef(ResVReg)
+ .addUse(GR.getSPIRVTypeID(ResType))
+ .addImm(
+ static_cast<uint32_t>(SPIRV::Opcode::InBoundsPtrAccessChain))
+ .addUse(GV)
+ .addUse(I.getOperand(2).getReg())
+ .constrainAllUses(TII, TRI, RBI);
+ }
+ }
+ // It's possible to translate G_PTR_ADD to OpSpecConstantOp: either to
+ // initialize a global variable with a constant expression (e.g., the test
+ // case opencl/basic/progvar_prog_scope_init.ll), or for another use case
+ Register Idx = buildZerosVal(GR.getOrCreateSPIRVIntegerType(32, I, TII), I);
auto MIB = BuildMI(BB, I, I.getDebugLoc(), TII.get(SPIRV::OpSpecConstantOp))
.addDef(ResVReg)
.addUse(GR.getSPIRVTypeID(ResType))
diff --git a/llvm/lib/Target/SPIRV/SPIRVSymbolicOperands.td b/llvm/lib/Target/SPIRV/SPIRVSymbolicOperands.td
index 96601dd8796c63..23cd32eff45d5b 100644
--- a/llvm/lib/Target/SPIRV/SPIRVSymbolicOperands.td
+++ b/llvm/lib/Target/SPIRV/SPIRVSymbolicOperands.td
@@ -1628,6 +1628,7 @@ multiclass OpcodeOperand<bits<32> value> {
defm : SymbolicOperandWithRequirements<OpcodeOperand, value, NAME, 0, 0, [], []>;
}
// TODO: implement other mnemonics.
+defm InBoundsAccessChain : OpcodeOperand<66>;
defm InBoundsPtrAccessChain : OpcodeOperand<70>;
defm PtrCastToGeneric : OpcodeOperand<121>;
defm Bitcast : OpcodeOperand<124>;
diff --git a/llvm/test/CodeGen/SPIRV/opt-gepoperator-of-gvar.ll b/llvm/test/CodeGen/SPIRV/opt-gepoperator-of-gvar.ll
new file mode 100644
index 00000000000000..5f9229f5a5bd6b
--- /dev/null
+++ b/llvm/test/CodeGen/SPIRV/opt-gepoperator-of-gvar.ll
@@ -0,0 +1,64 @@
+; RUN: llc -O2 -mtriple=spirv64-unknown-unknown %s -o - | FileCheck %s
+; RUN: %if spirv-tools %{ llc -O2 -mtriple=spirv64-unknown-unknown %s -o - -filetype=obj | spirv-val %}
+
+; RUN: llc -O2 -mtriple=spirv32-unknown-unknown %s -o - | FileCheck %s
+; RUN: %if spirv-tools %{ llc -O2 -mtriple=spirv32-unknown-unknown %s -o - -filetype=obj | spirv-val %}
+
+; CHECK-DAG: %[[#Char:]] = OpTypeInt 8 0
+; CHECK-DAG: %[[#PtrChar:]] = OpTypePointer CrossWorkgroup %[[#Char]]
+; CHECK-DAG: %[[#Int:]] = OpTypeInt 32 0
+; CHECK-DAG: %[[#PtrInt:]] = OpTypePointer CrossWorkgroup %[[#Int]]
+; CHECK-DAG: %[[#C648:]] = OpConstant %[[#]] 648
+; CHECK-DAG: %[[#Struct:]] = OpTypeStruct %[[#]] %[[#]] %[[#]] %[[#]] %[[#]] %[[#]] %[[#]] %[[#]] %[[#]] %[[#]] %[[#]]
+; CHECK-DAG: %[[#VarInit:]] = OpConstantNull %[[#Struct]]
+; CHECK-DAG: %[[#PtrStruct:]] = OpTypePointer CrossWorkgroup %[[#Struct]]
+; CHECK-DAG: %[[#Var:]] = OpVariable %[[#PtrStruct]] CrossWorkgroup %[[#VarInit]]
+; CHECK-DAG: %[[#Bytes:]] = OpVariable %[[#PtrChar]] CrossWorkgroup %[[#]]
+; CHECK-DAG: %[[#BytesGEP:]] = OpSpecConstantOp %[[#PtrChar]] 70 %[[#Bytes]] %[[#C648]]
+
+; CHECK: OpFunction
+; CHECK: %[[#]] = OpFunctionParameter %[[#]]
+; CHECK: %[[#Line:]] = OpFunctionParameter %[[#Int]]
+; CHECK: %[[#]] = OpFunctionParameter %[[#]]
+; CHECK: %[[#Casted:]] = OpBitcast %[[#PtrChar]] %[[#Var]]
+; CHECK: %[[#AddrChar:]] = OpInBoundsPtrAccessChain %[[#PtrChar]] %[[#Casted]] %[[#C648]]
+; CHECK: %[[#AddrInt:]] = OpBitcast %[[#PtrInt]] %[[#AddrChar]]
+; CHECK: OpStore %[[#AddrInt]] %[[#Line]]
+
+%struct = type { i32, [257 x i8], [257 x i8], [129 x i8], i32, i64, i64, i64, i64, i64, i64 }
+ at Mem = linkonce_odr dso_local addrspace(1) global %struct zeroinitializer, align 8
+
+define weak dso_local spir_func void @foo(ptr addrspace(4) noundef %expr, i32 noundef %line, i1 %fl) {
+entry:
+ %cmp = icmp eq i32 %line, 0
+ br i1 %cmp, label %lbl, label %exit
+
+lbl:
+ store i32 %line, ptr addrspace(1) getelementptr inbounds (i8, ptr addrspace(1) @Mem, i64 648), align 8
+ br i1 %fl, label %lbl, label %exit
+
+exit:
+ ret void
+}
+
+; CHECK: OpFunction
+; CHECK: %[[#]] = OpFunctionParameter %[[#]]
+; CHECK: %[[#Line2:]] = OpFunctionParameter %[[#Int]]
+; CHECK: %[[#]] = OpFunctionParameter %[[#]]
+; CHECK: %[[#AddrInt2:]] = OpBitcast %[[#PtrInt]] %[[#BytesGEP]]
+; CHECK: OpStore %[[#AddrInt2]] %[[#Line2]]
+
+ at Bytes = linkonce_odr dso_local addrspace(1) global i8 zeroinitializer, align 8
+
+define weak dso_local spir_func void @bar(ptr addrspace(4) noundef %expr, i32 noundef %line, i1 %fl) {
+entry:
+ %cmp = icmp eq i32 %line, 0
+ br i1 %cmp, label %lbl, label %exit
+
+lbl:
+ store i32 %line, ptr addrspace(1) getelementptr inbounds (i8, ptr addrspace(1) @Bytes, i64 648), align 8
+ br i1 %fl, label %lbl, label %exit
+
+exit:
+ ret void
+}
More information about the llvm-commits
mailing list