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

By class-instance values I mean 'Class<Arg>' in 'Class<Arg>.Field' or in
'Other<Class<Arg>>' (syntactically s 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 indepedently.)  They are then fully resolved before the
other defs while the multiclass is instantiated.

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>

git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@217886 91177308-0d34-0410-b5e6-96231b3b80d8

diff --git a/include/llvm/TableGen/Record.h b/include/llvm/TableGen/Record.h
index d4bc55724825ca6fcd17324b6be57a8b5f92f741..8c5452e511ca3040f92f9424d7f93a0f3bb257fd 100644 (file)
--- a/include/llvm/TableGen/Record.h
+++ b/include/llvm/TableGen/Record.h
@@ -1401,6 +1401,18 @@ class Record {
   DefInit *TheInit;
   bool IsAnonymous;
 
+  // Class-instance values can be used by other defs.  For example, Struct<i>
+  // is used here as a template argument to another class:
+  //
+  //   multiclass MultiClass<int i> {
+  //     def Def : Class<Struct<i>>;
+  //
+  // These need to get fully resolved before instantiating any other
+  // definitions that usie them (e.g. Def).  However, inside a multiclass they
+  // can't be immediately resolved so we mark them ResolveFirst to fully
+  // resolve them later as soon as the multiclass is instantiated.
+  bool ResolveFirst;
+
   void init();
   void checkName();
 
@@ -1409,13 +1421,15 @@ public:
   explicit Record(const std::string &N, ArrayRef<SMLoc> locs,
                   RecordKeeper &records, bool Anonymous = false) :
     ID(LastID++), Name(StringInit::get(N)), Locs(locs.begin(), locs.end()),
-    TrackedRecords(records), TheInit(nullptr), IsAnonymous(Anonymous) {
+    TrackedRecords(records), TheInit(nullptr), IsAnonymous(Anonymous),
+    ResolveFirst(false) {
     init();
   }
   explicit Record(Init *N, ArrayRef<SMLoc> locs, RecordKeeper &records,
                   bool Anonymous = false) :
     ID(LastID++), Name(N), Locs(locs.begin(), locs.end()),
-    TrackedRecords(records), TheInit(nullptr), IsAnonymous(Anonymous) {
+    TrackedRecords(records), TheInit(nullptr), IsAnonymous(Anonymous),
+    ResolveFirst(false) {
     init();
   }
 
@@ -1425,7 +1439,8 @@ public:
     ID(LastID++), Name(O.Name), Locs(O.Locs), TemplateArgs(O.TemplateArgs),
     Values(O.Values), SuperClasses(O.SuperClasses),
     SuperClassRanges(O.SuperClassRanges), TrackedRecords(O.TrackedRecords),
-    TheInit(O.TheInit), IsAnonymous(O.IsAnonymous) { }
+    TheInit(O.TheInit), IsAnonymous(O.IsAnonymous),
+    ResolveFirst(O.ResolveFirst) { }
 
   ~Record() {}
 
@@ -1553,6 +1568,14 @@ public:
     return IsAnonymous;
   }
 
+  bool isResolveFirst() const {
+    return ResolveFirst;
+  }
+
+  void setResolveFirst(bool b) {
+    ResolveFirst = b;
+  }
+
   void dump() const;
 
   //===--------------------------------------------------------------------===//

diff --git a/lib/TableGen/TGParser.cpp b/lib/TableGen/TGParser.cpp
index 150397920421bf4fa5492442e852dd3277eafdca..f2910019ed96dfb289ebfde95dc9b10f77dfb587 100644 (file)
--- a/lib/TableGen/TGParser.cpp
+++ b/lib/TableGen/TGParser.cpp
@@ -1264,6 +1264,9 @@ Init *TGParser::ParseSimpleValue(Record *CurRec, RecTy *ItemType,
       NewRec->resolveReferences();
       Records.addDef(NewRec);
     } else {
+      // This needs to get resolved once the multiclass template arguments are
+      // known before any use.
+      NewRec->setResolveFirst(true);
       // Otherwise, we're inside a multiclass, add it to the multiclass.
       CurMultiClass->DefPrototypes.push_back(NewRec);
 
@@ -2602,6 +2605,12 @@ bool TGParser::ParseDefm(MultiClass *CurMultiClass) {
       if (ResolveMulticlassDef(*MC, CurRec, DefProto, DefmLoc))
         return Error(SubClassLoc, "could not instantiate def");
 
+      // Defs that can be used by other definitions should be fully resolved
+      // before any use.
+      if (DefProto->isResolveFirst() && !CurMultiClass) {
+        CurRec->resolveReferences();
+        CurRec->setResolveFirst(false);
+      }
       NewRecDefs.push_back(CurRec);
     }
 

diff --git a/test/TableGen/ClassInstanceValue.td b/test/TableGen/ClassInstanceValue.td
new file mode 100644 (file)
index 0000000..b6c4c93
--- /dev/null
+++ b/test/TableGen/ClassInstanceValue.td
@@ -0,0 +1,19 @@
+// RUN: llvm-tblgen %s | FileCheck %s
+// XFAIL: vg_leak
+
+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>>;
+// CHECK: Class_J = 8
+// CHECK-NOT: Class_J = !shl(I, 1)
+}
+
+defm Defm : MultiClass<2>;