[LLVMdev] [llvm-commits] [PATCH][Review request] tablegen: extend list fields

[Sean Silva]

I think the "let ... += ..." seems reasonable, but the "+Base" when
inheriting seems like it is extremely error prone. Correct me if I'm wrong,
but the "+Base" basically makes *all* list fields extend, which I think is
a bit dangerous, since it's a "global" effect where all the inherited
fields are appended, meaning that it is not effectively composable; this
works OK in the case you gave, but I have doubts about its general
usefulness (not that that would be out of place in TableGen, unfortunately
:( ).

--Sean Silva

On Fri, Sep 14, 2012 at 5:41 PM, Akira Hatanaka <ahatanak at gmail.com> wrote:

> Please take a look at the attached patch.
>
> I updated the BNF and added comments in the code.
>
>
> On Wed, Sep 12, 2012 at 4:58 PM, Sean Silva <silvas at purdue.edu> wrote:
>
>> If you are changing the syntax, please update the BNF in the comments.
>>
>> --Sean Silva
>>
>> On Wed, Sep 12, 2012 at 6:16 PM, Akira Hatanaka <ahatanak at gmail.com>
>> wrote:
>> > The attached patch adds a construct that enables extending the base
>> class'
>> > lists rather than completely overwriting them.
>> > The patch hasn't gone through extensive testing yet (other than running
>> make
>> > check).
>> >
>> > The lists can be extended either with a "+=" operator in a let
>> statement or
>> > placing a '"+" in front of a superclass:
>> >
>> > - Example 1:
>> >
>> > def D0 : C1 {
>> >   let Predicates += [P2]; // Append P2 to C1's Predicates
>> > }
>> >
>> > - Example 2:
>> >
>> > def D0 : C1, +AddP1;
>> >
>> >
>> > Using a real example, MOVi16 (in ARMInstrInfo.td) which is defined as
>> >
>> > def MOVi16 : AI1<0b1000, (outs GPR:$Rd), (ins imm0_65535_expr:$imm),
>> >                  DPFrm, IIC_iMOVi,
>> >                  "movw", "\t$Rd, $imm",
>> >                  [(set GPR:$Rd, imm0_65535:$imm)]>,
>> >                  Requires<[IsARM, HasV6T2]>, UnaryDP {
>> >
>> >
>> > can be rewritten to this:
>> >
>> > class PredHasV6T2 {
>> >   list<Predicate> Predicates = [HasV6T2];
>> > }
>> >
>> > def MOVi16 : AI1<0b1000, (outs GPR:$Rd), (ins imm0_65535_expr:$imm),
>> >                  DPFrm, IIC_iMOVi,
>> >                  "movw", "\t$Rd, $imm",
>> >                  [(set GPR:$Rd, imm0_65535:$imm)]>,
>> >                  +PredHasV6T2, UnaryDP {
>> >
>> > Since AI1 already has "IsARM" in its predicate list, MOVi16 just has to
>> > extend the list with [HasV6T2].
>> >
>> >
>> > Also, I changed the last statement of ListRecTy::convertValue (in
>> > lib/TableGen/Record.cpp) to pass the element type of the list rather
>> than
>> > the list type.
>> > I would appreciate if someone could take a look at this code and tell me
>> > whether this is correct.
>> >
>> >
>> > _______________________________________________
>> > llvm-commits mailing list
>> > llvm-commits at cs.uiuc.edu
>> > http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>> >
>>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20120914/8afa8d69/attachment.html>