MIR-Canon tests and helper function additions.

[plotfi]

Summary

Need to add a simple test for the canonizer. Also adding some helper functions that a couple new features that another patch will use.

[bogner]

I've commented on both parts, but please keep each review to a single topic. Adding the test is one thing, and adding the helpers should be another review (or better yet, just add them when you use them).

lib/CodeGen/MIRCanonicalizerPass.cpp

102–106

These macros don't expand to expressions - adding semicolons doesn't make much sense.

134

Should this be static?

145–146

I think we usually put raw_string_ostream stuff in its own block to ensure it flushes or something. Please check other uses to see if this is right.

150

Spelling out the type is clearer than auto here, though it might be simpler still to use StringRef::split or StringRef::take_until instead.

152

I'm pretty sure we can do StringInstrMap.insert({Str, II}) and avoid the StringMapEntry typedef here. If not, std::make_pair is probably what we want.

173

Range-for probably cleans this up a bit.

test/CodeGen/MIR/AArch64/mirCanonTest1.mir

14–35

You should be able to remove the whole IR block here, it isn't really relevant to the test. You'll need to add -mtriple to the llc command.

38–67

I think you can get away with removing all of this metadata.

69

You'll need to drop the names from these to get rid of the IR.

99

Without IR this will presumably read STRXui %0, %stack.1, 0 :: (store 8)

[plotfi]

lib/CodeGen/MIRCanonicalizerPass.cpp

145–146

.str() appears to call flush():

raw_ostream.h:478:

std::string& str() {
  flush();
  return OS;
}

173

Going to leave this one out in first commit.

lib/CodeGen/MIRCanonicalizerPass.cpp

 char &llvm::MIRCanonicalizerID = MIRCanonicalizer::ID;
 
 INITIALIZE_PASS_BEGIN(MIRCanonicalizer, "mir-canonicalizer",
-                      "Rename Register Operands Canonically", false, false)
+                      "Rename Register Operands Canonically", false, false);
 
 INITIALIZE_PASS_END(MIRCanonicalizer, "mir-canonicalizer",
-                    "Rename Register Operands Canonically", false, false)
+                    "Rename Register Operands Canonically", false, false);
 
 static std::vector<MachineBasicBlock *> GetRPOList(MachineFunction &MF) {
   ReversePostOrderTraversal<MachineBasicBlock *> RPOT(&*MF.begin());

   return ~0U;
 }
 
