[PATCH] Allow ParserMatchClass attribute on RegisterClass

Hal Finkel hfinkel at anl.gov
Tue Apr 2 06:27:16 PDT 2013


----- Original Message -----
> From: "Ulrich Weigand" <Ulrich.Weigand at de.ibm.com>
> To: "Hal Finkel" <hfinkel at anl.gov>
> Cc: "Commit Messages and Patches for LLVM" <llvm-commits at cs.uiuc.edu>, "Jakob Stoklund Olesen" <stoklund at 2pi.dk>,
> "Jim Grosbach" <grosbach at apple.com>
> Sent: Tuesday, April 2, 2013 7:30:38 AM
> Subject: Re: [PATCH] Allow ParserMatchClass attribute on RegisterClass
> 
> Jim Grosbach <grosbach at apple.com> wrote on 28.03.2013 18:05:42:
> > On Mar 28, 2013, at 10:03 AM, Jakob Stoklund Olesen
> > <stoklund at 2pi.dk>
> wrote:
> >> On Mar 28, 2013, at 9:42 AM, Ulrich Weigand
> >> <ulrich.weigand at de.ibm.com>
> wrote:
> >>> This would be possible without common code changes by using
> RegisterOperand
> >>> classes to specify a ParserMatchClass -- but this means that
> >>> every
> instance
> >>> of a register class in the .td files must be replaced by the
> corresponding
> >>> RegisterOperand class, which seems a bit unnecessary to me, and
> >>> may
> cause
> >>> future maintainability hassles.  I'd propose to instead allow
> specifying a
> >>> ParserMatchClass directly on the RegisterClass itself, which can
> >>> be
> done by
> >>> a minor TableGen change as implemented by this patch.
> >>
> >> Actually, I think I would prefer the RegisterOperand solution.
> >>
> >> As we add new targets, it is becoming clearer that a register
> >> class
> >> is often insufficient for describing register operands, and I
> >> think
> >> it is healthy to separate the concepts.
> >
> > Yes. The RegisterOperand class is designed for exactly this sort of
> > separation of concerns.
> 
> OK, fine with me -- thanks for the comments!
> 
> Hal, would you be OK with changing the PowerPC back-end to
> pervasively
> use RegisterOperand predicates instead of plain RegisterClass?

Yes.

> 
> One question would be the naming; I guess as precedent for operand
> predicates is to be fully lower case, maybe it would make sense to
> create a RegisterOperand corresponding to every RegisterClass, where
> the name of the RegisterOperand is simply the lower-case version of
> the name of the RegisterClass?   E.g.
>    GPRC --> gprc
>    G8RC --> g8rc
>    VRRC --> vrrc
> 
> (This has the additional advantage that a global search-and-replace
> will not mess up 80-column limits etc.)

Sounds good to me.

Thanks again,
Hal

> 
> Thoughts or other suggestions?
> 
> Thanks,
> Ulrich
> 
> 



More information about the llvm-commits mailing list