[llvm] [SPIR-V] Insert a bitcast before load/store instruction to keep SPIR-V code valid (PR #84069)
Vyacheslav Levytskyy via llvm-commits
llvm-commits at lists.llvm.org
Tue Mar 5 13:04:48 PST 2024
https://github.com/VyacheslavLevytskyy created https://github.com/llvm/llvm-project/pull/84069
This PR introduces a step after instruction selection where instructions can be traversed from the perspective of their validity from the specification point of view. The PR adds also a way to correct load/store when there is a type mismatch contradicting the specification -- an additional bitcast is inserted to keep types consistent. Correspondent test cases are added and existing test cases are corrected.
This PR helps to successfully validate with the `spirv-val` tool (https://github.com/KhronosGroup/SPIRV-Tools) some output that previously led to validation errors and crashes of back translation from SPIRV to LLVM IR from the side of SPIRV Translator project (https://github.com/KhronosGroup/SPIRV-LLVM-Translator).
The added step of bringing instructions to required by the specification type correspondence can be (should be and will be) extended beyond load/store instructions to ensure validity rules of other SPIRV instructions related to type inference.
>From afcbb97922f521fa7be986eeb157bcef225cd5a5 Mon Sep 17 00:00:00 2001
From: "Levytskyy, Vyacheslav" <vyacheslav.levytskyy at intel.com>
Date: Tue, 5 Mar 2024 12:53:05 -0800
Subject: [PATCH] keep code valid by inserting additional bitcasts
---
llvm/lib/Target/SPIRV/SPIRVGlobalRegistry.cpp | 7 ++
llvm/lib/Target/SPIRV/SPIRVISelLowering.cpp | 78 +++++++++++++++++++
llvm/lib/Target/SPIRV/SPIRVISelLowering.h | 12 ++-
llvm/test/CodeGen/SPIRV/bitcast-fix-load.ll | 24 ++++++
llvm/test/CodeGen/SPIRV/bitcast-fix-store.ll | 35 +++++++++
.../SPIRV/constant/global-constants.ll | 3 +
llvm/test/CodeGen/SPIRV/spirv-load-store.ll | 10 ++-
7 files changed, 164 insertions(+), 5 deletions(-)
create mode 100644 llvm/test/CodeGen/SPIRV/bitcast-fix-load.ll
create mode 100644 llvm/test/CodeGen/SPIRV/bitcast-fix-store.ll
diff --git a/llvm/lib/Target/SPIRV/SPIRVGlobalRegistry.cpp b/llvm/lib/Target/SPIRV/SPIRVGlobalRegistry.cpp
index e88298f52fbe18..fea9366efc3a58 100644
--- a/llvm/lib/Target/SPIRV/SPIRVGlobalRegistry.cpp
+++ b/llvm/lib/Target/SPIRV/SPIRVGlobalRegistry.cpp
@@ -517,6 +517,13 @@ Register SPIRVGlobalRegistry::buildGlobalVariable(
LLT RegLLTy = LLT::pointer(MRI->getType(ResVReg).getAddressSpace(), 32);
MRI->setType(Reg, RegLLTy);
assignSPIRVTypeToVReg(BaseType, Reg, MIRBuilder.getMF());
+ } else {
+ // Our knowledge about the type may be updated.
+ // If that's the case, we need to update a type
+ // associated with the register.
+ SPIRVType *DefType = getSPIRVTypeForVReg(ResVReg);
+ if (!DefType || DefType != BaseType)
+ assignSPIRVTypeToVReg(BaseType, Reg, MIRBuilder.getMF());
}
// If it's a global variable with name, output OpName for it.
diff --git a/llvm/lib/Target/SPIRV/SPIRVISelLowering.cpp b/llvm/lib/Target/SPIRV/SPIRVISelLowering.cpp
index 33c6aa242969de..27539422302ab7 100644
--- a/llvm/lib/Target/SPIRV/SPIRVISelLowering.cpp
+++ b/llvm/lib/Target/SPIRV/SPIRVISelLowering.cpp
@@ -12,6 +12,13 @@
#include "SPIRVISelLowering.h"
#include "SPIRV.h"
+#include "SPIRVInstrInfo.h"
+#include "SPIRVRegisterBankInfo.h"
+#include "SPIRVRegisterInfo.h"
+#include "SPIRVSubtarget.h"
+#include "SPIRVTargetMachine.h"
+#include "llvm/CodeGen/MachineInstrBuilder.h"
+#include "llvm/CodeGen/MachineRegisterInfo.h"
#include "llvm/IR/IntrinsicsSPIRV.h"
#define DEBUG_TYPE "spirv-lower"
@@ -74,3 +81,74 @@ bool SPIRVTargetLowering::getTgtMemIntrinsic(IntrinsicInfo &Info,
}
return false;
}
+
+// Insert a bitcast before the instruction to keep SPIR-V code valid
+// when there is a type mismatch between results and operand types.
+static void validatePtrTypes(const SPIRVSubtarget &STI,
+ MachineRegisterInfo *MRI, SPIRVGlobalRegistry &GR,
+ MachineInstr &I, SPIRVType *ResType,
+ unsigned OpIdx) {
+ Register OpReg = I.getOperand(OpIdx).getReg();
+ SPIRVType *TypeInst = MRI->getVRegDef(OpReg);
+ SPIRVType *OpType = GR.getSPIRVTypeForVReg(
+ TypeInst && TypeInst->getOpcode() == SPIRV::OpFunctionParameter
+ ? TypeInst->getOperand(1).getReg()
+ : OpReg);
+ if (!ResType || !OpType || OpType->getOpcode() != SPIRV::OpTypePointer)
+ return;
+ SPIRVType *ElemType = GR.getSPIRVTypeForVReg(OpType->getOperand(2).getReg());
+ if (!ElemType || ElemType == ResType)
+ return;
+ // There is a type mismatch between results and operand types
+ // and we insert a bitcast before the instruction to keep SPIR-V code valid
+ SPIRV::StorageClass::StorageClass SC =
+ static_cast<SPIRV::StorageClass::StorageClass>(
+ OpType->getOperand(1).getImm());
+ MachineInstr *PrevI = I.getPrevNode();
+ MachineBasicBlock &MBB = *I.getParent();
+ MachineBasicBlock::iterator InsPt =
+ PrevI ? PrevI->getIterator() : MBB.begin();
+ MachineIRBuilder MIB(MBB, InsPt);
+ SPIRVType *NewPtrType = GR.getOrCreateSPIRVPointerType(ResType, MIB, SC);
+ if (!GR.isBitcastCompatible(NewPtrType, OpType))
+ report_fatal_error(
+ "insert validation bitcast: incompatible result and operand types");
+ Register NewReg = MRI->createGenericVirtualRegister(LLT::scalar(32));
+ bool Res = MIB.buildInstr(SPIRV::OpBitcast)
+ .addDef(NewReg)
+ .addUse(GR.getSPIRVTypeID(NewPtrType))
+ .addUse(OpReg)
+ .constrainAllUses(*STI.getInstrInfo(), *STI.getRegisterInfo(),
+ *STI.getRegBankInfo());
+ if (!Res)
+ report_fatal_error("insert validation bitcast: cannot constrain all uses");
+ MRI->setRegClass(NewReg, &SPIRV::IDRegClass);
+ GR.assignSPIRVTypeToVReg(NewPtrType, NewReg, MIB.getMF());
+ I.getOperand(OpIdx).setReg(NewReg);
+}
+
+void SPIRVTargetLowering::finalizeLowering(MachineFunction &MF) const {
+ MachineRegisterInfo *MRI = &MF.getRegInfo();
+ SPIRVGlobalRegistry &GR = *STI.getSPIRVGlobalRegistry();
+ GR.setCurrentFunc(MF);
+ for (MachineFunction::iterator I = MF.begin(), E = MF.end(); I != E; ++I) {
+ MachineBasicBlock *MBB = &*I;
+ for (MachineBasicBlock::iterator MBBI = MBB->begin(), MBBE = MBB->end();
+ MBBI != MBBE;) {
+ MachineInstr &MI = *MBBI++;
+ switch (MI.getOpcode()) {
+ case SPIRV::OpLoad:
+ // OpLoad <ResType>, ptr %Op implies that %Op is a pointer to <ResType>
+ validatePtrTypes(STI, MRI, GR, MI,
+ GR.getSPIRVTypeForVReg(MI.getOperand(0).getReg()), 2);
+ break;
+ case SPIRV::OpStore:
+ // OpStore ptr %Op, <Obj> implies that %Op points to the <Obj>'s type
+ validatePtrTypes(STI, MRI, GR, MI,
+ GR.getSPIRVTypeForVReg(MI.getOperand(1).getReg()), 0);
+ break;
+ }
+ }
+ }
+ TargetLowering::finalizeLowering(MF);
+}
diff --git a/llvm/lib/Target/SPIRV/SPIRVISelLowering.h b/llvm/lib/Target/SPIRV/SPIRVISelLowering.h
index d34f802e9d889f..b01571bfc1eeb5 100644
--- a/llvm/lib/Target/SPIRV/SPIRVISelLowering.h
+++ b/llvm/lib/Target/SPIRV/SPIRVISelLowering.h
@@ -14,16 +14,19 @@
#ifndef LLVM_LIB_TARGET_SPIRV_SPIRVISELLOWERING_H
#define LLVM_LIB_TARGET_SPIRV_SPIRVISELLOWERING_H
+#include "SPIRVGlobalRegistry.h"
#include "llvm/CodeGen/TargetLowering.h"
namespace llvm {
class SPIRVSubtarget;
class SPIRVTargetLowering : public TargetLowering {
+ const SPIRVSubtarget &STI;
+
public:
explicit SPIRVTargetLowering(const TargetMachine &TM,
- const SPIRVSubtarget &STI)
- : TargetLowering(TM) {}
+ const SPIRVSubtarget &ST)
+ : TargetLowering(TM), STI(ST) {}
// Stop IRTranslator breaking up FMA instrs to preserve types information.
bool isFMAFasterThanFMulAndFAdd(const MachineFunction &MF,
@@ -47,6 +50,11 @@ class SPIRVTargetLowering : public TargetLowering {
bool getTgtMemIntrinsic(IntrinsicInfo &Info, const CallInst &I,
MachineFunction &MF,
unsigned Intrinsic) const override;
+
+ // Call the default implementation and finalize target lowering by inserting
+ // extra instructions required to preserve validity of SPIR-V code imposed by
+ // the standard.
+ void finalizeLowering(MachineFunction &MF) const override;
};
} // namespace llvm
diff --git a/llvm/test/CodeGen/SPIRV/bitcast-fix-load.ll b/llvm/test/CodeGen/SPIRV/bitcast-fix-load.ll
new file mode 100644
index 00000000000000..a2b3cb9349aaf6
--- /dev/null
+++ b/llvm/test/CodeGen/SPIRV/bitcast-fix-load.ll
@@ -0,0 +1,24 @@
+; 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 %}
+
+; CHECK-DAG: %[[#TYLONG:]] = OpTypeInt 32 0
+; CHECK-DAG: %[[#TYSTRUCTLONG:]] = OpTypeStruct %[[#TYLONG]]
+; CHECK-DAG: %[[#TYARRAY:]] = OpTypeArray %[[#TYSTRUCTLONG]] %[[#]]
+; CHECK-DAG: %[[#TYSTRUCT:]] = OpTypeStruct %[[#TYARRAY]]
+; CHECK-DAG: %[[#TYSTRUCTPTR:]] = OpTypePointer Function %[[#TYSTRUCT]]
+; CHECK-DAG: %[[#TYLONGPTR:]] = OpTypePointer Function %[[#TYLONG]]
+; CHECK: %[[#PTRTOSTRUCT:]] = OpFunctionParameter %[[#TYSTRUCTPTR]]
+; CHECK: %[[#PTRTOLONG:]] = OpBitcast %[[#TYLONGPTR]] %[[#PTRTOSTRUCT]]
+; CHECK: OpLoad %[[#TYLONG]] %[[#PTRTOLONG]]
+
+target datalayout = "e-i64:64-v16:16-v24:32-v32:32-v48:64-v96:128-v192:256-v256:256-v512:512-v1024:1024-n8:16:32:64"
+target triple = "spir64-unknown-unknown"
+
+%struct.S = type { i32 }
+%struct.__wrapper_class = type { [7 x %struct.S] }
+
+define spir_kernel void @foo(ptr noundef byval(%struct.__wrapper_class) align 4 %_arg_Arr) {
+entry:
+ %val = load i32, ptr %_arg_Arr
+ ret void
+}
diff --git a/llvm/test/CodeGen/SPIRV/bitcast-fix-store.ll b/llvm/test/CodeGen/SPIRV/bitcast-fix-store.ll
new file mode 100644
index 00000000000000..4d216df8514a46
--- /dev/null
+++ b/llvm/test/CodeGen/SPIRV/bitcast-fix-store.ll
@@ -0,0 +1,35 @@
+; 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 %}
+
+; CHECK-DAG: %[[#TYLONG:]] = OpTypeInt 32 0
+; CHECK-DAG: %[[#TYLONGPTR:]] = OpTypePointer Function %[[#TYLONG]]
+; CHECK-DAG: %[[#TYSTRUCT:]] = OpTypeStruct %[[#TYLONG]]
+; CHECK-DAG: %[[#CONST:]] = OpConstant %[[#TYLONG]] 3
+; CHECK-DAG: %[[#TYSTRUCTPTR:]] = OpTypePointer Function %[[#TYSTRUCT]]
+; CHECK: OpFunction
+; CHECK: %[[#ARGPTR1:]] = OpFunctionParameter %[[#TYLONGPTR]]
+; CHECK: OpStore %[[#ARGPTR1]] %[[#CONST:]]
+; CHECK: OpFunction
+; CHECK: %[[#OBJ:]] = OpFunctionParameter %[[#TYSTRUCT]]
+; CHECK: %[[#ARGPTR2:]] = OpFunctionParameter %[[#TYLONGPTR]]
+; CHECK: %[[#PTRTOSTRUCT:]] = OpBitcast %[[#TYSTRUCTPTR]] %[[#ARGPTR2]]
+; CHECK: OpStore %[[#PTRTOSTRUCT]] %[[#OBJ]]
+
+target datalayout = "e-i64:64-v16:16-v24:32-v32:32-v48:64-v96:128-v192:256-v256:256-v512:512-v1024:1024-n8:16:32:64"
+target triple = "spir64-unknown-unknown"
+
+%struct.S = type { i32 }
+%struct.__wrapper_class = type { [7 x %struct.S] }
+
+;define spir_kernel void @foo(ptr noundef byval(%struct.__wrapper_class) align 4 %_arg_Arr) {
+define spir_kernel void @foo(%struct.S %arg, ptr %ptr) {
+entry:
+ store %struct.S %arg, ptr %ptr
+ ret void
+}
+
+define spir_kernel void @bar(ptr %ptr) {
+entry:
+ store i32 3, ptr %ptr
+ ret void
+}
diff --git a/llvm/test/CodeGen/SPIRV/constant/global-constants.ll b/llvm/test/CodeGen/SPIRV/constant/global-constants.ll
index 916c70628d0169..1e400accaec0c1 100644
--- a/llvm/test/CodeGen/SPIRV/constant/global-constants.ll
+++ b/llvm/test/CodeGen/SPIRV/constant/global-constants.ll
@@ -1,5 +1,8 @@
; RUN: llc -O0 -mtriple=spirv32-unknown-unknown %s -o - | FileCheck %s
+target datalayout = "e-i64:64-v16:16-v24:32-v32:32-v48:64-v96:128-v192:256-v256:256-v512:512-v1024:1024"
+target triple = "spir64"
+
@global = addrspace(1) constant i32 1 ; OpenCL global memory
@constant = addrspace(2) constant i32 2 ; OpenCL constant memory
@local = addrspace(3) constant i32 3 ; OpenCL local memory
diff --git a/llvm/test/CodeGen/SPIRV/spirv-load-store.ll b/llvm/test/CodeGen/SPIRV/spirv-load-store.ll
index a82bf0ab2e01f6..9788f0a651c4d2 100644
--- a/llvm/test/CodeGen/SPIRV/spirv-load-store.ll
+++ b/llvm/test/CodeGen/SPIRV/spirv-load-store.ll
@@ -1,9 +1,13 @@
; RUN: llc -O0 -mtriple=spirv64-unknown-unknown %s -o - | FileCheck %s
;; Translate SPIR-V friendly OpLoad and OpStore calls
-; CHECK: %[[#CONST:]] = OpConstant %[[#]] 42
-; CHECK: OpStore %[[#PTR:]] %[[#CONST]] Volatile|Aligned 4
-; CHECK: %[[#]] = OpLoad %[[#]] %[[#PTR]]
+; CHECK-DAG: %[[#TYLONG:]] = OpTypeInt 32 0
+; CHECK-DAG: %[[#TYFLOAT:]] = OpTypeFloat 64
+; CHECK-DAG: %[[#TYFLOATPTR:]] = OpTypePointer CrossWorkgroup %[[#TYFLOAT]]
+; CHECK-DAG: %[[#CONST:]] = OpConstant %[[#TYLONG]] 42
+; CHECK: OpStore %[[#PTRTOLONG:]] %[[#CONST]] Volatile|Aligned 4
+; CHECK: %[[#PTRTOFLOAT:]] = OpBitcast %[[#TYFLOATPTR]] %[[#PTRTOLONG]]
+; CHECK: OpLoad %[[#TYFLOAT]] %[[#PTRTOFLOAT]]
define weak_odr dso_local spir_kernel void @foo(i32 addrspace(1)* %var) {
entry:
More information about the llvm-commits
mailing list