[llvm] 8bfb2b6 - [SPIR-V] Remove switch G_ICMP+G_BRCOND+G_BR before ISel
Michal Paszkowski via llvm-commits
llvm-commits at lists.llvm.org
Tue Apr 4 14:54:11 PDT 2023
Author: Michal Paszkowski
Date: 2023-04-04T23:50:07+02:00
New Revision: 8bfb2b6d771ce1aa817b621bc922c1ab92eda034
URL: https://github.com/llvm/llvm-project/commit/8bfb2b6d771ce1aa817b621bc922c1ab92eda034
DIFF: https://github.com/llvm/llvm-project/commit/8bfb2b6d771ce1aa817b621bc922c1ab92eda034.diff
LOG: [SPIR-V] Remove switch G_ICMP+G_BRCOND+G_BR before ISel
IRTranslator lowers switches to [G_SUB] + G_ICMP + G_BRCOND + G_BR
sequences. Since values and destination MBBs are included in the
spv_switch intrinsics, the sequences are not needed for ISel.
Before this commit, the information decoded by these sequences were
added to spv_switch intrinsics in SPIRVPreLegalizer and the sequences
were kept until SPIRVModuleAnalysis where they were marked skipped for
emission.
After this commit, the [G_SUB] + G_ICMP + G_BRCOND + G_BR sequences
and MBBs containing only these MIs are erased in SPIRVPreLegalizer.
Differential Revision: https://reviews.llvm.org/D146923
Added:
llvm/test/CodeGen/SPIRV/branching/OpSwitch32.ll
llvm/test/CodeGen/SPIRV/branching/OpSwitch64.ll
llvm/test/CodeGen/SPIRV/branching/OpSwitchBranches.ll
llvm/test/CodeGen/SPIRV/branching/OpSwitchChar.ll
llvm/test/CodeGen/SPIRV/branching/OpSwitchEmpty.ll
llvm/test/CodeGen/SPIRV/branching/OpSwitchUnreachable.ll
llvm/test/CodeGen/SPIRV/branching/Two_OpSwitch_same_register.ll
Modified:
llvm/lib/Target/SPIRV/SPIRVAsmPrinter.cpp
llvm/lib/Target/SPIRV/SPIRVModuleAnalysis.cpp
llvm/lib/Target/SPIRV/SPIRVModuleAnalysis.h
llvm/lib/Target/SPIRV/SPIRVPreLegalizer.cpp
Removed:
llvm/test/CodeGen/SPIRV/transcoding/OpSwitch32.ll
llvm/test/CodeGen/SPIRV/transcoding/OpSwitch64.ll
llvm/test/CodeGen/SPIRV/transcoding/OpSwitchChar.ll
llvm/test/CodeGen/SPIRV/transcoding/OpSwitchEmpty.ll
llvm/test/CodeGen/SPIRV/transcoding/OpSwitchUnreachable.ll
llvm/test/CodeGen/SPIRV/transcoding/Two_OpSwitch_same_register.ll
################################################################################
diff --git a/llvm/lib/Target/SPIRV/SPIRVAsmPrinter.cpp b/llvm/lib/Target/SPIRV/SPIRVAsmPrinter.cpp
index 2e822a318ea63..d07c0bcdf9af2 100644
--- a/llvm/lib/Target/SPIRV/SPIRVAsmPrinter.cpp
+++ b/llvm/lib/Target/SPIRV/SPIRVAsmPrinter.cpp
@@ -134,8 +134,6 @@ void SPIRVAsmPrinter::emitFunctionBodyEnd() {
}
void SPIRVAsmPrinter::emitOpLabel(const MachineBasicBlock &MBB) {
- if (MAI->MBBsToSkip.contains(&MBB))
- return;
MCInst LabelInst;
LabelInst.setOpcode(SPIRV::OpLabel);
LabelInst.addOperand(MCOperand::createReg(MAI->getOrCreateMBBRegister(MBB)));
@@ -143,6 +141,8 @@ void SPIRVAsmPrinter::emitOpLabel(const MachineBasicBlock &MBB) {
}
void SPIRVAsmPrinter::emitBasicBlockStart(const MachineBasicBlock &MBB) {
+ assert(!MBB.empty() && "MBB is empty!");
+
// If it's the first MBB in MF, it has OpFunction and OpFunctionParameter, so
// OpLabel should be output after them.
if (MBB.getNumber() == MF->front().getNumber()) {
diff --git a/llvm/lib/Target/SPIRV/SPIRVModuleAnalysis.cpp b/llvm/lib/Target/SPIRV/SPIRVModuleAnalysis.cpp
index af48d51a056f9..22746788607b8 100644
--- a/llvm/lib/Target/SPIRV/SPIRVModuleAnalysis.cpp
+++ b/llvm/lib/Target/SPIRV/SPIRVModuleAnalysis.cpp
@@ -386,47 +386,6 @@ void SPIRVModuleAnalysis::numberRegistersGlobally(const Module &M) {
}
}
-// Find OpIEqual and OpBranchConditional instructions originating from
-// OpSwitches, mark them skipped for emission. Also mark MBB skipped if it
-// contains only these instructions.
-static void processSwitches(const Module &M, SPIRV::ModuleAnalysisInfo &MAI,
- MachineModuleInfo *MMI) {
- DenseSet<Register> SwitchRegs;
- for (auto F = M.begin(), E = M.end(); F != E; ++F) {
- MachineFunction *MF = MMI->getMachineFunction(*F);
- if (!MF)
- continue;
- for (MachineBasicBlock &MBB : *MF)
- for (MachineInstr &MI : MBB) {
- if (MAI.getSkipEmission(&MI))
- continue;
- if (MI.getOpcode() == SPIRV::OpSwitch) {
- assert(MI.getOperand(0).isReg());
- SwitchRegs.insert(MI.getOperand(0).getReg());
- }
- if (MI.getOpcode() == SPIRV::OpISubS &&
- SwitchRegs.contains(MI.getOperand(2).getReg())) {
- SwitchRegs.insert(MI.getOperand(0).getReg());
- MAI.setSkipEmission(&MI);
- }
- if ((MI.getOpcode() != SPIRV::OpIEqual &&
- MI.getOpcode() != SPIRV::OpULessThanEqual) ||
- !MI.getOperand(2).isReg() ||
- !SwitchRegs.contains(MI.getOperand(2).getReg()))
- continue;
- Register CmpReg = MI.getOperand(0).getReg();
- MachineInstr *CBr = MI.getNextNode();
- assert(CBr && CBr->getOpcode() == SPIRV::OpBranchConditional &&
- CBr->getOperand(0).isReg() &&
- CBr->getOperand(0).getReg() == CmpReg);
- MAI.setSkipEmission(&MI);
- MAI.setSkipEmission(CBr);
- if (&MBB.front() == &MI && &MBB.back() == CBr)
- MAI.MBBsToSkip.insert(&MBB);
- }
- }
-}
-
// RequirementHandler implementations.
void SPIRV::RequirementHandler::getAndAddRequirements(
SPIRV::OperandCategory::OperandCategory Category, uint32_t i,
@@ -1020,8 +979,6 @@ bool SPIRVModuleAnalysis::runOnModule(Module &M) {
collectReqs(M, MAI, MMI, *ST);
- processSwitches(M, MAI, MMI);
-
// Process type/const/global var/func decl instructions, number their
// destination registers from 0 to N, collect Extensions and Capabilities.
processDefInstrs(M);
diff --git a/llvm/lib/Target/SPIRV/SPIRVModuleAnalysis.h b/llvm/lib/Target/SPIRV/SPIRVModuleAnalysis.h
index a8b659ce3957f..abb6797c52180 100644
--- a/llvm/lib/Target/SPIRV/SPIRVModuleAnalysis.h
+++ b/llvm/lib/Target/SPIRV/SPIRVModuleAnalysis.h
@@ -136,9 +136,6 @@ struct ModuleAnalysisInfo {
// The set contains machine instructions which are necessary
// for correct MIR but will not be emitted in function bodies.
DenseSet<MachineInstr *> InstrsToDelete;
- // The set contains machine basic blocks which are necessary
- // for correct MIR but will not be emitted.
- DenseSet<MachineBasicBlock *> MBBsToSkip;
// The table contains global aliases of local registers for each machine
// function. The aliases are used to substitute local registers during
// code emission.
diff --git a/llvm/lib/Target/SPIRV/SPIRVPreLegalizer.cpp b/llvm/lib/Target/SPIRV/SPIRVPreLegalizer.cpp
index 2818329ece3cb..aad6049b20d8a 100644
--- a/llvm/lib/Target/SPIRV/SPIRVPreLegalizer.cpp
+++ b/llvm/lib/Target/SPIRV/SPIRVPreLegalizer.cpp
@@ -417,19 +417,23 @@ static void processSwitches(MachineFunction &MF, SPIRVGlobalRegistry *GR,
//
// Sometimes (in case of range-compare switches), additional G_SUBs
// instructions are inserted before G_ICMPs. Those need to be additionally
- // processed and require type assignment.
+ // processed.
//
// This function modifies spv_switch call's operands to include destination
// MBBs (default and for each constant value).
- // Note that this function does not remove G_ICMP + G_BRCOND + G_BR sequences,
- // but they are marked by ModuleAnalysis as skipped and as a result AsmPrinter
- // does not output them.
+ //
+ // At the end, the function removes redundant [G_SUB] + G_ICMP + G_BRCOND +
+ // G_BR sequences.
MachineRegisterInfo &MRI = MF.getRegInfo();
- // Collect all MIs relevant to switches across all MBBs in MF.
+ // Collect spv_switches and G_ICMPs across all MBBs in MF.
std::vector<MachineInstr *> RelevantInsts;
+ // Collect redundant MIs from [G_SUB] + G_ICMP + G_BRCOND + G_BR sequences.
+ // After updating spv_switches, the instructions can be removed.
+ std::vector<MachineInstr *> PostUpdateArtifacts;
+
// Temporary set of compare registers. G_SUBs and G_ICMPs relating to
// spv_switch use these registers.
DenseSet<Register> CompareRegs;
@@ -449,23 +453,21 @@ static void processSwitches(MachineFunction &MF, SPIRVGlobalRegistry *GR,
assert(MI.getOperand(0).isReg() && MI.getOperand(1).isReg());
Register Dst = MI.getOperand(0).getReg();
CompareRegs.insert(Dst);
- SPIRVType *Ty = GR->getSPIRVTypeForVReg(MI.getOperand(1).getReg());
- insertAssignInstr(Dst, nullptr, Ty, GR, MIB, MRI);
+ PostUpdateArtifacts.push_back(&MI);
}
// G_ICMPs relating to switches.
if (MI.getOpcode() == TargetOpcode::G_ICMP && MI.getOperand(2).isReg() &&
CompareRegs.contains(MI.getOperand(2).getReg())) {
Register Dst = MI.getOperand(0).getReg();
- // Set type info for destination register of switch's ICMP instruction.
- if (GR->getSPIRVTypeForVReg(Dst) == nullptr) {
- MIB.setInsertPt(*MI.getParent(), MI);
- Type *LLVMTy = IntegerType::get(MF.getFunction().getContext(), 1);
- SPIRVType *SpirvTy = GR->getOrCreateSPIRVType(LLVMTy, MIB);
- MRI.setRegClass(Dst, &SPIRV::IDRegClass);
- GR->assignSPIRVTypeToVReg(SpirvTy, Dst, MIB.getMF());
- }
RelevantInsts.push_back(&MI);
+ PostUpdateArtifacts.push_back(&MI);
+ MachineInstr *CBr = MRI.use_begin(Dst)->getParent();
+ assert(CBr->getOpcode() == SPIRV::G_BRCOND);
+ PostUpdateArtifacts.push_back(CBr);
+ MachineInstr *Br = CBr->getNextNode();
+ assert(Br->getOpcode() == SPIRV::G_BR);
+ PostUpdateArtifacts.push_back(Br);
}
}
}
@@ -509,6 +511,9 @@ static void processSwitches(MachineFunction &MF, SPIRVGlobalRegistry *GR,
// Map switch case Value to target MBB.
ValuesToMBBs[Value] = MBB;
+ // Add target MBB as successor to the switch's MBB.
+ Switch->getParent()->addSuccessor(MBB);
+
// The next MI is always G_BR to either the next case or the default.
MachineInstr *NextMI = CBr->getNextNode();
assert(NextMI->getOpcode() == SPIRV::G_BR &&
@@ -518,8 +523,11 @@ static void processSwitches(MachineFunction &MF, SPIRVGlobalRegistry *GR,
// register.
if (NextMBB->front().getOpcode() != SPIRV::G_ICMP ||
(NextMBB->front().getOperand(2).isReg() &&
- NextMBB->front().getOperand(2).getReg() != CompareReg))
+ NextMBB->front().getOperand(2).getReg() != CompareReg)) {
+ // Set default MBB and add it as successor to the switch's MBB.
DefaultMBB = NextMBB;
+ Switch->getParent()->addSuccessor(DefaultMBB);
+ }
}
// Modify considered spv_switch operands using collected Values and
@@ -546,6 +554,24 @@ static void processSwitches(MachineFunction &MF, SPIRVGlobalRegistry *GR,
Switch->addOperand(MachineOperand::CreateMBB(MBBs[k]));
}
}
+
+ for (MachineInstr *MI : PostUpdateArtifacts) {
+ MachineBasicBlock *ParentMBB = MI->getParent();
+ MI->eraseFromParent();
+ // If G_ICMP + G_BRCOND + G_BR were the only MIs in MBB, erase this MBB. It
+ // can be safely assumed, there are no breaks or phis directing into this
+ // MBB. However, we need to remove this MBB from the CFG graph. MBBs must be
+ // erased top-down.
+ if (ParentMBB->empty()) {
+ while (!ParentMBB->pred_empty())
+ (*ParentMBB->pred_begin())->removeSuccessor(ParentMBB);
+
+ while (!ParentMBB->succ_empty())
+ ParentMBB->removeSuccessor(ParentMBB->succ_begin());
+
+ ParentMBB->eraseFromParent();
+ }
+ }
}
bool SPIRVPreLegalizer::runOnMachineFunction(MachineFunction &MF) {
diff --git a/llvm/test/CodeGen/SPIRV/transcoding/OpSwitch32.ll b/llvm/test/CodeGen/SPIRV/branching/OpSwitch32.ll
similarity index 100%
rename from llvm/test/CodeGen/SPIRV/transcoding/OpSwitch32.ll
rename to llvm/test/CodeGen/SPIRV/branching/OpSwitch32.ll
diff --git a/llvm/test/CodeGen/SPIRV/transcoding/OpSwitch64.ll b/llvm/test/CodeGen/SPIRV/branching/OpSwitch64.ll
similarity index 100%
rename from llvm/test/CodeGen/SPIRV/transcoding/OpSwitch64.ll
rename to llvm/test/CodeGen/SPIRV/branching/OpSwitch64.ll
diff --git a/llvm/test/CodeGen/SPIRV/branching/OpSwitchBranches.ll b/llvm/test/CodeGen/SPIRV/branching/OpSwitchBranches.ll
new file mode 100644
index 0000000000000..145c43c6da32b
--- /dev/null
+++ b/llvm/test/CodeGen/SPIRV/branching/OpSwitchBranches.ll
@@ -0,0 +1,41 @@
+; RUN: llc -O0 -mtriple=spirv32-unknown-unknown %s -o - | FileCheck %s --check-prefix=CHECK-SPIRV
+
+define i32 @test_switch_branches(i32 %a) {
+entry:
+ %alloc = alloca i32
+; CHECK-SPIRV: OpSwitch %[[#]] %[[#DEFAULT:]] 1 %[[#CASE1:]] 2 %[[#CASE2:]] 3 %[[#CASE3:]]
+ switch i32 %a, label %default [
+ i32 1, label %case1
+ i32 2, label %case2
+ i32 3, label %case3
+ ]
+
+; CHECK-SPIRV: %[[#CASE1]] = OpLabel
+case1:
+ store i32 1, ptr %alloc
+; CHECK-SPIRV: OpBranch %[[#END:]]
+ br label %end
+
+; CHECK-SPIRV: %[[#CASE2]] = OpLabel
+case2:
+ store i32 2, ptr %alloc
+; CHECK-SPIRV: OpBranch %[[#END]]
+ br label %end
+
+; CHECK-SPIRV: %[[#CASE3]] = OpLabel
+case3:
+ store i32 3, ptr %alloc
+; CHECK-SPIRV: OpBranch %[[#END]]
+ br label %end
+
+; CHECK-SPIRV: %[[#DEFAULT]] = OpLabel
+default:
+ store i32 0, ptr %alloc
+; CHECK-SPIRV: OpBranch %[[#END]]
+ br label %end
+
+; CHECK-SPIRV: %[[#END]] = OpLabel
+end:
+ %result = load i32, ptr %alloc
+ ret i32 %result
+}
diff --git a/llvm/test/CodeGen/SPIRV/transcoding/OpSwitchChar.ll b/llvm/test/CodeGen/SPIRV/branching/OpSwitchChar.ll
similarity index 100%
rename from llvm/test/CodeGen/SPIRV/transcoding/OpSwitchChar.ll
rename to llvm/test/CodeGen/SPIRV/branching/OpSwitchChar.ll
diff --git a/llvm/test/CodeGen/SPIRV/transcoding/OpSwitchEmpty.ll b/llvm/test/CodeGen/SPIRV/branching/OpSwitchEmpty.ll
similarity index 100%
rename from llvm/test/CodeGen/SPIRV/transcoding/OpSwitchEmpty.ll
rename to llvm/test/CodeGen/SPIRV/branching/OpSwitchEmpty.ll
diff --git a/llvm/test/CodeGen/SPIRV/transcoding/OpSwitchUnreachable.ll b/llvm/test/CodeGen/SPIRV/branching/OpSwitchUnreachable.ll
similarity index 100%
rename from llvm/test/CodeGen/SPIRV/transcoding/OpSwitchUnreachable.ll
rename to llvm/test/CodeGen/SPIRV/branching/OpSwitchUnreachable.ll
diff --git a/llvm/test/CodeGen/SPIRV/transcoding/Two_OpSwitch_same_register.ll b/llvm/test/CodeGen/SPIRV/branching/Two_OpSwitch_same_register.ll
similarity index 100%
rename from llvm/test/CodeGen/SPIRV/transcoding/Two_OpSwitch_same_register.ll
rename to llvm/test/CodeGen/SPIRV/branching/Two_OpSwitch_same_register.ll
More information about the llvm-commits
mailing list