[llvm] fdc6f4b - [llvm] Fixing MIRVRegNamerUtils to properly handle 2+ MachineBasicBlocks.
Puyan Lotfi via llvm-commits
llvm-commits at lists.llvm.org
Wed Dec 4 15:36:38 PST 2019
Author: Puyan Lotfi
Date: 2019-12-04T18:36:08-05:00
New Revision: fdc6f4b97b0e7378d6a3bd45458e7bed206d9305
URL: https://github.com/llvm/llvm-project/commit/fdc6f4b97b0e7378d6a3bd45458e7bed206d9305
DIFF: https://github.com/llvm/llvm-project/commit/fdc6f4b97b0e7378d6a3bd45458e7bed206d9305.diff
LOG: [llvm] Fixing MIRVRegNamerUtils to properly handle 2+ MachineBasicBlocks.
An interplay of code from D70210, along with code from the
Value-Numbering-esque hash-based namer from D70210, as well as some
crusty code from the original MIR-Canon code lead to multiple causes of
failure when canonicalizing or renaming vregs for MIR with multiple
basic blocks. This patch fixes those issues while deleting some no
longer needed code and adding a nice diamond test case to boot.
Differential Revision: https://reviews.llvm.org/D70478
Added:
llvm/test/CodeGen/MIR/X86/mir-canon-hash-bb.mir
Modified:
llvm/lib/CodeGen/MIRCanonicalizerPass.cpp
llvm/lib/CodeGen/MIRNamerPass.cpp
llvm/lib/CodeGen/MIRVRegNamerUtils.h
Removed:
################################################################################
diff --git a/llvm/lib/CodeGen/MIRCanonicalizerPass.cpp b/llvm/lib/CodeGen/MIRCanonicalizerPass.cpp
index 7a57cd6890d1..5ef907b88315 100644
--- a/llvm/lib/CodeGen/MIRCanonicalizerPass.cpp
+++ b/llvm/lib/CodeGen/MIRCanonicalizerPass.cpp
@@ -49,10 +49,6 @@ static cl::opt<unsigned>
cl::value_desc("N"),
cl::desc("Function number to canonicalize."));
-static cl::opt<unsigned> CanonicalizeBasicBlockNumber(
- "canon-nth-basicblock", cl::Hidden, cl::init(~0u), cl::value_desc("N"),
- cl::desc("BasicBlock number to canonicalize."));
-
namespace {
class MIRCanonicalizer : public MachineFunctionPass {
@@ -374,24 +370,7 @@ static bool doDefKillClear(MachineBasicBlock *MBB) {
}
static bool runOnBasicBlock(MachineBasicBlock *MBB,
- std::vector<StringRef> &bbNames,
- unsigned &basicBlockNum, VRegRenamer &Renamer) {
-
- if (CanonicalizeBasicBlockNumber != ~0U) {
- if (CanonicalizeBasicBlockNumber != basicBlockNum++)
- return false;
- LLVM_DEBUG(dbgs() << "\n Canonicalizing BasicBlock " << MBB->getName()
- << "\n";);
- }
-
- if (llvm::find(bbNames, MBB->getName()) != bbNames.end()) {
- LLVM_DEBUG({
- dbgs() << "Found potentially duplicate BasicBlocks: " << MBB->getName()
- << "\n";
- });
- return false;
- }
-
+ unsigned BasicBlockNum, VRegRenamer &Renamer) {
LLVM_DEBUG({
dbgs() << "\n\n NEW BASIC BLOCK: " << MBB->getName() << " \n\n";
dbgs() << "\n\n================================================\n\n";
@@ -399,7 +378,6 @@ static bool runOnBasicBlock(MachineBasicBlock *MBB,
bool Changed = false;
- bbNames.push_back(MBB->getName());
LLVM_DEBUG(dbgs() << "\n\n NEW BASIC BLOCK: " << MBB->getName() << "\n\n";);
LLVM_DEBUG(dbgs() << "MBB Before Canonical Copy Propagation:\n";
@@ -412,8 +390,10 @@ static bool runOnBasicBlock(MachineBasicBlock *MBB,
Changed |= rescheduleCanonically(IdempotentInstCount, MBB);
LLVM_DEBUG(dbgs() << "MBB After Scheduling:\n"; MBB->dump(););
- Changed |= Renamer.renameVRegs(MBB);
+ Changed |= Renamer.renameVRegs(MBB, BasicBlockNum);
+ // TODO: Consider dropping this. Dropping kill defs is probably not
+ // semantically sound.
Changed |= doDefKillClear(MBB);
LLVM_DEBUG(dbgs() << "Updated MachineBasicBlock:\n"; MBB->dump();
@@ -445,16 +425,12 @@ bool MIRCanonicalizer::runOnMachineFunction(MachineFunction &MF) {
: RPOList) { dbgs() << MBB->getName() << "\n"; } dbgs()
<< "\n\n================================================\n\n";);
- std::vector<StringRef> BBNames;
-
unsigned BBNum = 0;
-
bool Changed = false;
-
MachineRegisterInfo &MRI = MF.getRegInfo();
VRegRenamer Renamer(MRI);
for (auto MBB : RPOList)
- Changed |= runOnBasicBlock(MBB, BBNames, BBNum, Renamer);
+ Changed |= runOnBasicBlock(MBB, BBNum++, Renamer);
return Changed;
}
diff --git a/llvm/lib/CodeGen/MIRNamerPass.cpp b/llvm/lib/CodeGen/MIRNamerPass.cpp
index 62d0f2e52c7d..9f61dd9ef243 100644
--- a/llvm/lib/CodeGen/MIRNamerPass.cpp
+++ b/llvm/lib/CodeGen/MIRNamerPass.cpp
@@ -57,9 +57,10 @@ class MIRNamer : public MachineFunctionPass {
VRegRenamer Renamer(MF.getRegInfo());
+ unsigned BBIndex = 0;
ReversePostOrderTraversal<MachineBasicBlock *> RPOT(&*MF.begin());
for (auto &MBB : RPOT)
- Changed |= Renamer.renameVRegs(MBB);
+ Changed |= Renamer.renameVRegs(MBB, BBIndex++);
return Changed;
}
diff --git a/llvm/lib/CodeGen/MIRVRegNamerUtils.h b/llvm/lib/CodeGen/MIRVRegNamerUtils.h
index ebe309757f27..8e76bfa2bbd4 100644
--- a/llvm/lib/CodeGen/MIRVRegNamerUtils.h
+++ b/llvm/lib/CodeGen/MIRVRegNamerUtils.h
@@ -84,7 +84,7 @@ class VRegRenamer {
/// Same as the above, but sets a BBNum depending on BB traversal that
/// will be used as prefix for the vreg names.
- bool renameVRegs(MachineBasicBlock *MBB, unsigned BBNum = 0);
+ bool renameVRegs(MachineBasicBlock *MBB, unsigned BBNum);
unsigned getCurrentBBNumber() const { return CurrentBBNumber; }
};
diff --git a/llvm/test/CodeGen/MIR/X86/mir-canon-hash-bb.mir b/llvm/test/CodeGen/MIR/X86/mir-canon-hash-bb.mir
new file mode 100644
index 000000000000..94c69f1be36a
--- /dev/null
+++ b/llvm/test/CodeGen/MIR/X86/mir-canon-hash-bb.mir
@@ -0,0 +1,61 @@
+# RUN: llc -run-pass mir-namer -x mir -verify-machineinstrs %s -o - | FileCheck %s
+# RUN: llc -run-pass mir-canonicalizer -x mir -verify-machineinstrs %s -o - | FileCheck %s
+--- |
+ target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
+ target triple = "x86_64-unknown-linux-gnu"
+ define i32 @_Z1fi(i32 %arg) {
+ %tmp = alloca i32, align 4
+ %tmp1 = alloca i32, align 4
+ ret i32 %arg
+ }
+
+...
+---
+name: _Z1fi
+registers:
+ - { id: 0, class: _, preferred-register: '' }
+ - { id: 1, class: _, preferred-register: '' }
+ - { id: 2, class: _, preferred-register: '' }
+ - { id: 3, class: _, preferred-register: '' }
+ - { id: 4, class: _, preferred-register: '' }
+ - { id: 5, class: _, preferred-register: '' }
+ - { id: 6, class: _, preferred-register: '' }
+ - { id: 7, class: _, preferred-register: '' }
+ - { id: 8, class: _, preferred-register: '' }
+stack:
+ - { id: 0, name: tmp, type: default, offset: 0, size: 4, alignment: 4 }
+ - { id: 1, name: tmp1, type: default, offset: 0, size: 4, alignment: 4 }
+body: |
+ bb.0:
+ %tmp0:_(s32) = COPY $edi
+ %tmp1:_(s32) = G_CONSTANT i32 0
+ %tmp5:_(p0) = G_FRAME_INDEX %stack.0.tmp
+ %tmp6:_(p0) = G_FRAME_INDEX %stack.1.tmp1
+ G_STORE %tmp0(s32), %tmp5(p0) :: (store 4 into %ir.tmp)
+ %tmp7:_(s32) = G_LOAD %tmp5(p0) :: (load 4 from %ir.tmp)
+ %tmp8:_(s1) = G_ICMP intpred(ne), %tmp7(s32), %tmp1
+ G_BRCOND %tmp8(s1), %bb.1
+ G_BR %bb.2
+
+ ; CHECK: bb.1:
+ ; CHECK: %bb2_{{[0-9]+}}__1:_(s32) = G_CONSTANT
+ bb.1:
+ %tmp4:_(s32) = G_CONSTANT i32 1
+ G_STORE %tmp4(s32), %tmp6(p0) :: (store 4 into %ir.tmp1)
+ G_BR %bb.3
+
+
+ ; CHECK: bb.2:
+ ; CHECK: %bb1_{{[0-9]+}}__1:_(s32) = G_CONSTANT
+ bb.2:
+ %tmp3:_(s32) = G_CONSTANT i32 2
+ G_STORE %tmp3(s32), %tmp6(p0) :: (store 4 into %ir.tmp1)
+
+ ; CHECK: bb.3:
+ ; CHECK: %bb3_{{[0-9]+}}__1:_(s32) = G_LOAD
+ bb.3:
+ %tmp9:_(s32) = G_LOAD %tmp6(p0) :: (load 4 from %ir.tmp1)
+ $eax = COPY %tmp9(s32)
+ RET 0, implicit $eax
+
+...
More information about the llvm-commits
mailing list