TableGen: Explicitly check whether a record has been resolved

Summary:
There are various places where resolving and constant folds can
get stuck, especially around casts. We don't always signal an
error for those, because in many cases they can legitimately
occur without being an error in the "untaken branch" of an !if.

Change-Id: I3befc0e4234c8e6cc61190504702918c9f29ce5c

Reviewers: arsenm, craig.topper, tra, MartinO

Subscribers: wdng, llvm-commits

Differential Revision: https://reviews.llvm.org/D43754

llvm-svn: 326786

include/llvm/TableGen/Record.h

@@ -348,6 +348,10 @@ public:
   /// not be completely specified yet.
   virtual bool isComplete() const { return true; }
 
+  /// Is this a concrete and fully resolved value without any references or
+  /// stuck operations? Unset values are concrete.
+  virtual bool isConcrete() const { return false; }
+
   /// Print out this value.
   void print(raw_ostream &OS) const { OS << getAsString(); }
 
@@ -468,6 +472,7 @@ public:
   }
 
   bool isComplete() const override { return false; }
+  bool isConcrete() const override { return true; }
   std::string getAsString() const override { return "?"; }
 };
 
@@ -496,6 +501,7 @@ public:
     return const_cast<BitInit*>(this);
   }
 
+  bool isConcrete() const override { return true; }
   std::string getAsString() const override { return Value ? "1" : "0"; }
 };
 
@@ -540,6 +546,7 @@ public:
     return true;
   }
 
+  bool isConcrete() const override;
   std::string getAsString() const override;
 
   Init *resolveReferences(Resolver &R) const override;
@@ -572,6 +579,7 @@ public:
   Init *convertInitializerTo(RecTy *Ty) const override;
   Init *convertInitializerBitRange(ArrayRef<unsigned> Bits) const override;
 
