[llvm-commits] PATCH: Tablegen - Add HWEncoding field to Register class and generate getHWEncoding() function

[Tom Stellard]

Hi Jim,

Thanks for the feedback.

On Mon, May 07, 2012 at 03:59:51PM -0700, Jim Grosbach wrote:
> Hi Tom,
> 
> Good idea. ARM could use this, too.
> 
> The function shouldn't go into the <Target>GenRegisterInfo class, though. That's derived from TargetRegisterInfo, which is part of the Target layer. The code layers that will want to access this information, at least for the most part, are part of the MC layer and can't access that. In particular, the <Target>MCCodeEmitter bits want this and can't hook into Target classes. It could be made a lookup table and hooked in MCRegisterInfo as a generic function w/ TableGen'erated data payload, perhaps. That would require teaching the MCCodeEmitter how to get access to the MCRegisterInfo instance. There was a recent patch to do that for the InstPrinter that would provide an example of the sorts of things needed for that. TargetRegisterInfo derives from MCRegisterInfo, so anything in the Target layer that actually wants the encoding bits for a register could still get to them that way.
>

I'm not really familiar with the MC layer, so I need to spend some time
learning about it before I can come up with a new patch.  Is this more
or less what you are suggesting that I do:

1. Have tablegen generate a lookup table for registers and their
   encodings.
2. Add an extra parameter to InitMCRegisterInfo for the new lookup table
3. Add a function to the MCRegisterInfo class called getHWEncoding() (or
   something similar) that uses the new lookup table.
4. Teach MCCodeEmitter how to access MCRegisterInfo.

> As a side note, if the R600 target is referencing anything in the Target layer from anything MC related, you'll want to fix that. That will absolutely be a pre-requisite before it could be mainlined.
> 

The R600 target doesn't use the MC layer at all and is still using
the regular CodeEmitter. Would switching from CodeEmitter to
MCCodeEmitter also be a requirement for it to be mainlined?  What are
the advantages and disadvantages of using the MCCodeEmitter instead of
the regular CodeEmitter?

Thanks,
Tom


