[llvm] [SPIR-V] Improve general validity of emitted code between passes (PR #119202)
Vyacheslav Levytskyy via llvm-commits
llvm-commits at lists.llvm.org
Mon Dec 9 03:50:35 PST 2024
https://github.com/VyacheslavLevytskyy created https://github.com/llvm/llvm-project/pull/119202
This PR improves general validity of emitted code between passes due to generation of `TargetOpcode::PHI` instead of `SPIRV::OpPhi` after Instruction Selection, fixing generation of OpTypePointer instructions and using of proper virtual register classes.
The test suite with expensive checks set ON now has only 18 fails out of 569 total test cases.
>From 906cdbe9571c7b653af78ee157d2867e00bd3929 Mon Sep 17 00:00:00 2001
From: "Levytskyy, Vyacheslav" <vyacheslav.levytskyy at intel.com>
Date: Mon, 9 Dec 2024 03:47:08 -0800
Subject: [PATCH] improve validity of emitted code between passes
---
llvm/lib/Target/SPIRV/SPIRVGlobalRegistry.cpp | 19 ++++++++-------
.../Target/SPIRV/SPIRVInstructionSelector.cpp | 10 ++++++--
llvm/lib/Target/SPIRV/SPIRVModuleAnalysis.cpp | 23 +++++++++++++++++++
llvm/lib/Target/SPIRV/SPIRVPreLegalizer.cpp | 9 ++++++--
llvm/lib/Target/SPIRV/SPIRVTargetMachine.cpp | 7 +-----
.../CodeGen/SPIRV/branching/if-merging.ll | 2 +-
.../CodeGen/SPIRV/branching/if-non-merging.ll | 2 +-
.../SPIRV/branching/switch-range-check.ll | 2 +-
llvm/test/CodeGen/SPIRV/half_no_extension.ll | 7 +++---
llvm/test/CodeGen/SPIRV/keep-tracked-const.ll | 2 +-
llvm/test/CodeGen/SPIRV/phi-insert-point.ll | 2 +-
.../CodeGen/SPIRV/phi-ptrcast-dominate.ll | 2 +-
.../SPIRV/phi-spvintrinsic-dominate.ll | 2 +-
.../OpPhi_ArgumentsPlaceholders.ll | 1 -
14 files changed, 61 insertions(+), 29 deletions(-)
diff --git a/llvm/lib/Target/SPIRV/SPIRVGlobalRegistry.cpp b/llvm/lib/Target/SPIRV/SPIRVGlobalRegistry.cpp
index fabedb5e06c1d5..fb9efbdceafacb 100644
--- a/llvm/lib/Target/SPIRV/SPIRVGlobalRegistry.cpp
+++ b/llvm/lib/Target/SPIRV/SPIRVGlobalRegistry.cpp
@@ -1551,14 +1551,17 @@ SPIRVType *SPIRVGlobalRegistry::getOrCreateSPIRVPointerType(
if (Reg.isValid())
return getSPIRVTypeForVReg(Reg);
// create a new type
- auto MIB = BuildMI(MIRBuilder.getMBB(), MIRBuilder.getInsertPt(),
- MIRBuilder.getDebugLoc(),
- MIRBuilder.getTII().get(SPIRV::OpTypePointer))
- .addDef(createTypeVReg(CurMF->getRegInfo()))
- .addImm(static_cast<uint32_t>(SC))
- .addUse(getSPIRVTypeID(BaseType));
- DT.add(PointerElementType, AddressSpace, CurMF, getSPIRVTypeID(MIB));
- return finishCreatingSPIRVType(LLVMTy, MIB);
+ return createOpType(MIRBuilder, [&](MachineIRBuilder &MIRBuilder) {
+ auto MIB = BuildMI(MIRBuilder.getMBB(), MIRBuilder.getInsertPt(),
+ MIRBuilder.getDebugLoc(),
+ MIRBuilder.getTII().get(SPIRV::OpTypePointer))
+ .addDef(createTypeVReg(CurMF->getRegInfo()))
+ .addImm(static_cast<uint32_t>(SC))
+ .addUse(getSPIRVTypeID(BaseType));
+ DT.add(PointerElementType, AddressSpace, CurMF, getSPIRVTypeID(MIB));
+ finishCreatingSPIRVType(LLVMTy, MIB);
+ return MIB;
+ });
}
SPIRVType *SPIRVGlobalRegistry::getOrCreateSPIRVPointerType(
diff --git a/llvm/lib/Target/SPIRV/SPIRVInstructionSelector.cpp b/llvm/lib/Target/SPIRV/SPIRVInstructionSelector.cpp
index 3a98b74b3d6757..98edf53e721320 100644
--- a/llvm/lib/Target/SPIRV/SPIRVInstructionSelector.cpp
+++ b/llvm/lib/Target/SPIRV/SPIRVInstructionSelector.cpp
@@ -2162,6 +2162,7 @@ bool SPIRVInstructionSelector::selectBuildVector(Register ResVReg,
report_fatal_error(
"There must be at least two constituent operands in a vector");
+ MRI->setRegClass(ResVReg, GR.getRegClass(ResType));
auto MIB = BuildMI(*I.getParent(), I, I.getDebugLoc(),
TII.get(IsConst ? SPIRV::OpConstantComposite
: SPIRV::OpCompositeConstruct))
@@ -2195,6 +2196,7 @@ bool SPIRVInstructionSelector::selectSplatVector(Register ResVReg,
report_fatal_error(
"There must be at least two constituent operands in a vector");
+ MRI->setRegClass(ResVReg, GR.getRegClass(ResType));
auto MIB = BuildMI(*I.getParent(), I, I.getDebugLoc(),
TII.get(IsConst ? SPIRV::OpConstantComposite
: SPIRV::OpCompositeConstruct))
@@ -2701,7 +2703,7 @@ bool SPIRVInstructionSelector::wrapIntoSpecConstantOp(
continue;
}
// Create a new register for the wrapper
- WrapReg = MRI->createVirtualRegister(&SPIRV::iIDRegClass);
+ WrapReg = MRI->createVirtualRegister(GR.getRegClass(OpType));
GR.add(OpDefine, MF, WrapReg);
CompositeArgs.push_back(WrapReg);
// Decorate the wrapper register and generate a new instruction
@@ -2766,6 +2768,7 @@ bool SPIRVInstructionSelector::selectIntrinsic(Register ResVReg,
if (!wrapIntoSpecConstantOp(I, CompositeArgs))
return false;
}
+ MRI->setRegClass(ResVReg, GR.getRegClass(ResType));
auto MIB = BuildMI(BB, I, I.getDebugLoc(), TII.get(Opcode))
.addDef(ResVReg)
.addUse(GR.getSPIRVTypeID(ResType));
@@ -3388,7 +3391,10 @@ bool SPIRVInstructionSelector::selectPhi(Register ResVReg,
MIB.addUse(I.getOperand(i + 0).getReg());
MIB.addMBB(I.getOperand(i + 1).getMBB());
}
- return MIB.constrainAllUses(TII, TRI, RBI);
+ bool Res = MIB.constrainAllUses(TII, TRI, RBI);
+ MIB->setDesc(TII.get(TargetOpcode::PHI));
+ MIB->removeOperand(1);
+ return Res;
}
bool SPIRVInstructionSelector::selectGlobalValue(
diff --git a/llvm/lib/Target/SPIRV/SPIRVModuleAnalysis.cpp b/llvm/lib/Target/SPIRV/SPIRVModuleAnalysis.cpp
index 4ee71c703f90bf..4dea4056799fec 100644
--- a/llvm/lib/Target/SPIRV/SPIRVModuleAnalysis.cpp
+++ b/llvm/lib/Target/SPIRV/SPIRVModuleAnalysis.cpp
@@ -1770,6 +1770,27 @@ static void addMBBNames(const Module &M, const SPIRVInstrInfo &TII,
}
}
+// patching Instruction::PHI to SPIRV::OpPhi
+static void patchPhis(const Module &M, SPIRVGlobalRegistry *GR,
+ const SPIRVInstrInfo &TII, MachineModuleInfo *MMI) {
+ for (auto F = M.begin(), E = M.end(); F != E; ++F) {
+ MachineFunction *MF = MMI->getMachineFunction(*F);
+ if (!MF)
+ continue;
+ for (auto &MBB : *MF) {
+ for (MachineInstr &MI : MBB) {
+ if (MI.getOpcode() != TargetOpcode::PHI)
+ continue;
+ MI.setDesc(TII.get(SPIRV::OpPhi));
+ Register ResTypeReg = GR->getSPIRVTypeID(
+ GR->getSPIRVTypeForVReg(MI.getOperand(0).getReg(), MF));
+ MI.insert(MI.operands_begin() + 1,
+ {MachineOperand::CreateReg(ResTypeReg, false)});
+ }
+ }
+ }
+}
+
struct SPIRV::ModuleAnalysisInfo SPIRVModuleAnalysis::MAI;
void SPIRVModuleAnalysis::getAnalysisUsage(AnalysisUsage &AU) const {
@@ -1788,6 +1809,8 @@ bool SPIRVModuleAnalysis::runOnModule(Module &M) {
setBaseInfo(M);
+ patchPhis(M, GR, *TII, MMI);
+
addMBBNames(M, *TII, MMI, *ST, MAI);
addDecorations(M, *TII, MMI, *ST, MAI);
diff --git a/llvm/lib/Target/SPIRV/SPIRVPreLegalizer.cpp b/llvm/lib/Target/SPIRV/SPIRVPreLegalizer.cpp
index d5299043d9ef2f..ceccf55d1de4df 100644
--- a/llvm/lib/Target/SPIRV/SPIRVPreLegalizer.cpp
+++ b/llvm/lib/Target/SPIRV/SPIRVPreLegalizer.cpp
@@ -466,8 +466,13 @@ void processInstr(MachineInstr &MI, MachineIRBuilder &MIB,
for (auto &Op : MI.operands()) {
if (!Op.isReg() || Op.isDef())
continue;
- auto IdOpInfo = createNewIdReg(nullptr, Op.getReg(), MRI, *GR);
- MIB.buildInstr(IdOpInfo.second).addDef(IdOpInfo.first).addUse(Op.getReg());
+ Register OpReg = Op.getReg();
+ SPIRVType *SpvType = GR->getSPIRVTypeForVReg(OpReg);
+ auto IdOpInfo = createNewIdReg(SpvType, OpReg, MRI, *GR);
+ MIB.buildInstr(IdOpInfo.second).addDef(IdOpInfo.first).addUse(OpReg);
+ const TargetRegisterClass *RC = GR->getRegClass(SpvType);
+ if (RC != MRI.getRegClassOrNull(OpReg))
+ MRI.setRegClass(OpReg, RC);
Op.setReg(IdOpInfo.first);
}
}
diff --git a/llvm/lib/Target/SPIRV/SPIRVTargetMachine.cpp b/llvm/lib/Target/SPIRV/SPIRVTargetMachine.cpp
index 8f38d4b8307da2..d9d0d00cc71e41 100644
--- a/llvm/lib/Target/SPIRV/SPIRVTargetMachine.cpp
+++ b/llvm/lib/Target/SPIRV/SPIRVTargetMachine.cpp
@@ -130,13 +130,8 @@ FunctionPass *SPIRVPassConfig::createTargetRegisterAllocator(bool) {
return nullptr;
}
-// Disable passes that may break CFG.
+// A place to disable passes that may break CFG.
void SPIRVPassConfig::addMachineSSAOptimization() {
- // Some standard passes that optimize machine instructions in SSA form uses
- // MI.isPHI() that doesn't account for OpPhi in SPIR-V and so are able to
- // break the CFG (e.g., MachineSink).
- disablePass(&MachineSinkingID);
-
TargetPassConfig::addMachineSSAOptimization();
}
diff --git a/llvm/test/CodeGen/SPIRV/branching/if-merging.ll b/llvm/test/CodeGen/SPIRV/branching/if-merging.ll
index eb8ca515c58225..18fbe417addf8c 100644
--- a/llvm/test/CodeGen/SPIRV/branching/if-merging.ll
+++ b/llvm/test/CodeGen/SPIRV/branching/if-merging.ll
@@ -1,4 +1,4 @@
-; RUN: llc -O0 -mtriple=spirv32-unknown-unknown %s -o - | FileCheck %s
+; RUN: llc -verify-machineinstrs -O0 -mtriple=spirv32-unknown-unknown %s -o - | FileCheck %s
;; NOTE: This does not check for structured control-flow operations.
diff --git a/llvm/test/CodeGen/SPIRV/branching/if-non-merging.ll b/llvm/test/CodeGen/SPIRV/branching/if-non-merging.ll
index f978c44888e0f3..b664a65a5829c8 100644
--- a/llvm/test/CodeGen/SPIRV/branching/if-non-merging.ll
+++ b/llvm/test/CodeGen/SPIRV/branching/if-non-merging.ll
@@ -1,4 +1,4 @@
-; RUN: llc -O0 -mtriple=spirv32-unknown-unknown %s -o - | FileCheck %s
+; RUN: llc -verify-machineinstrs -O0 -mtriple=spirv32-unknown-unknown %s -o - | FileCheck %s
; CHECK-DAG: [[I32:%.+]] = OpTypeInt 32
; CHECK-DAG: [[BOOL:%.+]] = OpTypeBool
diff --git a/llvm/test/CodeGen/SPIRV/branching/switch-range-check.ll b/llvm/test/CodeGen/SPIRV/branching/switch-range-check.ll
index 9981e0d86eaa34..25c2c8627de4e6 100644
--- a/llvm/test/CodeGen/SPIRV/branching/switch-range-check.ll
+++ b/llvm/test/CodeGen/SPIRV/branching/switch-range-check.ll
@@ -1,4 +1,4 @@
-; RUN: llc -O0 -mtriple=spirv32-unknown-unknown %s -o - | FileCheck %s
+; RUN: llc -verify-machineinstrs -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: OpFunction
diff --git a/llvm/test/CodeGen/SPIRV/half_no_extension.ll b/llvm/test/CodeGen/SPIRV/half_no_extension.ll
index a5b0ec9c92d236..63cecbc7399aec 100644
--- a/llvm/test/CodeGen/SPIRV/half_no_extension.ll
+++ b/llvm/test/CodeGen/SPIRV/half_no_extension.ll
@@ -5,10 +5,11 @@
;; vstorea_half4_rtp( data, 0, f );
;; }
-; RUN: llc -O0 -mtriple=spirv64-unknown-unknown %s -o - | FileCheck %s --check-prefix=CHECK-SPIRV
+; RUN: llc -verify-machineinstrs -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-SPIRV: OpCapability Float16Buffer
-; CHECK-SPIRV-NOT: OpCapability Float16
+; CHECK-DAG: OpCapability Float16Buffer
+; CHECK-DAG: OpCapability Float16
define spir_kernel void @test(<4 x float> addrspace(1)* %p, half addrspace(1)* %f) {
entry:
diff --git a/llvm/test/CodeGen/SPIRV/keep-tracked-const.ll b/llvm/test/CodeGen/SPIRV/keep-tracked-const.ll
index 0c25d8a67aca05..0dc86233f8e160 100644
--- a/llvm/test/CodeGen/SPIRV/keep-tracked-const.ll
+++ b/llvm/test/CodeGen/SPIRV/keep-tracked-const.ll
@@ -1,6 +1,6 @@
; This test case ensures that cleaning of temporary constants doesn't purge tracked ones.
-; RUN: llc -O0 -mtriple=spirv64-unknown-unknown %s -o - | FileCheck %s --check-prefix=CHECK-SPIRV
+; RUN: llc -verify-machineinstrs -O0 -mtriple=spirv64-unknown-unknown %s -o - | FileCheck %s --check-prefix=CHECK-SPIRV
; RUN: %if spirv-tools %{ llc -O0 -mtriple=spirv64-unknown-unknown %s -o - -filetype=obj | spirv-val %}
; CHECK-SPIRV: %[[#Int:]] = OpTypeInt 32 0
diff --git a/llvm/test/CodeGen/SPIRV/phi-insert-point.ll b/llvm/test/CodeGen/SPIRV/phi-insert-point.ll
index 41cc514f4dbae3..70d121cdf4b3ac 100644
--- a/llvm/test/CodeGen/SPIRV/phi-insert-point.ll
+++ b/llvm/test/CodeGen/SPIRV/phi-insert-point.ll
@@ -2,7 +2,7 @@
; operand are inserted at the correct positions, and don't break rules of
; instruction domination and PHI nodes grouping at top of basic block.
-; RUN: llc -O0 -mtriple=spirv64-unknown-unknown %s -o - | FileCheck %s
+; RUN: llc -verify-machineinstrs -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: OpName %[[#Foo:]] "foo"
diff --git a/llvm/test/CodeGen/SPIRV/phi-ptrcast-dominate.ll b/llvm/test/CodeGen/SPIRV/phi-ptrcast-dominate.ll
index 102eb76356fe32..bc090ce55fbecd 100644
--- a/llvm/test/CodeGen/SPIRV/phi-ptrcast-dominate.ll
+++ b/llvm/test/CodeGen/SPIRV/phi-ptrcast-dominate.ll
@@ -3,7 +3,7 @@
; positions, and don't break rules of instruction domination and PHI nodes
; grouping at top of basic block.
-; RUN: llc -O0 -mtriple=spirv64-unknown-unknown %s -o - | FileCheck %s
+; RUN: llc -verify-machineinstrs -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 %}
diff --git a/llvm/test/CodeGen/SPIRV/phi-spvintrinsic-dominate.ll b/llvm/test/CodeGen/SPIRV/phi-spvintrinsic-dominate.ll
index 471ab03ed89f65..45831ce97263d2 100644
--- a/llvm/test/CodeGen/SPIRV/phi-spvintrinsic-dominate.ll
+++ b/llvm/test/CodeGen/SPIRV/phi-spvintrinsic-dominate.ll
@@ -3,7 +3,7 @@
; positions, and don't break rules of instruction domination and PHI nodes
; grouping at top of basic block.
-; RUN: llc -O0 -mtriple=spirv64-unknown-unknown %s -o - | FileCheck %s
+; RUN: llc -verify-machineinstrs -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: OpFunction
diff --git a/llvm/test/CodeGen/SPIRV/transcoding/OpPhi_ArgumentsPlaceholders.ll b/llvm/test/CodeGen/SPIRV/transcoding/OpPhi_ArgumentsPlaceholders.ll
index ee5596ed38b1b7..63b517c8054942 100644
--- a/llvm/test/CodeGen/SPIRV/transcoding/OpPhi_ArgumentsPlaceholders.ll
+++ b/llvm/test/CodeGen/SPIRV/transcoding/OpPhi_ArgumentsPlaceholders.ll
@@ -14,7 +14,6 @@
; RUN: llc -verify-machineinstrs -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 %}
-; XFAIL: *
%struct.Node = type { %struct.Node.0 addrspace(1)* }
%struct.Node.0 = type opaque
More information about the llvm-commits
mailing list