[PATCH] D22397: MIRParser: Rewrite register info initialization; mostly NFC

[Quentin Colombet via llvm-commits]

qcolombet added inline comments.

================
Comment at: lib/CodeGen/MIRParser/MIParser.cpp:52
@@ +51,3 @@
+    for (unsigned I = MRI.getNumVirtRegs(); I < VRegIndex+1; ++I)
+      MRI.createIncompleteVirtualRegister();
+  }
----------------
Could we maintain a map of "virtual ID” to actual vreg number instead of creating a bunch of useless ones?
Although, that’s probably cheap, I don’t like the idea of a bunch of undef registers in the middle.

Also, that should reduce the number of incomplete virtual registers, since we would create registers only at use/def points and we will know the register class there for the most part.

================
Comment at: lib/CodeGen/MIRParser/MIParser.h:39
@@ +38,3 @@
+  } Kind = UNKNOWN;
+  bool Explicit = false; ///< VReg was explicitely specified in the .mir file
+  bool Used = false;
----------------
explicitly

================
Comment at: lib/CodeGen/MachineFunctionAnalysis.cpp:58
@@ -54,1 +57,3 @@
+    }
+  }
   return false;
----------------
Unrelated to the patch (+ I pushed a fix for that yesterday ;))

================
Comment at: test/CodeGen/MIR/X86/undefined-virtual-register.mir:22
@@ -21,3 +21,3 @@
     %0 = COPY %edi
-    ; CHECK: [[@LINE+1]]:17: use of undefined virtual register '%10'
-    %eax = COPY %10
+    ; CHECK: Cannot deduce type of virtual register 1 in function 'test'
+    %eax = COPY %1
----------------
For that specific case, I would prefer a different message. Like %1 never defined.
For cases where the register is properly defined, but we can’t deduce the type with the use/def (like here with copy), I believe we shouldn’t call that a type. We should use register class/bank.

What I am saying is that we should reserve the word “type” for instruction types like sN, unsized, etc.

================
Comment at: test/CodeGen/MIR/X86/unused-virtual-register.mir:14
@@ -13,2 +13,3 @@
 ---
+# CHECK: Unused virtual register 0 in function 'test'
 name:            test
----------------
I would get rid of that test. I basically don’t care if we don’t use dense numbering.


Repository:
  rL LLVM

https://reviews.llvm.org/D22397