[PATCH] Target: Add ParserMatchClass field to RegisterClass

[Tom Stellard]

On Wed, Dec 03, 2014 at 06:13:42PM -0600, Hal Finkel wrote:
> ----- Original Message -----
> > From: "Tom Stellard" <thomas.stellard at amd.com>
> > To: llvm-commits at cs.uiuc.edu
> > Cc: "Tom Stellard" <thomas.stellard at amd.com>
> > Sent: Tuesday, December 2, 2014 10:24:27 PM
> > Subject: [PATCH] Target: Add ParserMatchClass field to RegisterClass
> > 
> > This allows targets to specify an AsmOperandClass for registers
> > without having to wrap the RegisterClass in a RegisterOperand.
> 
> Hi Tom,
> 
> LGTM.
> 
> Do you need to add a PrintMethod too? As far as I can tell RegisterOperand exposes two customization points (ParserMatchClass and PrintMethod), and this copies one of the two into RegisterClass itself. I'm curious as to whether we could just get rid of RegisterOperand by moving all of its 'functionality' into RegisterClass.
> 

This was my first thought, but ARM has several different RegisterOperands
which use the same RegisterClass, so I don't think this is will work.

What ARM is doing is actually what I would like to do in R600, which
is have multiple RegisterOperands using the same register class.  The reason
for this is that some operands can take immediates or registers and the range
of legal immediates depends on the instruction, so we have three classes of
operands:

- Reg Only
- Reg or 32-bit Immediate
- Reg or -16 <= Immediate < 64

Right now we implement this by creating three different register classes
with identical sets of registers.  This way, we can query the MCInstrDesc
object for the register class in order to determine whether or not an
immediate is legal for a particular operand.

The reason we don't use RegisterOperands for this is that there is no way
to look up which RegisterOperand was used to define an instruction like
you can with register classes.  Solving this problem would be a good
alternative to this patch, but I couldn't think of a way to do it without
increasing the size of MCInstrDesc or generating a huge switch table.
I'm not sure if either of these would be a desirable solution.  Any
suggestions for other ways to solve this?

-Tom

>  -Hal
> 
> > ---
> >  include/llvm/Target/Target.td        | 4 ++++
> >  utils/TableGen/AsmMatcherEmitter.cpp | 8 ++++++++
> >  2 files changed, 12 insertions(+)
> > 
> > diff --git a/include/llvm/Target/Target.td
> > b/include/llvm/Target/Target.td
> > index 902647e..7f26edc 100644
> > --- a/include/llvm/Target/Target.td
> > +++ b/include/llvm/Target/Target.td
> > @@ -20,6 +20,7 @@ include "llvm/IR/Intrinsics.td"
> >  // description classes.
> >  
> >  class RegisterClass; // Forward def
> > +class AsmOperandClass; // Forward def
> >  
> >  // SubRegIndex - Use instances of SubRegIndex to identify
> >  subregisters.
> >  class SubRegIndex<int size, int offset = 0> {
> > @@ -207,6 +208,9 @@ class RegisterClass<string namespace,
> > list<ValueType> regTypes, int alignment,
> >    // The function should return 0 to select the default order
> >    defined by
> >    // MemberList, 1 to select the first AltOrders entry and so on.
> >    code AltOrderSelect = [{}];
> > +
> > +  // Operand class for this register
> > +  AsmOperandClass ParserMatchClass;
> >  }
> >  
> >  // The memberList in a RegisterClass is a dag of set operations.
> >  TableGen
> > diff --git a/utils/TableGen/AsmMatcherEmitter.cpp
> > b/utils/TableGen/AsmMatcherEmitter.cpp
> > index 891328f..143a358 100644
> > --- a/utils/TableGen/AsmMatcherEmitter.cpp
> > +++ b/utils/TableGen/AsmMatcherEmitter.cpp
> > @@ -1047,6 +1047,14 @@ AsmMatcherInfo::getOperandClass(Record *Rec,
> > int SubOpIdx) {
> >  
> >  
> >    if (Rec->isSubClassOf("RegisterClass")) {
> > +    const RecordVal *R = Rec->getValue("ParserMatchClass");
> > +    if (R && R->getValue()) {
> > +      if (DefInit *DI = dyn_cast<DefInit>(R->getValue())) {
> > +        Record *MatchClass = DI->getDef();
> > +        if (ClassInfo *CI = AsmOperandClasses[MatchClass])
> > +          return CI;
> > +      }
> > +    }
> >      if (ClassInfo *CI = RegisterClassClasses[Rec])
> >        return CI;
> >      PrintFatalError(Rec->getLoc(), "register class has no class
> >      info!");
> > --
> > 1.8.1.5
> > 
> > _______________________________________________
> > llvm-commits mailing list
> > llvm-commits at cs.uiuc.edu
> > http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
> > 
> 
> -- 
> Hal Finkel
> Assistant Computational Scientist
> Leadership Computing Facility
> Argonne National Laboratory
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits