[mir-canon][NFCi] Adding opt arg to enable vreg renaming only mode.

[plotfi]

Summary

Similar to instnamer for llvm IR. I think it could be nice to have more readable string names. Might eventually add an option for a user defined string prefix for the vreg names instead of "namedVReg1234"

[compnerd]

This is not a NFCI change - it is adding new functionality. Does it make sense perhaps to have this be a separate pass?

[paquette]

+1 for a separate pass. If we're going for an equivalent of -instnamer, I think it makes sense to keep the mode of use consistent.

I can imagine someone being thrown off by having to use

-run-pass mir-canonicalizer -canon-vregs-namer=1

versus something like

-run-pass=mirnamer

if they're used to just having to run -instnamer.

[plotfi]

Sounds good.

[paquette]

llvm/test/CodeGen/MIR/AArch64/mirCanonCopyCopyProp.mir

1–2

Even though this test is pretty small, I think it would be better to have an explicit separate test for this.

(Then maybe you can use llvm/utils/update_mir_test_checks.py too, which is a nice time-saver. Not sure if it would work on this though?)

Also, is there any particular reason you don't pass -verify-machineinstrs on these tests? IIRC it always kicks in on debug builds, so it's nice to just always have it to prevent bot failures.

llvm/test/CodeGen/MIR/AArch64/mirCanonVRegnamer.mir

7–12

I don't think you need the IR if you pass in -mtriple aarch64-unknown-unknown to the llc line.

[paquette]

llvm/test/CodeGen/MIR/AArch64/mirCanonIdempotent.mir

11–19

I think this should be split out into a separate test as well.

Also, in the separate test, it looks like you can just axe everything beyond the FMOVDi since that's all that's tested anyway.

(Is there any particular reason this test is so long in the first place?)

[plotfi]

llvm/test/CodeGen/MIR/AArch64/mirCanonCopyCopyProp.mir

1–2

actually, if I am going to do a new pass then year a separate test makes more sense.

I dont think there is a good reason the verifier isnt enabled here. I will add it.

llvm/test/CodeGen/MIR/AArch64/mirCanonIdempotent.mir

11–19

The reason it's so big I dont think is a very good one. I think I was just looking for some MIR code that had a ton of instructions like .* = FMOVDi 20 that could be moved to the top and sorted alphabetically.

llvm/test/CodeGen/MIR/AArch64/mirCanonVRegnamer.mir

7–12

Noted.

llvm/lib/CodeGen/MIRCanonicalizerPass.cpp

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."));
 