+  bool isConcrete() const override { return true; }
   std::string getAsString() const override;
 
   Init *getBit(unsigned Bit) const override {
@@ -600,6 +608,7 @@ public:
 
   Init *convertInitializerTo(RecTy *Ty) const override;
 
+  bool isConcrete() const override { return true; }
   std::string getAsString() const override { return "\"" + Value.str() + "\""; }
 
   std::string getAsUnquotedString() const override { return Value; }
@@ -630,6 +639,7 @@ public:
 
   Init *convertInitializerTo(RecTy *Ty) const override;
 
+  bool isConcrete() const override { return true; }
   std::string getAsString() const override {
     return "[{" + Value.str() + "}]";
   }
@@ -689,6 +699,7 @@ public:
   ///
   Init *resolveReferences(Resolver &R) const override;
 
+  bool isConcrete() const override;
   std::string getAsString() const override;
 
   ArrayRef<Init*> getValues() const {
@@ -1033,6 +1044,7 @@ public:
 
   RecTy *getFieldType(StringInit *FieldName) const override;
 
+  bool isConcrete() const override { return true; }
   std::string getAsString() const override;
 
   Init *getBit(unsigned Bit) const override {
@@ -1140,6 +1152,7 @@ public:
 
   Init *resolveReferences(Resolver &R) const override;
 
+  bool isConcrete() const override;
   std::string getAsString() const override;
 
   using const_arg_iterator = SmallVectorImpl<Init*>::const_iterator;

lib/TableGen/Record.cpp

@@ -393,6 +393,14 @@ BitsInit::convertInitializerBitRange(ArrayRef<unsigned> Bits) const {
   return BitsInit::get(NewBits);
 }
 
+bool BitsInit::isConcrete() const {
+  for (unsigned i = 0, e = getNumBits(); i != e; ++i) {
+    if (!getBit(i)->isConcrete())
+      return false;
+  }
+  return true;
+}
+
 std::string BitsInit::getAsString() const {
   std::string Result = "{ ";
   for (unsigned i = 0, e = getNumBits(); i != e; ++i) {
@@ -641,6 +649,14 @@ Init *ListInit::resolveReferences(Resolver &R) const {
   return const_cast<ListInit *>(this);
 }
 
+bool ListInit::isConcrete() const {
+  for (Init *Element : *this) {
+    if (!Element->isConcrete())
+      return false;
+  }
+  return true;
+}
+
 std::string ListInit::getAsString() const {
   std::string Result = "[";
   const char *sep = "";
@@ -1444,6 +1460,16 @@ Init *DagInit::resolveReferences(Resolver &R) const {
   return const_cast<DagInit *>(this);
 }
 
+bool DagInit::isConcrete() const {
+  if (!Val->isConcrete())
+    return false;
+  for (const Init *Elt : getArgs()) {
+    if (!Elt->isConcrete())
+      return false;
+  }
+  return true;
+}
+
 std::string DagInit::getAsString() const {
   std::string Result = "(" + Val->getAsString();
   if (ValName)

lib/TableGen/TGParser.cpp

@@ -68,6 +68,47 @@ LLVM_DUMP_METHOD void SubMultiClassReference::dump() const {
 
 } // end namespace llvm
 
+static bool checkBitsConcrete(Record &R, const RecordVal &RV) {
+  BitsInit *BV = cast<BitsInit>(RV.getValue());
+  for (unsigned i = 0, e = BV->getNumBits(); i != e; ++i) {
+    Init *Bit = BV->getBit(i);
+    bool IsReference = false;
+    if (auto VBI = dyn_cast<VarBitInit>(Bit)) {
+      if (auto VI = dyn_cast<VarInit>(VBI->getBitVar())) {
+        if (R.getValue(VI->getName()))
+          IsReference = true;
+      }
+    } else if (isa<VarInit>(Bit)) {
+      IsReference = true;
+    }
+    if (!(IsReference || Bit->isConcrete()))
+      return false;
+  }
+  return true;
+}
+
+static void checkConcrete(Record &R) {
+  for (const RecordVal &RV : R.getValues()) {
+    // HACK: Disable this check for variables declared with 'field'. This is
+    // done merely because existing targets have legitimate cases of
+    // non-concrete variables in helper defs. Ideally, we'd introduce a
+    // 'maybe' or 'optional' modifier instead of this.
+    if (RV.getPrefix())
+      continue;
+
+    if (Init *V = RV.getValue()) {
+      bool Ok = isa<BitsInit>(V) ? checkBitsConcrete(R, RV) : V->isConcrete();
+      if (!Ok) {
+        PrintError(R.getLoc(),
+                   Twine("Initializer of '") + RV.getNameInitAsString() +
+                   "' in '" + R.getNameInitAsString() +
+                   "' could not be fully resolved: " +
+                   RV.getValue()->getAsString());
+      }
+    }
+  }
+}
+
 bool TGParser::AddValue(Record *CurRec, SMLoc Loc, const RecordVal &RV) {
   if (!CurRec)
     CurRec = &CurMultiClass->Rec;
@@ -371,6 +412,7 @@ bool TGParser::ProcessForeachDefs(Record *CurRec, SMLoc Loc, IterSet &IterVals){
   Record *IterRecSave = IterRec.get(); // Keep a copy before release.
   Records.addDef(std::move(IterRec));
   IterRecSave->resolveReferences();
+  checkConcrete(*IterRecSave);
   return false;
 }
 
@@ -2150,11 +2192,14 @@ bool TGParser::ParseDef(MultiClass *CurMultiClass) {
     return true;
   }
 
-  if (!CurMultiClass)  // Def's in multiclasses aren't really defs.
+  if (!CurMultiClass) { // Def's in multiclasses aren't really defs.
     // See Record::setName().  This resolve step will see any new name
     // for the def that might have been created when resolving
     // inheritance, values and arguments above.
     CurRec->resolveReferences();
+    if (Loops.empty())
+      checkConcrete(*CurRec);
+  }
 
   // If ObjectBody has template arguments, it's an error.
   assert(CurRec->getTemplateArgs().empty() && "How'd this get template args?");
@@ -2747,12 +2792,15 @@ bool TGParser::ParseDefm(MultiClass *CurMultiClass) {
     }
   }
 
-  if (!CurMultiClass)
-    for (Record *CurRec : NewRecDefs)
+  if (!CurMultiClass) {
+    for (Record *CurRec : NewRecDefs) {
       // See Record::setName().  This resolve step will see any new
       // name for the def that might have been created when resolving
       // inheritance, values and arguments above.
       CurRec->resolveReferences();
+      checkConcrete(*CurRec);
+    }
+  }
 
   if (Lex.getCode() != tgtok::semi)
     return TokError("expected ';' at end of defm");

test/TableGen/BitOffsetDecoder.td

@@ -55,18 +55,8 @@ def baz : Instruction {
     field bits<16> SoftFail = 0;
     }
 
-def bum : Instruction {
-    let InOperandList = (ins i32imm:$factor);
-    field bits<16> Inst;
-    bits<32> factor;
-    let Inst{7-0} = 0xEE;
-    let Inst{15-8} = !srl(factor,5);
-    let AsmString = "bum  $factor";
-    field bits<16> SoftFail = 0;
-    }
 }
 
-
 // CHECK: tmp = fieldFromInstruction(insn, 8, 7);
 // CHECK: tmp = fieldFromInstruction(insn, 8, 8) << 3;
 // CHECK: tmp |= fieldFromInstruction(insn, 8, 4) << 7;

test/TableGen/BitsInitOverflow.td

@@ -1,8 +1,8 @@
-// RUN: llvm-tblgen %s | FileCheck %s
+// RUN: not llvm-tblgen %s 2>&1 | FileCheck %s
 
 // Check that a large integer is not truncated to a small bit sequence.
 //
-// CHECK: bits<2> X = { !cast<bits<2>>(5){1}, !cast<bits<2>>(5){0} };
+// CHECK: error: Initializer of 'X' in 'anonymous_0' could not be fully resolved:
 
 def {
   bits<2> X = 5;  // bitfield is too small, reject