[PATCH] D10981: MIR Serialization: serialize the virtual register definitions.

[Duncan P. N. Exon Smith]

> On 2015-Jul-09, at 13:25, Alex L <arphaman at gmail.com> wrote:
> 
> 
> 
> 2015-07-09 13:14 GMT-07:00 Duncan P. N. Exon Smith <dexonsmith at apple.com>:
> 
> > On 2015-Jul-09, at 12:59, Alex Lorenz <arphaman at gmail.com> wrote:
> >
> > arphaman updated this revision to Diff 29375.
> > arphaman added a comment.
> >
> > I've rebased this patch on ToT.
> >
> >
> > Repository:
> >  rL LLVM
> >
> > http://reviews.llvm.org/D10981
> >
> > Files:
> >  include/llvm/CodeGen/MIRYamlMapping.h
> >  lib/CodeGen/MIRParser/MIRParser.cpp
> >  lib/CodeGen/MIRPrinter.cpp
> >  test/CodeGen/MIR/X86/undefined-register-class.mir
> >  test/CodeGen/MIR/X86/virtual-registers.mir
> >
> > <D10981.29375.patch>
> 
> > Index: lib/CodeGen/MIRParser/MIRParser.cpp
> > ===================================================================
> > --- lib/CodeGen/MIRParser/MIRParser.cpp
> > +++ lib/CodeGen/MIRParser/MIRParser.cpp
> > @@ -315,14 +326,27 @@
> >  }
> >
> >  bool MIRParserImpl::initializeRegisterInfo(
> > -    MachineRegisterInfo &RegInfo, const yaml::MachineFunction &YamlMF) {
> > +    const MachineFunction &MF, MachineRegisterInfo &RegInfo,
> > +    const yaml::MachineFunction &YamlMF) {
> >    assert(RegInfo.isSSA());
> >    if (!YamlMF.IsSSA)
> >      RegInfo.leaveSSA();
> >    assert(RegInfo.tracksLiveness());
> >    if (!YamlMF.TracksRegLiveness)
> >      RegInfo.invalidateLiveness();
> >    RegInfo.enableSubRegLiveness(YamlMF.TracksSubRegLiveness);
> > +
> > +  // Parse the virtual register information.
> > +  for (const auto &VReg : YamlMF.VirtualRegisters) {
> > +    const auto *RC = getRegClass(MF, VReg.Class.Value);
> > +    if (!RC)
> > +      return error(VReg.Class.SourceRange.Start,
> > +                   Twine("use of undefined register class '") +
> > +                       VReg.Class.Value + "'");
> > +    // TODO: create the mapping from IDs to registers so that the virtual
> > +    // register references can be parsed corretly.
> 
> s/corretly/correctly/
> 
> Thanks, I missed this typo.
>  
> 
> I'm not following this TODO.  How is that different from the mapping
> used in `getRegClass()`?
> 
> The mapping in getRegClass maps from strings to register classes, this TODO mapping has to map from
> register IDs as specified in the source to register numbers. 
> 
> The IDs can be arbitrary numbers, they don't have to correspond to the actual virtual register numbers
> used internally. The IDs are used for references as well, so when we are parsing the references, we
> have to map from IDs to the virtual register numbers, that's why we need the mapping.
> 
> My follow up patch http://reviews.llvm.org/D11005 (MIR Serialization: serialize virtual register machine operands.)
> has an example of this:
> 
> ---
> # CHECK:      registers:
> # CHECK-NEXT:   - { id: 0, class: gr32 }
> # CHECK-NEXT:   - { id: 1, class: gr32 }
> # CHECK-NEXT:   - { id: 2, class: gr32 }
> registers:
>   - { id: 2, class: gr32 }
>   - { id: 0, class: gr32 }
>   - { id: 10, class: gr32 }
> body:
>   - id:          0
>     name:        entry
>     # CHECK:      %0 = COPY %edi
>     # CHECK-NEXT: %1 = SUB32ri8 %0, 10
>     instructions:
>       - '%2 = COPY %edi'
>       - '%0 = SUB32ri8 %2, 10, implicit-def %eflags'
> ...
> 
> Notice how the printed register IDs are different than the ones we parsed - that's because the printed IDs use the virtual register numbers, but the in this case the parsed IDs are just arbitrary numbers set by the user.
> 

Ah, I get it.  Thanks.

LGTM!

>  
> 
> > +    RegInfo.createVirtualRegister(RC);
> > +  }
> >    return false;
> >  }
> >
> > @@ -392,6 +416,26 @@
> >                        Error.getFixIts());
> >  }
> >
> > +void MIRParserImpl::initNames2RegClasses(const MachineFunction &MF) {
> > +  if (!Names2RegClasses.empty())
> > +    return;
> > +  const TargetRegisterInfo *TRI = MF.getSubtarget().getRegisterInfo();
> > +  for (unsigned I = 0, E = TRI->getNumRegClasses(); I < E; ++I) {
> > +    const auto *RC = TRI->getRegClass(I);
> > +    Names2RegClasses.insert(
> > +        std::make_pair(StringRef(TRI->getRegClassName(RC)).lower(), RC));
> > +  }
> > +}
> > +
> > +const TargetRegisterClass *MIRParserImpl::getRegClass(const MachineFunction &MF,
> > +                                                      StringRef Name) {
> > +  initNames2RegClasses(MF);
> > +  auto RegClassInfo = Names2RegClasses.find(Name);
> > +  if (RegClassInfo == Names2RegClasses.end())
> > +    return nullptr;
> > +  return RegClassInfo->getValue();
> > +}
> > +
> >  MIRParser::MIRParser(std::unique_ptr<MIRParserImpl> Impl)
> >      : Impl(std::move(Impl)) {}
> >