[llvm] [SPIRV] Return success when selecting reads and writes. (PR #122162)
Steven Perron via llvm-commits
llvm-commits at lists.llvm.org
Wed Jan 8 12:07:35 PST 2025
https://github.com/s-perron updated https://github.com/llvm/llvm-project/pull/122162
>From 7c84a9a351e3a904d7b843b73fa6d162d78c34a3 Mon Sep 17 00:00:00 2001
From: Steven Perron <stevenperron at google.com>
Date: Tue, 3 Dec 2024 12:42:40 -0500
Subject: [PATCH 1/2] [SPIRV] Return success when selecting reads and writes.
The function `selectImageWriteIntrinsic` and `selectReadImageIntrinsic`
are void functions. The should return true if they succeed, and false
otherwise. This commit updates the code to do this.
---
.../Target/SPIRV/SPIRVInstructionSelector.cpp | 72 +++++++++++--------
1 file changed, 41 insertions(+), 31 deletions(-)
diff --git a/llvm/lib/Target/SPIRV/SPIRVInstructionSelector.cpp b/llvm/lib/Target/SPIRV/SPIRVInstructionSelector.cpp
index 28c9b81db51f51..b7b32dd0d626c6 100644
--- a/llvm/lib/Target/SPIRV/SPIRVInstructionSelector.cpp
+++ b/llvm/lib/Target/SPIRV/SPIRVInstructionSelector.cpp
@@ -274,10 +274,10 @@ class SPIRVInstructionSelector : public InstructionSelector {
bool selectHandleFromBinding(Register &ResVReg, const SPIRVType *ResType,
MachineInstr &I) const;
- void selectReadImageIntrinsic(Register &ResVReg, const SPIRVType *ResType,
+ bool selectReadImageIntrinsic(Register &ResVReg, const SPIRVType *ResType,
MachineInstr &I) const;
- void selectImageWriteIntrinsic(MachineInstr &I) const;
+ bool selectImageWriteIntrinsic(MachineInstr &I) const;
// Utilities
std::pair<Register, bool>
@@ -305,7 +305,7 @@ class SPIRVInstructionSelector : public InstructionSelector {
Register IndexReg, bool IsNonUniform,
MachineIRBuilder MIRBuilder) const;
SPIRVType *widenTypeToVec4(const SPIRVType *Type, MachineInstr &I) const;
- void extractSubvector(Register &ResVReg, const SPIRVType *ResType,
+ bool extractSubvector(Register &ResVReg, const SPIRVType *ResType,
Register &ReadReg, MachineInstr &InsertionPoint) const;
bool BuildCOPY(Register DestReg, Register SrcReg, MachineInstr &I) const;
bool loadVec3BuiltinInputID(SPIRV::BuiltIn::BuiltIn BuiltInValue,
@@ -3002,12 +3002,10 @@ bool SPIRVInstructionSelector::selectIntrinsic(Register ResVReg,
return selectHandleFromBinding(ResVReg, ResType, I);
}
case Intrinsic::spv_resource_store_typedbuffer: {
- selectImageWriteIntrinsic(I);
- return true;
+ return selectImageWriteIntrinsic(I);
}
case Intrinsic::spv_resource_load_typedbuffer: {
- selectReadImageIntrinsic(ResVReg, ResType, I);
- return true;
+ return selectReadImageIntrinsic(ResVReg, ResType, I);
}
case Intrinsic::spv_discard: {
return selectDiscard(ResVReg, ResType, I);
@@ -3049,7 +3047,7 @@ bool SPIRVInstructionSelector::selectHandleFromBinding(Register &ResVReg,
.constrainAllUses(TII, TRI, RBI);
}
-void SPIRVInstructionSelector::selectReadImageIntrinsic(
+bool SPIRVInstructionSelector::selectReadImageIntrinsic(
Register &ResVReg, const SPIRVType *ResType, MachineInstr &I) const {
// If the load of the image is in a different basic block, then
@@ -3064,35 +3062,40 @@ void SPIRVInstructionSelector::selectReadImageIntrinsic(
uint64_t ResultSize = GR.getScalarOrVectorComponentCount(ResType);
if (ResultSize == 4) {
- BuildMI(*I.getParent(), I, I.getDebugLoc(), TII.get(SPIRV::OpImageRead))
+ return BuildMI(*I.getParent(), I, I.getDebugLoc(),
+ TII.get(SPIRV::OpImageRead))
.addDef(ResVReg)
.addUse(GR.getSPIRVTypeID(ResType))
.addUse(ImageReg)
- .addUse(I.getOperand(3).getReg());
- return;
+ .addUse(I.getOperand(3).getReg())
+ .constrainAllUses(TII, TRI, RBI);
}
SPIRVType *ReadType = widenTypeToVec4(ResType, I);
Register ReadReg = MRI->createVirtualRegister(GR.getRegClass(ReadType));
- BuildMI(*I.getParent(), I, I.getDebugLoc(), TII.get(SPIRV::OpImageRead))
- .addDef(ReadReg)
- .addUse(GR.getSPIRVTypeID(ReadType))
- .addUse(ImageReg)
- .addUse(I.getOperand(3).getReg());
+ bool Succeed =
+ BuildMI(*I.getParent(), I, I.getDebugLoc(), TII.get(SPIRV::OpImageRead))
+ .addDef(ReadReg)
+ .addUse(GR.getSPIRVTypeID(ReadType))
+ .addUse(ImageReg)
+ .addUse(I.getOperand(3).getReg())
+ .constrainAllUses(TII, TRI, RBI);
+ if (!Succeed)
+ return false;
if (ResultSize == 1) {
- BuildMI(*I.getParent(), I, I.getDebugLoc(),
- TII.get(SPIRV::OpCompositeExtract))
+ return BuildMI(*I.getParent(), I, I.getDebugLoc(),
+ TII.get(SPIRV::OpCompositeExtract))
.addDef(ResVReg)
.addUse(GR.getSPIRVTypeID(ResType))
.addUse(ReadReg)
- .addImm(0);
- return;
+ .addImm(0)
+ .constrainAllUses(TII, TRI, RBI);
}
- extractSubvector(ResVReg, ResType, ReadReg, I);
+ return extractSubvector(ResVReg, ResType, ReadReg, I);
}
-void SPIRVInstructionSelector::extractSubvector(
+bool SPIRVInstructionSelector::extractSubvector(
Register &ResVReg, const SPIRVType *ResType, Register &ReadReg,
MachineInstr &InsertionPoint) const {
SPIRVType *InputType = GR.getResultType(ReadReg);
@@ -3108,12 +3111,16 @@ void SPIRVInstructionSelector::extractSubvector(
const TargetRegisterClass *ScalarRegClass = GR.getRegClass(ScalarType);
for (uint64_t I = 0; I < ResultSize; I++) {
Register ComponentReg = MRI->createVirtualRegister(ScalarRegClass);
- BuildMI(*InsertionPoint.getParent(), InsertionPoint,
- InsertionPoint.getDebugLoc(), TII.get(SPIRV::OpCompositeExtract))
- .addDef(ComponentReg)
- .addUse(ScalarType->getOperand(0).getReg())
- .addUse(ReadReg)
- .addImm(I);
+ bool Succeed = BuildMI(*InsertionPoint.getParent(), InsertionPoint,
+ InsertionPoint.getDebugLoc(),
+ TII.get(SPIRV::OpCompositeExtract))
+ .addDef(ComponentReg)
+ .addUse(ScalarType->getOperand(0).getReg())
+ .addUse(ReadReg)
+ .addImm(I)
+ .constrainAllUses(TII, TRI, RBI);
+ if (!Succeed)
+ return false;
ComponentRegisters.emplace_back(ComponentReg);
}
@@ -3125,9 +3132,10 @@ void SPIRVInstructionSelector::extractSubvector(
for (Register ComponentReg : ComponentRegisters)
MIB.addUse(ComponentReg);
+ return MIB.constrainAllUses(TII, TRI, RBI);
}
-void SPIRVInstructionSelector::selectImageWriteIntrinsic(
+bool SPIRVInstructionSelector::selectImageWriteIntrinsic(
MachineInstr &I) const {
// If the load of the image is in a different basic block, then
// this will generate invalid code. A proper solution is to move
@@ -3142,10 +3150,12 @@ void SPIRVInstructionSelector::selectImageWriteIntrinsic(
Register DataReg = I.getOperand(3).getReg();
assert(GR.getResultType(DataReg)->getOpcode() == SPIRV::OpTypeVector);
assert(GR.getScalarOrVectorComponentCount(GR.getResultType(DataReg)) == 4);
- BuildMI(*I.getParent(), I, I.getDebugLoc(), TII.get(SPIRV::OpImageWrite))
+ return BuildMI(*I.getParent(), I, I.getDebugLoc(),
+ TII.get(SPIRV::OpImageWrite))
.addUse(ImageReg)
.addUse(CoordinateReg)
- .addUse(DataReg);
+ .addUse(DataReg)
+ .constrainAllUses(TII, TRI, RBI);
}
Register SPIRVInstructionSelector::buildPointerToResource(
>From d0477bdd9364b09c93093154c1ae1d398a8d3bd3 Mon Sep 17 00:00:00 2001
From: Steven Perron <stevenperron at google.com>
Date: Wed, 8 Jan 2025 15:06:58 -0500
Subject: [PATCH 2/2] Modify reference parameters to value parameters.
---
llvm/lib/Target/SPIRV/SPIRVInstructionSelector.cpp | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/llvm/lib/Target/SPIRV/SPIRVInstructionSelector.cpp b/llvm/lib/Target/SPIRV/SPIRVInstructionSelector.cpp
index b7b32dd0d626c6..7d413b8186666c 100644
--- a/llvm/lib/Target/SPIRV/SPIRVInstructionSelector.cpp
+++ b/llvm/lib/Target/SPIRV/SPIRVInstructionSelector.cpp
@@ -271,10 +271,10 @@ class SPIRVInstructionSelector : public InstructionSelector {
bool selectUnmergeValues(MachineInstr &I) const;
- bool selectHandleFromBinding(Register &ResVReg, const SPIRVType *ResType,
+ bool selectHandleFromBinding(Register ResVReg, const SPIRVType *ResType,
MachineInstr &I) const;
- bool selectReadImageIntrinsic(Register &ResVReg, const SPIRVType *ResType,
+ bool selectReadImageIntrinsic(Register ResVReg, const SPIRVType *ResType,
MachineInstr &I) const;
bool selectImageWriteIntrinsic(MachineInstr &I) const;
@@ -3021,7 +3021,7 @@ bool SPIRVInstructionSelector::selectIntrinsic(Register ResVReg,
return true;
}
-bool SPIRVInstructionSelector::selectHandleFromBinding(Register &ResVReg,
+bool SPIRVInstructionSelector::selectHandleFromBinding(Register ResVReg,
const SPIRVType *ResType,
MachineInstr &I) const {
@@ -3048,7 +3048,7 @@ bool SPIRVInstructionSelector::selectHandleFromBinding(Register &ResVReg,
}
bool SPIRVInstructionSelector::selectReadImageIntrinsic(
- Register &ResVReg, const SPIRVType *ResType, MachineInstr &I) const {
+ Register ResVReg, const SPIRVType *ResType, MachineInstr &I) const {
// If the load of the image is in a different basic block, then
// this will generate invalid code. A proper solution is to move
More information about the llvm-commits
mailing list