> -Jim
> 
> > diff --git include/llvm/Target/Target.td include/llvm/Target/Target.td
> > index bd959d0..1f2bd47 100644
> > --- include/llvm/Target/Target.td
> > +++ include/llvm/Target/Target.td
> > @@ -96,6 +96,9 @@ class Register<string n, list<string> altNames = []> {
> >    // x86 register AX is covered by its sub-registers AL and AH, but EAX is not
> >    // covered by its sub-register AX.
> >    bit CoveredBySubRegs = 0;
> > +
> > +  // HWEncoding - The target specific hardware encoding for this register.
> > +  bits<16> HWEncoding = 0;
> >  }
> >  
> >  // RegisterWithSubRegs - This can be used to define instances of Register which
> > diff --git utils/TableGen/RegisterInfoEmitter.cpp utils/TableGen/RegisterInfoEmitter.cpp
> > index 5b4a876..92c3fa8 100644
> > --- utils/TableGen/RegisterInfoEmitter.cpp
> > +++ utils/TableGen/RegisterInfoEmitter.cpp
> > @@ -412,6 +412,40 @@ RegisterInfoEmitter::EmitRegMapping(raw_ostream &OS,
> >    }
> >  }
> >  
> > +void
> > +RegisterInfoEmitter::EmitRegHWEncoding(raw_ostream &OS,
> > +                                       const std::vector<CodeGenRegister*> &Regs,
> > +                                       const std::string &ClassName) const {
> > +
> > +  std::map<uint64_t, std::vector<std::string> > CaseMap;
> > +
> > +  for (unsigned i = 0, e = Regs.size(); i != e; ++i) {
> > +    Record *Reg = Regs[i]->TheDef;
> > +    BitsInit *BI = Reg->getValueAsBitsInit("HWEncoding");
> > +    uint64_t Value = 0;
> > +    for (unsigned b = 0, be = BI->getNumBits(); b != be; ++b) {
> > +      if (BitInit *B = dynamic_cast<BitInit*>(BI->getBit(b)))
> > +        Value |= (uint64_t)B->getValue() << b;
> > +    }
> > +    CaseMap[Value].push_back(getQualifiedName(Reg));
> > +  }
> > +  OS << "uint64_t " << ClassName << "::getHWEncoding(" << "unsigned reg) const";
> > +  OS << "\n{\n  switch(reg) {\n";
> > +  OS << "default: llvm_unreachable(\"Unknown Register\");\n";
> > +  std::map<uint64_t, std::vector<std::string> >::iterator CI, CE;
> > +  for (CI = CaseMap.begin(), CE = CaseMap.end(); CI != CE; ++CI) {
> > +    uint64_t HWEncoding = (*CI).first;
> > +    std::vector<std::string> RegNames = (*CI).second;
> > +    for (std::vector<std::string>::iterator RI = RegNames.begin(),
> > +                                          RE = RegNames.end(); RI != RE; ++RI) {
> > +      OS << "  case " << *RI << ":\n";
> > +    }
> > +    OS << "    return " << HWEncoding << ";\n";
> > +  }
> > +
> > +  OS << "  }\n}\n";
> > +}
> > +
> >  // Print a BitVector as a sequence of hex numbers using a little-endian mapping.
> >  // Width is the number of bits per hex number.
> >  static void printBitVectorAsHex(raw_ostream &OS,
> > @@ -689,6 +723,7 @@ RegisterInfoEmitter::runTargetHeader(raw_ostream &OS, CodeGenTarget &Target,
> >       << "  unsigned getRegPressureSetLimit(unsigned Idx) const;\n"
> >       << "  const int *getRegClassPressureSets("
> >       << "const TargetRegisterClass *RC) const;\n"
> > +     << "  uint64_t getHWEncoding(unsigned reg) const;\n"
> >       << "};\n\n";
> >  
> >    ArrayRef<CodeGenRegisterClass*> RegisterClasses = RegBank.getRegClasses();
> > @@ -994,6 +1029,7 @@ RegisterInfoEmitter::runTargetDesc(raw_ostream &OS, CodeGenTarget &Target,
> >    }
> >  
> >    EmitRegUnitPressure(OS, RegBank, ClassName);
> > +  EmitRegHWEncoding(OS, Regs, ClassName);
> >  
> >    // Emit the constructor of the class...
> >    OS << "extern const MCRegisterDesc " << TargetName << "RegDesc[];\n";
> > diff --git utils/TableGen/RegisterInfoEmitter.h utils/TableGen/RegisterInfoEmitter.h
> > index ee9903c..b8dfab2 100644
> > --- utils/TableGen/RegisterInfoEmitter.h
> > +++ utils/TableGen/RegisterInfoEmitter.h
> > @@ -57,6 +57,9 @@ private:
> >  
> >    void EmitRegUnitPressure(raw_ostream &OS, const CodeGenRegBank &RegBank,
> >                             const std::string &ClassName);
> > +  void EmitRegHWEncoding(raw_ostream &OS,
> > +                         const std::vector<CodeGenRegister*> &Regs,
> > +                         const std::string &ClassName) const;
> >  };
> >  
> >  } // End llvm namespace
> 
> On May 4, 2012, at 1:55 PM, Tom Stellard <thomas.stellard at amd.com> wrote:
> 
> > Hi,
> > 
> > The attached patch causes tablegen with the -gen-register-info flag to
> > generate a getHWEncoding function that returns the encoding for the given
> > register.  This is useful for targets like R600 that have hundreds of
> > registers, which makes it very tedious to write a function like this by
> > hand.
> > 
> > Please Review.
> > 
> > Thanks,
> > Tom Stellard
> > <tablegen-gen-reg-hw-encoding.patch>_______________________________________________
> > llvm-commits mailing list
> > llvm-commits at cs.uiuc.edu
> > http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
> 
>