[llvm] [SPIRV] Add a `SPIRVTypeInst` type with some guardrails (PR #179947)
Juan Manuel Martinez Caamaño via llvm-commits
llvm-commits at lists.llvm.org
Tue Feb 10 04:10:24 PST 2026
https://github.com/jmmartinez updated https://github.com/llvm/llvm-project/pull/179947
>From 541016c9e51ea743cd82f4baeb78410d21486351 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Juan=20Manuel=20Martinez=20Caama=C3=B1o?=
<jmartinezcaamao at gmail.com>
Date: Thu, 5 Feb 2026 14:09:52 +0100
Subject: [PATCH 1/4] [SPIRV] Add a `SPIRVTypeInst` type with some guardrails
Currently `SPIRVType` is an alias of `MachineInstr`:
```cpp
using SPIRVType = const MachineInstr;
```
Consider the function below from the backend:
```cpp
inline Register getTypeReg(MachineRegisterInfo *MRI, Register OpReg) {
SPIRVType *TypeInst = MRI->getVRegDef(OpReg);
return TypeInst && TypeInst->getOpcode() ==
SPIRV::OpFunctionParameter
? TypeInst->getOperand(1).getReg()
: OpReg;
}
```
How can `TypeInst` be an `OpFunctionParameter` which returns a value?
In all our tests `OpReg` is a value register. Why do we return the type
operand for `OpFunctionParamter` and something else anything else?
This code doesn't make any sense.
To avoid continuing down this path, this patch proposes `SPIRVTypeInst`
which is a wrapper around `MachineInstr` with an extra `assert`.
The idea is to replace the use of `SPIRVType` little-by-little.
---
llvm/lib/Target/SPIRV/SPIRVGlobalRegistry.h | 23 +++++++++++++++++++++
1 file changed, 23 insertions(+)
diff --git a/llvm/lib/Target/SPIRV/SPIRVGlobalRegistry.h b/llvm/lib/Target/SPIRV/SPIRVGlobalRegistry.h
index b7cfa4f6f2ac1..f35a52de98fd4 100644
--- a/llvm/lib/Target/SPIRV/SPIRVGlobalRegistry.h
+++ b/llvm/lib/Target/SPIRV/SPIRVGlobalRegistry.h
@@ -28,6 +28,29 @@ class SPIRVSubtarget;
using SPIRVType = const MachineInstr;
using StructOffsetDecorator = std::function<void(Register)>;
+class SPIRVTypeInst {
+ const MachineInstr *MI;
+
+public:
+ static bool definesATypeRegister(const MachineInstr *MI) {
+ const MachineRegisterInfo &MRI = MI->getMF()->getRegInfo();
+ return MRI.getRegClass(MI->getOperand(0).getReg()) == &SPIRV::TYPERegClass;
+ }
+
+ SPIRVTypeInst(const MachineInstr &MI) : SPIRVTypeInst(&MI) {}
+ SPIRVTypeInst(const MachineInstr *MI) : MI(MI) {
+ // A SPIRV Type whose result is not a type is invalid.
+ assert(definesATypeRegister(MI));
+ }
+
+ const MachineInstr &operator*() const { return *MI; }
+ const MachineInstr *operator->() const { return MI; }
+ operator const MachineInstr *() const { return MI; }
+
+ bool operator==(const SPIRVTypeInst &Other) const { return MI == Other.MI; }
+ bool operator!=(const SPIRVTypeInst &Other) const { return MI != Other.MI; }
+};
+
class SPIRVGlobalRegistry : public SPIRVIRMapping {
// Registers holding values which have types associated with them.
// Initialized upon VReg definition in IRTranslator.
>From fa22d41c811a5bc71073e8f99987c77e963bd8c9 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Juan=20Manuel=20Martinez=20Caama=C3=B1o?=
<jmartinezcaamao at gmail.com>
Date: Thu, 5 Feb 2026 14:37:34 +0100
Subject: [PATCH 2/4] [SPIRV][NFC] Replace `SPIRVType` with `SPIRVTypeInst`
---
llvm/lib/Target/SPIRV/SPIRVISelLowering.cpp | 13 ++++++++-----
1 file changed, 8 insertions(+), 5 deletions(-)
diff --git a/llvm/lib/Target/SPIRV/SPIRVISelLowering.cpp b/llvm/lib/Target/SPIRV/SPIRVISelLowering.cpp
index 7d3711a583e50..b16dae012b10d 100644
--- a/llvm/lib/Target/SPIRV/SPIRVISelLowering.cpp
+++ b/llvm/lib/Target/SPIRV/SPIRVISelLowering.cpp
@@ -39,7 +39,8 @@ SPIRVTargetLowering::SPIRVTargetLowering(const TargetMachine &TM,
// Returns true of the types logically match, as defined in
// https://registry.khronos.org/SPIR-V/specs/unified1/SPIRV.html#OpCopyLogical.
-static bool typesLogicallyMatch(const SPIRVType *Ty1, const SPIRVType *Ty2,
+static bool typesLogicallyMatch(const SPIRVTypeInst Ty1,
+ const SPIRVTypeInst Ty2,
SPIRVGlobalRegistry &GR) {
if (Ty1->getOpcode() != Ty2->getOpcode())
return false;
@@ -52,17 +53,19 @@ static bool typesLogicallyMatch(const SPIRVType *Ty1, const SPIRVType *Ty2,
if (Ty1->getOperand(2).getReg() != Ty2->getOperand(2).getReg())
return false;
- SPIRVType *ElemType1 = GR.getSPIRVTypeForVReg(Ty1->getOperand(1).getReg());
- SPIRVType *ElemType2 = GR.getSPIRVTypeForVReg(Ty2->getOperand(1).getReg());
+ SPIRVTypeInst ElemType1 =
+ GR.getSPIRVTypeForVReg(Ty1->getOperand(1).getReg());
+ SPIRVTypeInst ElemType2 =
+ GR.getSPIRVTypeForVReg(Ty2->getOperand(1).getReg());
return ElemType1 == ElemType2 ||
typesLogicallyMatch(ElemType1, ElemType2, GR);
}
if (Ty1->getOpcode() == SPIRV::OpTypeStruct) {
for (unsigned I = 1; I < Ty1->getNumOperands(); I++) {
- SPIRVType *ElemType1 =
+ SPIRVTypeInst ElemType1 =
GR.getSPIRVTypeForVReg(Ty1->getOperand(I).getReg());
- SPIRVType *ElemType2 =
+ SPIRVTypeInst ElemType2 =
GR.getSPIRVTypeForVReg(Ty2->getOperand(I).getReg());
if (ElemType1 != ElemType2 &&
!typesLogicallyMatch(ElemType1, ElemType2, GR))
>From 5682417691f1c3c298bd5c34d6163ae66f3801c9 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Juan=20Manuel=20Martinez=20Caama=C3=B1o?=
<jmartinezcaamao at gmail.com>
Date: Mon, 9 Feb 2026 13:41:35 +0100
Subject: [PATCH 3/4] [SPIRV] Deprecate SPIRVType
---
llvm/lib/Target/SPIRV/SPIRVGlobalRegistry.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/llvm/lib/Target/SPIRV/SPIRVGlobalRegistry.h b/llvm/lib/Target/SPIRV/SPIRVGlobalRegistry.h
index f35a52de98fd4..6a239812f6375 100644
--- a/llvm/lib/Target/SPIRV/SPIRVGlobalRegistry.h
+++ b/llvm/lib/Target/SPIRV/SPIRVGlobalRegistry.h
@@ -25,7 +25,7 @@
namespace llvm {
class SPIRVSubtarget;
-using SPIRVType = const MachineInstr;
+using SPIRVType [[deprecated]] = const MachineInstr;
using StructOffsetDecorator = std::function<void(Register)>;
class SPIRVTypeInst {
>From a4df742f24fc62d7b0c443c8f33d42b577021768 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Juan=20Manuel=20Martinez=20Caama=C3=B1o?=
<jmartinezcaamao at gmail.com>
Date: Tue, 10 Feb 2026 12:15:42 +0100
Subject: [PATCH 4/4] [SPIRV] Finish SPIRVTypeInst implementation
---
llvm/lib/Target/SPIRV/SPIRVGlobalRegistry.h | 35 ++++++++++++++++++---
1 file changed, 31 insertions(+), 4 deletions(-)
diff --git a/llvm/lib/Target/SPIRV/SPIRVGlobalRegistry.h b/llvm/lib/Target/SPIRV/SPIRVGlobalRegistry.h
index 6a239812f6375..3b58167d6cb40 100644
--- a/llvm/lib/Target/SPIRV/SPIRVGlobalRegistry.h
+++ b/llvm/lib/Target/SPIRV/SPIRVGlobalRegistry.h
@@ -32,15 +32,24 @@ class SPIRVTypeInst {
const MachineInstr *MI;
public:
- static bool definesATypeRegister(const MachineInstr *MI) {
- const MachineRegisterInfo &MRI = MI->getMF()->getRegInfo();
- return MRI.getRegClass(MI->getOperand(0).getReg()) == &SPIRV::TYPERegClass;
+ static bool definesATypeRegister(const MachineInstr &MI) {
+ const MachineRegisterInfo &MRI = MI.getMF()->getRegInfo();
+ return MRI.getRegClass(MI.getOperand(0).getReg()) == &SPIRV::TYPERegClass;
}
SPIRVTypeInst(const MachineInstr &MI) : SPIRVTypeInst(&MI) {}
SPIRVTypeInst(const MachineInstr *MI) : MI(MI) {
// A SPIRV Type whose result is not a type is invalid.
- assert(definesATypeRegister(MI));
+ assert(!MI || definesATypeRegister(*MI));
+ }
+
+ // No need to verify the register since it's already verified by the copied
+ // object.
+ SPIRVTypeInst(const SPIRVTypeInst &Other) : MI(Other.MI) {}
+
+ SPIRVTypeInst &operator=(const SPIRVTypeInst &Other) {
+ MI = Other.MI;
+ return *this;
}
const MachineInstr &operator*() const { return *MI; }
@@ -49,6 +58,24 @@ class SPIRVTypeInst {
bool operator==(const SPIRVTypeInst &Other) const { return MI == Other.MI; }
bool operator!=(const SPIRVTypeInst &Other) const { return MI != Other.MI; }
+
+ bool operator==(const MachineInstr *Other) const { return MI == Other; }
+ bool operator!=(const MachineInstr *Other) const { return MI != Other; }
+
+ operator bool() const { return MI; }
+
+ unsigned getHashValue() const {
+ return DenseMapInfo<const MachineInstr *>::getHashValue(MI);
+ }
+};
+
+template <> struct DenseMapInfo<SPIRVTypeInst> {
+ static SPIRVTypeInst getEmptyKey() { return SPIRVTypeInst(nullptr); }
+ static SPIRVTypeInst getTombstoneKey() { return SPIRVTypeInst(nullptr); }
+ static unsigned getHashValue(SPIRVTypeInst Ty) { return Ty.getHashValue(); }
+ static bool isEqual(SPIRVTypeInst Ty1, SPIRVTypeInst Ty2) {
+ return Ty1 == Ty2;
+ }
};
class SPIRVGlobalRegistry : public SPIRVIRMapping {
More information about the llvm-commits
mailing list