-static bool rescheduleCanonically(MachineBasicBlock *MBB) {
+bool rescheduleLexographically(
+  std::vector<MachineInstr*> instructions,
+  MachineBasicBlock *MBB,
+  std::function<MachineBasicBlock::iterator ()> getPos) {
+
+  bool Changed = false;
+  typedef std::pair<std::string, MachineInstr *> StringMapEntry;
+  std::map<std::string, MachineInstr*> StringInstrMap;
+
+  for (auto *II : instructions) {
+    std::string s;
+    raw_string_ostream sstr(s);
+    II->print(sstr);
+
+    // Trim the assignment, or start from the begining in the case of a store.
+    const size_t i = sstr.str().find("=");
+    auto Str = (i == std::string::npos) ? sstr.str() : sstr.str().substr(i);
+
+    StringInstrMap.insert(StringMapEntry(Str, II));
+  }
+
+  for (auto &II : StringInstrMap) {
+
+    DEBUG({
+      dbgs() << "Splicing ";
+      II.second->dump();
+      dbgs() << " right before: ";
+      getPos()->dump();
+    });
+
+    Changed = true;
+    MBB->splice(getPos(), MBB, II.second);
+  }
+
+  return Changed;
+}
+
+static MachineBasicBlock::iterator findMBBI(MachineInstr *MI) {
+  MachineBasicBlock *MBB = MI->getParent();
+  for (auto BBI = MBB->instr_begin(), BBE = MBB->instr_end(); BBI != BBE; ++BBI)
+    if (&*BBI == MI)
+      return BBI;
+  return MBB->instr_end();
+}
+
+static bool rescheduleCanonically(unsigned &pseudoIdempotentInstCount,
+                                  MachineBasicBlock *MBB) {
 
   bool Changed = false;
 

   DEBUG(dbgs() << "\n\n NEW BASIC BLOCK: " << MBB->getName() << "\n\n";);
 
   DEBUG(dbgs() << "MBB Before Scheduling:\n"; MBB->dump(););
-  Changed |= rescheduleCanonically(MBB);
+  unsigned pseudoIdempotentInstCount = 0;
+  Changed |= rescheduleCanonically(pseudoIdempotentInstCount, MBB);
   DEBUG(dbgs() << "MBB After Scheduling:\n"; MBB->dump(););
 
   std::vector<MachineInstr *> Candidates = populateCandidates(MBB);

test/CodeGen/MIR/AArch64/mirCanonTest1.mir

+# RUN: llc -o - -run-pass mir-canonicalizer %s | FileCheck %s
+
+# CHECK: %1363:gpr64common = COPY $x0
+# CHECK: STRXui %1363
+# CHECK: %1366:gpr64 = COPY $x1
+# CHECK: STRXui %1366
+# CHECK: %1369:fpr64 = COPY $d0
+# CHECK: STRDui %1369
+# CHECK: %1372:fpr64 = COPY $d1
+# CHECK: STRDui %1372
+# CHECK: %1375:gpr32 = LDRWui
+# CHECK: $w0 = COPY %1375
+
+--- |
+  target datalayout = "e-m:o-i64:64-i128:128-n32:64-S128"
+  target triple = "arm64-apple-ios11.0.0"
+
+  @IntGlob = common global double 0.000000e+00, align 8
+
+  define i32 @Proc8(double* %arg, [51 x double]* %arg1, double %arg2, double %arg3) #0 {
+  bb:
+    %tmp = alloca i32, align 4
+    %tmp4 = alloca double*, align 8
+    %tmp5 = alloca [51 x double]*, align 8
+    %tmp6 = alloca double, align 8
+    %tmp7 = alloca double, align 8
+    %tmp8 = alloca double, align 8
+    %tmp9 = alloca double, align 8
+    %tmp86 = load i32, i32* %tmp, align 4
+    ret i32 %tmp86
+  }
+
+  attributes #0 = { noinline nounwind ssp "correctly-rounded-divide-sqrt-fp-math"="false" "disable-tail-calls"="false" "less-precise-fpmad"="false" "no-frame-pointer-elim"="true" "no-frame-pointer-elim-non-leaf" "no-infs-fp-math"="false" "no-jump-tables"="false" "no-nans-fp-math"="false" "no-signed-zeros-fp-math"="false" "no-trapping-math"="false" "stack-protector-buffer-size"="8" "target-cpu"="cyclone" "target-features"="+crypto,+neon,+zcm,+zcz" "unsafe-fp-math"="false" "use-soft-float"="false" }
+
+...
+---
+name:            Proc8
+alignment:       2
+exposesReturnsTwice: false
+legalized:       false
+regBankSelected: false
+selected:        false
+tracksRegLiveness: true
+registers:
+liveins:
+  - { reg: '$x0', virtual-reg: '%0' }
+  - { reg: '$x1', virtual-reg: '%1' }
+  - { reg: '$d0', virtual-reg: '%2' }
+  - { reg: '$d1', virtual-reg: '%3' }
+frameInfo:
+  isFrameAddressTaken: false
+  isReturnAddressTaken: false
+  hasStackMap:     false
+  hasPatchPoint:   false
+  stackSize:       0
+  offsetAdjustment: 0
+  maxAlignment:    8
+  adjustsStack:    false
+  hasCalls:        false
+  stackProtector:  ''
+  maxCallFrameSize: 4294967295
+  hasOpaqueSPAdjustment: false
+  hasVAStart:      false
+  hasMustTailInVarArgFunc: false
+  savePoint:       ''
+  restorePoint:    ''
+fixedStack:
+stack:
+  - { id: 0, name: tmp, type: default, offset: 0, size: 4, alignment: 4,
+      stack-id: 0, callee-saved-register: '', callee-saved-restored: true,
+      local-offset: -4, di-variable: '', di-expression: '', di-location: '' }
+  - { id: 1, name: tmp4, type: default, offset: 0, size: 8, alignment: 8,
+      stack-id: 0, callee-saved-register: '', callee-saved-restored: true,
+      local-offset: -16, di-variable: '', di-expression: '', di-location: '' }
+  - { id: 2, name: tmp5, type: default, offset: 0, size: 8, alignment: 8,
+      stack-id: 0, callee-saved-register: '', callee-saved-restored: true,
+      local-offset: -24, di-variable: '', di-expression: '', di-location: '' }
+  - { id: 3, name: tmp6, type: default, offset: 0, size: 8, alignment: 8,
+      stack-id: 0, callee-saved-register: '', callee-saved-restored: true,
+      local-offset: -32, di-variable: '', di-expression: '', di-location: '' }
+  - { id: 4, name: tmp7, type: default, offset: 0, size: 8, alignment: 8,
+      stack-id: 0, callee-saved-register: '', callee-saved-restored: true,
+      local-offset: -40, di-variable: '', di-expression: '', di-location: '' }
+  - { id: 5, name: tmp8, type: default, offset: 0, size: 8, alignment: 8,
+      stack-id: 0, callee-saved-register: '', callee-saved-restored: true,
+      local-offset: -48, di-variable: '', di-expression: '', di-location: '' }
+  - { id: 6, name: tmp9, type: default, offset: 0, size: 8, alignment: 8,
+      stack-id: 0, callee-saved-register: '', callee-saved-restored: true,
+      local-offset: -56, di-variable: '', di-expression: '', di-location: '' }
+constants:
+body:             |
+  bb.0.bb:
+    liveins: $x0, $x1, $d0, $d1
+
+    %3:fpr64 = COPY $d1
+    %2:fpr64 = COPY $d0
+    %1:gpr64 = COPY $x1
+    %0:gpr64common = COPY $x0
+    STRXui %0, %stack.1.tmp4, 0 :: (store 8 into %ir.tmp4)
+    STRXui %1, %stack.2.tmp5, 0 :: (store 8 into %ir.tmp5)
+    STRDui %2, %stack.3.tmp6, 0 :: (store 8 into %ir.tmp6)
+    STRDui %3, %stack.4.tmp7, 0 :: (store 8 into %ir.tmp7)
+
+
+    %42:gpr32 = LDRWui %stack.0.tmp, 0 :: (dereferenceable load 4 from %ir.tmp)
+    $w0 = COPY %42
+    RET_ReallyLR implicit $w0
+...