[PATCH][TableGen] Fully resolve class-instance values before defs in multiclasses

[Tom Stellard]

On Wed, Sep 10, 2014 at 05:56:57PM -0700, Adam Nemet wrote:
> By class-instance values I mean 'Class<Arg>' in 'Class<Arg>.Field' or in
> 'Other<Class<Arg>>' (syntactically a SimpleValue).  This is to differentiate
> from unnamed/anonymous record definitions (syntactically an ObjectBody) which
> are not affected by this change.
> 
> Consider the testcase:
> 
>     class Struct<int i> {
>       int I = !shl(i, 1);
>       int J = !shl(I, 1);
>     }
> 
>     class Class<Struct s> {
>         int Class_J = s.J;
>     }
> 
>     multiclass MultiClass<int i> {
>       def Def : Class<Struct<i>>;
>     }
> 
>     defm Defm : MultiClass<2>;
> 
> Before this fix, DefmDef.Class_J yields !shl(I, 1) instead of 8.
> 
> This is the sequence of events.  We start with this:
> 
>     multiclass MultiClass<int i> {
>       def Def : Class<Struct<i>>;
>     }
> 
> During ParseDef the anonymous object for the class-instance value is created:
> 
>     multiclass Multiclass<int i> {
>       def anonymous_0 : Struct<i>;
> 
>       def Def : Class<NAME#anonymous_0>;
>     }
> 
> Then class Struct<i> is added to anonymous_0.  Also Class<NAME#anonymous_0> is
> added to Def:
> 
>     multiclass Multiclass<int i> {
>       def anonymous_0 {
>         int I = !shl(i, 1);
>         int J = !shl(I, 1);
>       }
> 
>       def Def {
>         int Class_J = NAME#anonymous_0.J;
>       }
>     }
> 
> So far so good but then we move on to instantiating this in the defm
> by substituting the template arg 'i'.
> 
> This is how the anonymous prototype looks after fully instantiating.
> 
>     defm Defm = {
>       def Defmanonymous_0 {
>          int I = 4;
>          int J = !shl(I, 1);
>       }
> 
> Note that we only resolved the reference to the template arg.  The
> non-template-arg reference in 'J' has not been resolved yet.
> 
> Then we go on to instantiating the Def prototype:
> 
>       def DefmDef {
>          int Class_J = NAME#anonymous_0.J;
>       }
> 
> Which is resolved to Defmanonymous_0.J and then to !shl(I, 1).
> 
> When we fully resolve each record in a defm, Defmanonymous_0.J does get set to 8
> but that's too late for its use.
> 
> The patch adds a new attribute to the Record class that indicates that this
> def is actually a class-instance value that may be *used* by other defs
> in a multiclass.  (This is unlike regular defs which don't reference each
> other and thus can be resolved independently.)  These are then fully resolved before the
> other defs while the multiclass is instantiated.
> 

Hi Adam,

Is there any reason why your patch works on defs inside multiclasses and not all
defs?  I'm running into a similar problem with this example:

class A <bits<2> op> {
  field bits<2> OP = op;
}

class B <bits <2> op> {
  field A OP = A<op>;
}

def : B <0>;

As you can see, the value of B.OP.OP fails to resolve:

------------- Classes -----------------
class A<bits<2> A:op = { ?, ? }> {
  field bits<2> OP = { A:op{1}, A:op{0} };
  string NAME = ?;
}
class B<bits<2> B:op = { ?, ? }> {
  field A OP = anonymous_0;
  string NAME = ?;
}
------------- Defs -----------------
def anonymous_0 {       // A
  field bits<2> OP = { B:op{1}, B:op{0} };
  string NAME = ?;
}
def anonymous_1 {       // B
  field A OP = anonymous_0;
  string NAME = ?;
}

I'm wondering if modifying your patch to work everywhere and not just
in multiclasses will fix this.  Do you think this might work?

Thanks,
Tom



> I added vg_leak to the new test.  I am not sure if this is necessary but I
> don't think I have a way to test it.  I can also check in without the XFAIL
> and let the bots test this part.
> 
> Also tested that X86.td.expanded and AAarch64.td.expanded were unchange before
> and after this change.  (This issue triggering this problem is a WIP patch.)
> 
> Part of <rdar://problem/17688758>
> 
> Please let me know if it looks good.
> 
> Thanks,
> Adam
> 


> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits