[PATCH] D43916: Named VReg support for MIR

[Puyan Lotfi via Phabricator via llvm-commits]

plotfi marked 2 inline comments as done.
plotfi added inline comments.


================
Comment at: lib/CodeGen/MIRParser/MIParser.cpp:986
+    return error(Twine("unknown register name '") + Name + "'");
+
+  lex();
----------------
thegameg wrote:
> plotfi wrote:
> > thegameg wrote:
> > > I think the code would be much simpler if we do the same thing as parseVirtualRegister here. By that I mean call createIncompleteVirtualRegister, create a VRegInfo object without any reg class, then let the rest of the code parse the reg class and follow the usual path. I also think that you don't need to explicitly parse the reg class in this function since the user will get an error like `Cannot determine class/bank of virtual register` if no reg class is specified at def time anyway. This would allow you to get rid of the other change you have where you conditionally lex things.
> > > 
> > > Please let me know if what I'm saying doesn't make sense in this case as I haven't really tried to implement this.
> > I don't understand this completely, but I think one thing I could do is add a name parameter to createIncompleteVirtualRegister and to getVRegInfo and you're saying parseRegisterClassOrBank will just finish building the vreg for me?
> What I mean by that is:
> * the only way to parse a named virtual register is through `parseRegisterOperand`
> * `parseRegisterOperand` handles all kinds of registers, sub registers, register classes / banks.
> * (1) the only thing `parseVirtualRegister` does is create an empty `VRegInfo` and create an "incomplete" virtual register.
> * (2) the call to `parseRegisterClassOrBank` in `parseRegisterOperand` will continue the parsing.
> 
> This is the part where I am not completely sure:
> * can you do the same as (1), with additional steps as inserting the name into `Names2VRegs` and `MRI::VReg2Name`
> * will re-using (2) work with no additional changes here?
Actually the only reason I created parseVirtualRegister was to use Names2VRegs. Since we use a different sigil for the physregs I think we should use a different table to map the names. 




Repository:
  rL LLVM

https://reviews.llvm.org/D43916