[PATCH] D43916: Named VReg support for MIR

[Puyan Lotfi via Phabricator via llvm-commits]

plotfi added inline comments.


================
Comment at: .gitignore:26
 /build
+/Debug
 
----------------
I will not commit this. Added by accident. 


================
Comment at: include/llvm/CodeGen/MachineRegisterInfo.h:86-88
+  /// VRegNames - StringSet that is used to unique the vreg names.
+  ///
+  /// Used to prevent vreg names from being reused.
----------------
bogner wrote:
> No need to repeat yourself, one sentence will do here.
Ah, sorry. I was just doing the same as the VRegInfo comment. 


================
Comment at: include/llvm/CodeGen/MachineRegisterInfo.h:434
 
+  std::string getVRegName(unsigned Reg) const {
+    return VReg2Name.inBounds(Reg) ? VReg2Name[Reg] : "";
----------------
plotfi wrote:
> thegameg wrote:
> > StringRef might be better here?
> Yeah. I'll update the patch.
Tried stringref, had some bugs in debug mode. Changed it back. 


================
Comment at: lib/CodeGen/MIRParser/MIParser.cpp:1202-1207
+  bool isNamedVreg = Token.is(MIToken::NamedVirtualRegister);
   if (parseRegister(Reg, RegInfo))
     return true;
-  lex();
+  if (isNamedVreg && !Token.is(MIToken::NamedVirtualRegister)) {
+    if (parseRegisterClassOrBank(*RegInfo))
+        return true;
----------------
bogner wrote:
> This doesn't seem right. Parsing a named virtual register should either always or never parse the register class. As is, it seems that it sometimes does, so you have to do this complicated dance here.
In the case of the vreg assignment it always parses it, in the case of a use it doesn't need to. For example:

%foo:gpr32 = ...
$5 = ADD %foo, %foo:gpr32

Hmm, I should probably make it never parse the regclass in the case of use. 


Repository:
  rL LLVM

https://reviews.llvm.org/D43916