[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

Author: plotfi

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,32 +370,14 @@ 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";
   });
 
   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;
 }

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;
   }

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; }
 };

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
+
+...