[MIRVRegNamerUtils] Fix for BasicBlock numbering parameter.

[plotfi]

Summary

A comment I posted in D70210 about setting the BB Number was missed.
This patch sets the BB Number based on RPO order when calling renameVRegs is called.

TODO: Will add a test with 2+ BBs.

[plotfi]

Will land on Monday when holiday is over. I don't want to be that guy.

llvm/lib/CodeGen/MIRCanonicalizerPass.cpp

 
 #define DEBUG_TYPE "mir-canonicalizer"
 
 static cl::opt<unsigned>
     CanonicalizeFunctionNumber("canon-nth-function", cl::Hidden, cl::init(~0u),
                                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 {
 public:
   static char ID;
   MIRCanonicalizer() : MachineFunctionPass(ID) {}
 
   StringRef getPassName() const override {
for (auto &MO : MI.operands()) {
       }
     }
   }
 
   return Changed;
 }
 
 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";
              MBB->dump(););
   Changed |= propagateLocalCopies(MBB);
   LLVM_DEBUG(dbgs() << "MBB After Canonical Copy Propagation:\n"; MBB->dump(););
 
   LLVM_DEBUG(dbgs() << "MBB Before Scheduling:\n"; MBB->dump(););
   unsigned IdempotentInstCount = 0;
   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();
              dbgs() << "\n";);
   LLVM_DEBUG(
       dbgs() << "\n\n================================================\n\n");
   return Changed;
 }
bool MIRCanonicalizer::runOnMachineFunction(MachineFunction &MF) {
   LLVM_DEBUG(
       dbgs() << "\n\n  NEW MACHINE FUNCTION: " << MF.getName() << "  \n\n";
       dbgs() << "\n\n================================================\n\n";
       dbgs() << "Total Basic Blocks: " << RPOList.size() << "\n";
       for (auto MBB
            : 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

public:
   bool runOnMachineFunction(MachineFunction &MF) override {
     bool Changed = false;
 
     if (MF.empty())
       return Changed;
 
     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;
   }
 };
 
 } // end anonymous namespace
 
 char MIRNamer::ID;
 
 char &llvm::MIRNamerID = MIRNamer::ID;
 
 INITIALIZE_PASS_BEGIN(MIRNamer, "mir-namer", "Rename Register Operands", false,
                       false)
 
 INITIALIZE_PASS_END(MIRNamer, "mir-namer", "Rename Register Operands", false,
                     false)

llvm/lib/CodeGen/MIRVRegNamerUtils.h

unsigned createVirtualRegisterWithName(unsigned VReg,
                                          const std::string &Name);
   /// Linearly traverse the MachineBasicBlock and rename each instruction's
   /// vreg definition based on the semantics of the instruction.
   /// Names are as follows bb<BBNum>_hash_[0-9]+
   bool renameInstsInMBB(MachineBasicBlock *MBB);
 
   /// 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; }
 };
 
 } // namespace llvm
 
 #endif

llvm/test/CodeGen/MIR/X86/mir-canon-hash-bb.mir

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