+static cl::opt<bool> CanonicalizeVRegNamesOnly(
+    "canon-vregs-namer", cl::Hidden, cl::init(false),
+    cl::desc("Disables all mir-canon features aside from vreg renaming: "
+             "equivalent of -instnamer for mir."));
+
 namespace {
 
 class MIRCanonicalizer : public MachineFunctionPass {
 public:
   static char ID;
   MIRCanonicalizer() : MachineFunctionPass(ID) {}
 
   StringRef getPassName() const override {
for (auto I = VRegRenameMap.begin(), E = VRegRenameMap.end(); I != E; ++I) {
     for (auto &MO : MRI.reg_operands(VReg)) {
       RenameMOs.push_back(&MO);
     }
 
     for (auto *MO : RenameMOs) {
       Changed = true;
       MO->setReg(Rename);
 
-      if (!MO->isDef())
+      if (!CanonicalizeVRegNamesOnly && !MO->isDef())
         MO->setIsKill(false);
     }
   }
 
   return Changed;
 }
 
 static bool doDefKillClear(MachineBasicBlock *MBB) {
static bool runOnBasicBlock(MachineBasicBlock *MBB,
 
   bool Changed = false;
   MachineFunction &MF = *MBB->getParent();
   MachineRegisterInfo &MRI = MF.getRegInfo();
 
   bbNames.push_back(MBB->getName());
   LLVM_DEBUG(dbgs() << "\n\n NEW BASIC BLOCK: " << MBB->getName() << "\n\n";);
 
+  unsigned IdempotentInstCount = 0;
+  if (!CanonicalizeVRegNamesOnly) {
     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 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(););
+  }
 
   std::vector<MachineInstr *> Candidates = populateCandidates(MBB);
   std::vector<MachineInstr *> VisitedMIs;
   llvm::copy(Candidates, std::back_inserter(VisitedMIs));
 
   std::vector<TypedVReg> VRegs;
   for (auto candidate : Candidates) {
     VRegs.push_back(TypedVReg(RSE_NewCandidate));
static bool runOnBasicBlock(MachineBasicBlock *MBB,
   // If we have populated no vregs to rename then bail.
   // The rest of this function does the vreg remaping.
   if (VRegs.size() == 0)
     return Changed;
 
   auto VRegRenameMap = GetVRegRenameMap(VRegs, renamedInOtherBB, MRI, NVC);
   Changed |= doVRegRenaming(renamedInOtherBB, VRegRenameMap, MRI);
 
-  // Here we renumber the def vregs for the idempotent instructions from the top
-  // of the MachineBasicBlock so that they are named in the order that we sorted
-  // them alphabetically. Eventually we wont need SkipVRegs because we will use
-  // named vregs instead.
+  if (!CanonicalizeVRegNamesOnly) {
+    // Here we renumber the def vregs for the idempotent instructions from the
+    // top of the MachineBasicBlock so that they are named in the order that we
+    // sorted them alphabetically. Eventually we wont need SkipVRegs because we
+    // will use named vregs instead.
     if (IdempotentInstCount)
       NVC.SkipVRegs();
 
     auto MII = MBB->begin();
     for (unsigned i = 0; i < IdempotentInstCount && MII != MBB->end(); ++i) {
       MachineInstr &MI = *MII++;
       Changed = true;
       Register vRegToRename = MI.getOperand(0).getReg();
       auto Rename = NVC.createVirtualRegister(vRegToRename);
 
       std::vector<MachineOperand *> RenameMOs;
       for (auto &MO : MRI.reg_operands(vRegToRename)) {
         RenameMOs.push_back(&MO);
       }
 
       for (auto *MO : RenameMOs) {
         MO->setReg(Rename);
       }
     }
 
     Changed |= doDefKillClear(MBB);
+  }
 
   LLVM_DEBUG(dbgs() << "Updated MachineBasicBlock:\n"; MBB->dump();
              dbgs() << "\n";);
   LLVM_DEBUG(
       dbgs() << "\n\n================================================\n\n");
   return Changed;
 }
 

llvm/test/CodeGen/MIR/AArch64/mirCanonCopyCopyProp.mir

-# RUN: llc -mtriple=arm64-apple-ios11.0.0 -o - -run-pass mir-canonicalizer %s | FileCheck %s
+# RUN: llc -mtriple=arm64-apple-ios11.0.0 -o - -run-pass mir-canonicalizer %s | FileCheck --check-prefixes=CHECK-FULL %s
+# RUN: llc -mtriple=arm64-apple-ios11.0.0 -o - -run-pass mir-canonicalizer -canon-vregs-namer=1 %s | FileCheck --check-prefixes=CHECK-NAMER %s
+
+
 
 ...
 ---
 name: Proc8
 stack:
   - { id: 0, type: default, offset: 0, size: 4, alignment: 4,
       stack-id: default, callee-saved-register: '', callee-saved-restored: true,
       local-offset: -4, debug-info-variable: '', debug-info-expression: '',
- { id: 6, type: default, offset: 0, size: 8, alignment: 8,
       debug-info-location: '' }
 constants:
 body: |
   bb.0:
     liveins: $x0, $x1, $d0, $d1
 
     %42:gpr32 = LDRWui %stack.0, 0 :: (dereferenceable load 8)
 
-    ;CHECK: %namedVReg1352:gpr32 = LDRWui %stack.0, 0 :: (dereferenceable load 8)
-    ;CHECK: $w0 = COPY %namedVReg1352
-    ;CHECK: RET_ReallyLR implicit $w0
+    ;CHECK-FULL: %namedVReg1352:gpr32 = LDRWui %stack.0, 0 :: (dereferenceable load 8)
+    ;CHECK-FULL-NEXT: $w0 = COPY %namedVReg1352
+    ;CHECK-FULL-NEXT: RET_ReallyLR implicit $w0
+
+    ;CHECK-NAMER: %namedVReg1355:gpr32 = LDRWui %stack.0, 0 :: (dereferenceable load 8)
+    ;CHECK-NAMER-NEXT: %namedVReg1354:gpr32 = COPY %namedVReg1355
+    ;CHECK-NAMER-NEXT: %namedVReg1353:gpr32 = COPY %namedVReg1354
+    ;CHECK-NAMER-NEXT: %namedVReg1352:gpr32 = COPY %namedVReg1353
+    ;CHECK-NAMER-NEXT: $w0 = COPY %namedVReg1352
+    ;CHECK-NAMER-NEXT: RET_ReallyLR implicit $w0
 
     %vreg1234:gpr32 = COPY %42
     %vreg1235:gpr32 = COPY %vreg1234
     %vreg1236:gpr32 = COPY %vreg1235
     $w0 = COPY %vreg1236
     RET_ReallyLR implicit $w0
 
 ...

llvm/test/CodeGen/MIR/AArch64/mirCanonIdempotent.mir

-# RUN: llc -mtriple=arm64-apple-ios11.0.0 -o - -run-pass mir-canonicalizer %s | FileCheck %s
+# RUN: llc -mtriple=arm64-apple-ios11.0.0 -o - -run-pass mir-canonicalizer %s | FileCheck --check-prefixes=CHECK-FULL %s
+# RUN: llc -mtriple=arm64-apple-ios11.0.0 -o - -run-pass mir-canonicalizer -canon-vregs-namer=1 %s | FileCheck --check-prefixes=CHECK-NAMER %s
 # These Idempotent instructions are sorted alphabetically (based on after the '=')
-# CHECK: %namedVReg4352:gpr64 = MOVi64imm 4617315517961601024
-# CHECK: %namedVReg4353:gpr32 = MOVi32imm 408
-# CHECK: %namedVReg4354:gpr32 = MOVi32imm 408
-# CHECK: %namedVReg4355:gpr64all = IMPLICIT_DEF
-# CHECK: %namedVReg4356:fpr64 = FMOVDi 20
-# CHECK: %namedVReg4357:fpr64 = FMOVDi 112
+# CHECK-FULL: %namedVReg4352:gpr64 = MOVi64imm 4617315517961601024
+# CHECK-FULL-NEXT: %namedVReg4353:gpr32 = MOVi32imm 408
+# CHECK-FULL-NEXT: %namedVReg4354:gpr32 = MOVi32imm 408
+# CHECK-FULL-NEXT: %namedVReg4355:gpr64all = IMPLICIT_DEF
+# CHECK-FULL-NEXT: %namedVReg4356:fpr64 = FMOVDi 20
+# CHECK-FULL-NEXT: %namedVReg4357:fpr64 = FMOVDi 112
+
+# CHECK-NAMER: %namedVReg1361:fpr64 = COPY $d1
+# CHECK-NAMER-NEXT: %namedVReg1358:fpr64 = COPY $d0
+# CHECK-NAMER-NEXT: %namedVReg1355:gpr64 = COPY $x1
+# CHECK-NAMER-NEXT: %namedVReg1352:gpr64common = COPY $x0
+# CHECK-NAMER-NEXT: STRXui %namedVReg1352, %stack.1, 0 :: (store 8)
+# CHECK-NAMER-NEXT: STRXui %namedVReg1355, %stack.2, 0 :: (store 8)
+# CHECK-NAMER-NEXT: STRDui %namedVReg1358, %stack.3, 0 :: (store 8)
+# CHECK-NAMER-NEXT: STRDui %namedVReg1361, %stack.4, 0 :: (store 8)
+# CHECK-NAMER-NEXT: %namedVReg1366:fpr64 = FMOVDi 20
+
 ...
 ---
 name: Proc8
 stack:
   - { id: 0, type: default, offset: 0, size: 4, alignment: 4,
       stack-id: default, callee-saved-register: '', callee-saved-restored: true,
       local-offset: -4, debug-info-variable: '', debug-info-expression: '',
       debug-info-location: '' }

llvm/test/CodeGen/MIR/AArch64/mirCanonVRegnamer.mir

+# RUN: llc -run-pass mir-canonicalizer -o - -canon-vregs-namer=1 %s | FileCheck %s
+
+# CHECK: %namedVReg1353:_(p0) = COPY $d0
+# CHECK-NEXT: %namedVReg1352:_(<4 x s32>) = COPY $q0
+# CHECK-NEXT: G_STORE %namedVReg1352(<4 x s32>), %namedVReg1353(p0) :: (store 16)
+
+--- |
+  target datalayout = "e-m:e-i8:8:32-i16:16:32-i64:64-i128:128-n32:64-S128"
+  target triple = "aarch64-unknown-linux-gnu"
+  define void @foo() {
+    ret void
+  }
+...
+---
+name: foo
+body:             |
+  bb.0:
+    liveins: $q0, $d0
+    %1:fpr(p0) = COPY $d0
+    %0:fpr(<4 x s32>) = COPY $q0
+    G_STORE %0(<4 x s32>), %1(p0) :: (store 16)
+...
+