[tablegen] Add !iscomplete(Value)

[dsanders]

Summary

It's currently very difficult to distinguish 0 from ? in TableGen.
Comparisons such as !eq(?, 0) currently assert on the ? because it isn't
a TypedInit, and !eq({?}, 0) is true because bits<> coerces ? to 0. For
the same reason !isa<int>(?) and !isa<int>({?}) give different outcomes
because ? isn't convertible to int (and is therefore not foldable), even
though {?} is.

As part of this there is a slight relaxation to the folding requirements
of FieldInit so that field references are folded if they have a Concrete
value (fully evaluated) rather than a Complete value (fully evaluated but not
containing ?).

[nhaehnle]

I like this direction. Please add !iscomplete to the documentation, plus some more comments inline.

llvm/test/TableGen/field-access-initializers.td

11

Thank you, this makes much more sense.

llvm/test/TableGen/isa-consistency.td

34

That comment is out of place now, isn't it?

51

Same.

60–61

Same.

llvm/test/TableGen/iscomplete.td

160–162

It doesn't assert, does it? Rather it produces an error message.

[dsanders]

Additional comments

llvm/test/TableGen/isa-consistency.td

34

Z4's ret still fails to resolve like it did before this change. The check is what I'd expect from it but we actually get:

llvm-project/llvm/test/TableGen/isa-consistency.td:37:1: error: Initializer of 'ret' in 'Z4' could not be fully resolved: !isa<int>(?)

I'm not sure what the cause of the inconsistency is but I believe the type of the field A is being accounted for in a way that ? and !cast<int>(?) somehow don't.

Overall, this file is documenting the strange behaviour of isa<> that I ran into while trying to distinguish ? from 0. I haven't changed that behaviour at all. I've added a comment to the top of the test to explain that

51

It's still the case that the !cast<bit>(?) in Z5 doesn't work but the {?, ?} in Z6 does.
We get the same error as Z4 but with a different expression

60–61

It's still the case that the !cast<int>(?) in Z5 doesn't work but the {?, ?}{0} in Z7 does. This one appears to be due to it having the type bits<1> instead of bit which is a bit surprising but does make some sense.

llvm/test/TableGen/iscomplete.td

160–162

This bit is talking about the original bug where it was asserting. I've added a bit mentioning that it now errors out properly

llvm/include/llvm/TableGen/Record.h

public:
 
   /// This method is used by classes that refer to other
   /// variables which may not be defined at the time they expression is formed.
   /// If a value is set for the variable later, this method will be called on
   /// users of the value to allow the value to propagate out.
   ///
   Init *resolveReferences(Resolver &R) const override;
 
+  bool isComplete() const override;
   bool isConcrete() const override;
   std::string getAsString() const override;
 
   ArrayRef<Init*> getValues() const {
     return makeArrayRef(getTrailingObjects<Init *>(), NumValues);
   }
 
   const_iterator begin() const { return getTrailingObjects<Init *>(); }
public:
 
   Init *getBit(unsigned Bit) const override;
 };
 
 /// !op (X) - Transform an init.
 ///
 class UnOpInit : public OpInit, public FoldingSetNode {
 public:
-  enum UnaryOp : uint8_t { CAST, HEAD, TAIL, SIZE, EMPTY, GETOP };
+  enum UnaryOp : uint8_t { CAST, HEAD, TAIL, SIZE, EMPTY, GETOP, ISCOMPLETE };
 
 private:
   Init *LHS;
 
   UnOpInit(UnaryOp opc, Init *lhs, RecTy *Type)
     : OpInit(IK_UnOpInit, Type, opc), LHS(lhs) {}
 
 public:

llvm/lib/TableGen/Record.cpp

for (Init *CurElt : getValues()) {
     Resolved.push_back(E);
   }
 
   if (Changed)
     return ListInit::get(Resolved, getElementType());
   return const_cast<ListInit *>(this);
 }
 
+bool ListInit::isComplete() const {
+  for (Init *Element : *this) {
+    if (!Element->isComplete())
+      return false;
+  }
+  return true;
+}
+
 bool ListInit::isConcrete() const {
   for (Init *Element : *this) {
     if (!Element->isConcrete())
       return false;
   }
   return true;
 }
 
if (DagInit *Dag = dyn_cast<DagInit>(LHS)) {
                         getType()->getAsString() + "', got '" +
                         DI->getType()->getAsString() + "' in: " +
                         getAsString() + "\n");
       } else {
         return DI;
       }
     }
     break;
+
+  case ISCOMPLETE:
+    if (LHS->isConcrete())
+      return IntInit::get(LHS->isComplete());
   }
   return const_cast<UnOpInit *>(this);
 }
 
 Init *UnOpInit::resolveReferences(Resolver &R) const {
   Init *lhs = LHS->resolveReferences(R);
 
   if (LHS != lhs || (R.isFinal() && getOpcode() == CAST))
     return (UnOpInit::get(getOpcode(), lhs, getType()))
         ->Fold(R.getCurrentRecord(), R.isFinal());
   return const_cast<UnOpInit *>(this);
 }
 
 std::string UnOpInit::getAsString() const {
   std::string Result;
   switch (getOpcode()) {
   case CAST: Result = "!cast<" + getType()->getAsString() + ">"; break;
   case HEAD: Result = "!head"; break;
   case TAIL: Result = "!tail"; break;
   case SIZE: Result = "!size"; break;
   case EMPTY: Result = "!empty"; break;
   case GETOP: Result = "!getop"; break;
+  case ISCOMPLETE: Result = "!iscomplete"; break;
   }
   return Result + "(" + LHS->getAsString() + ")";
 }
 
 static void
 ProfileBinOpInit(FoldingSetNodeID &ID, unsigned Opcode, Init *LHS, Init *RHS,
                  RecTy *Type) {
   ID.AddInteger(Opcode);
Init *FieldInit::Fold(Record *CurRec) const {
   if (DefInit *DI = dyn_cast<DefInit>(Rec)) {
     Record *Def = DI->getDef();
     if (Def == CurRec)
       PrintFatalError(CurRec->getLoc(),
                       Twine("Attempting to access field '") +
                       FieldName->getAsUnquotedString() + "' of '" +
                       Rec->getAsString() + "' is a forbidden self-reference");
     Init *FieldVal = Def->getValue(FieldName)->getValue();
-    if (FieldVal->isComplete())
+    if (FieldVal->isConcrete())
       return FieldVal;
   }
   return const_cast<FieldInit *>(this);
 }
 
 bool FieldInit::isConcrete() const {
   if (DefInit *DI = dyn_cast<DefInit>(Rec)) {
     Init *FieldVal = DI->getDef()->getValue(FieldName)->getValue();

llvm/lib/TableGen/TGLexer.h

enum TokKind {
     // that means the preprocessor #else.)
     Bit, Bits, Class, Code, Dag, Def, Foreach, Defm, Field, In, Int, Let, List,
     MultiClass, String, Defset, Defvar, If, Then, ElseKW,
 
     // !keywords.
     XConcat, XADD, XMUL, XAND, XOR, XSRA, XSRL, XSHL, XListConcat, XListSplat,
     XStrConcat, XCast, XSubst, XForEach, XFoldl, XHead, XTail, XSize, XEmpty,
     XIf, XCond, XEq, XIsA, XDag, XNe, XLe, XLt, XGe, XGt, XSetOp, XGetOp,
+    XIsComplete,
 
     // Integer value.
     IntVal,
 
     // Binary constant.  Note that these are sized according to the number of
     // bits given.
     BinaryIntVal,
 

llvm/lib/TableGen/TGLexer.cpp

tgtok::TokKind Kind =
     .Case("subst", tgtok::XSubst)
     .Case("foldl", tgtok::XFoldl)
     .Case("foreach", tgtok::XForEach)
     .Case("listconcat", tgtok::XListConcat)
     .Case("listsplat", tgtok::XListSplat)
     .Case("strconcat", tgtok::XStrConcat)
     .Case("setop", tgtok::XSetOp)
     .Case("getop", tgtok::XGetOp)
+    .Case("iscomplete", tgtok::XIsComplete)
     .Default(tgtok::Error);
 
   return Kind != tgtok::Error ? Kind : ReturnError(Start-1, "Unknown operator");
 }
 
 bool TGLexer::prepExitInclude(bool IncludeStackMustBeEmpty) {
   // Report an error, if preprocessor control stack for the current
   // file is not empty.

llvm/lib/TableGen/TGParser.cpp

Init *TGParser::ParseOperation(Record *CurRec, RecTy *ItemType) {
   default:
     TokError("unknown operation");
     return nullptr;
   case tgtok::XHead:
   case tgtok::XTail:
   case tgtok::XSize:
   case tgtok::XEmpty:
   case tgtok::XCast:
-  case tgtok::XGetOp: {  // Value ::= !unop '(' Value ')'
+  case tgtok::XGetOp:
+  case tgtok::XIsComplete: {  // Value ::= !unop '(' Value ')'
     UnOpInit::UnaryOp Code;
     RecTy *Type = nullptr;
 
     switch (Lex.getCode()) {
     default: llvm_unreachable("Unhandled code!");
     case tgtok::XCast:
       Lex.Lex();  // eat the operation
       Code = UnOpInit::CAST;
case tgtok::XSize:
       Code = UnOpInit::SIZE;
       Type = IntRecTy::get();
       break;
     case tgtok::XEmpty:
       Lex.Lex();  // eat the operation
       Code = UnOpInit::EMPTY;
       Type = IntRecTy::get();
       break;
+    case tgtok::XIsComplete:
+      Lex.Lex();  // eat the operation
+      Code = UnOpInit::ISCOMPLETE;
+      Type = IntRecTy::get();
+      break;
     case tgtok::XGetOp:
       Lex.Lex();  // eat the operation
       if (Lex.getCode() == tgtok::less) {
         // Parse an optional type suffix, so that you can say
         // !getop<BaseClass>(someDag) as a shorthand for
         // !cast<BaseClass>(!getop(someDag)).
         Type = ParseOperatorType();
 
Init *TGParser::ParseSimpleValue(Record *CurRec, RecTy *ItemType,
   case tgtok::XListConcat:
   case tgtok::XListSplat:
   case tgtok::XStrConcat:
   case tgtok::XSetOp:   // Value ::= !binop '(' Value ',' Value ')'
   case tgtok::XIf:
   case tgtok::XCond:
   case tgtok::XFoldl:
   case tgtok::XForEach:
-  case tgtok::XSubst: {  // Value ::= !ternop '(' Value ',' Value ',' Value ')'
+  case tgtok::XSubst: // Value ::= !ternop '(' Value ',' Value ',' Value ')'
+  case tgtok::XIsComplete:
     return ParseOperation(CurRec, ItemType);
   }
-  }
 
   return R;
 }
 
 /// ParseValue - Parse a tblgen value.  This returns null on error.
 ///
 ///   Value       ::= SimpleValue ValueSuffix*
 ///   ValueSuffix ::= '{' BitList '}'

llvm/test/TableGen/field-access-initializers.td

 // RUN: llvm-tblgen %s | FileCheck %s
 // XFAIL: vg_leak
 
 // CHECK: --- Defs ---
 
 // CHECK: def A1 {
 // CHECK:   string value = ?;
 // CHECK: }
 
 // CHECK: def B1 {
-// CHECK:   string value = A1.value;
+// CHECK:   string value = ?;
 // CHECK: }
 
 class A {
   string value = ?;
 }
 
 class B<A impl> : A {
   let value = impl.value;
 }
 
 def A1 : A;
 def B1 : B<A1>;

llvm/test/TableGen/isa-consistency.td

+// RUN: llvm-tblgen %s | FileCheck %s
+
+// This file is documenting some strange inconsistencies in the behaviour of
+// isa<>() that were encountered while attempting to use it to distinguish
+// `0` from `?`. In particular, Z4 and Z5 behave inconsistently compared to the
+// others.
+// XFAIL: *
+
+// CHECK: def Z1 {
+// CHECK:   int ret = 1;
+// CHECK: }
+
+def Z1 {
+  bit A = ?;
+  int ret = !isa<int>(A);
+}
+
+// CHECK: def Z2 {
+// CHECK:   int ret = 1;
+// CHECK: }
+
+def Z2 {
+  bits<1> A = {?};
+  int ret = !isa<int>(A{0});
+}
+
+// CHECK: def Z3 {
+// CHECK:   int ret = 1;
+// CHECK: }
+
+def Z3 {
+  int A = ?;
+  int ret = !isa<int>(A);
+}
+
+// So far so good, but it doesn't work without the field assignment.
+// CHECK: def Z4 {
+// CHECK:   int ret = 1;
+// CHECK: }
+
+def Z4 {
+  int ret = !isa<int>(!cast<int>(?));
+}
+
+// CHECK: def Z5 {
+// CHECK:   int ret = 1;
+// CHECK: }
+
+def Z5 {
+  int ret = !isa<int>(!cast<bit>(?));
+}
+
+// Yet it does work on bits<>, even though it didn't work on bit.
+// CHECK: def Z6 {
+// CHECK:   int ret = 1;
+// CHECK: }
+
+def Z6 {
+  int ret = !isa<int>({?, ?});
+}
+
+// Stranger still, if you extract an unset bit from a bits<> it still works even
+// though bit didn't.
+// CHECK: def Z7 {
+// CHECK:   int ret = 1;
+// CHECK: }
+
+def Z7 {
+  int ret = !isa<int>({?, ?}{0});
+}

llvm/test/TableGen/iscomplete.td

+// RUN: llvm-tblgen %s | FileCheck %s
+
+def op;
+
+class DefTy<int a> {
+  int A = a;
+}
+
+def Inputs {
+  string StringA = "";
+  string StringB = "foo";
+  string StringC = ?;
+  code CodeA = [{}];
+  code CodeB = [{foo}];
+  code CodeC = ?;
+  bit BitA = 0;
+  bit BitB = 1;
+  bit BitC = ?;
+  int IntA = 0;
+  int IntB = 1;
+  int IntC = ?;
+  dag DagA = (op);
+  dag DagB = (op 1);
+  dag DagC = ?;
+  bits<2> Bit2A = {0, 0};
+  bits<2> Bit2B = {0, 1};
+  bits<2> Bit2C = ?;
+  bits<2> Bit2D = { 0, ? };
+  list<bit> LstBitA = [0, 0];
+  list<bit> LstBitB = [0, 1];
+  list<bit> LstBitC = ?;
+  list<bit> LstBitD = [ ?, 0 ];
+  DefTy DefA = DefTy<0>;
+  DefTy DefB = DefTy<1>;
+  DefTy DefC = ?;
+}
+
+// CHECK-LABEL: def Z1Strings {
+// CHECK-NEXT:    bit A = 1;
+// CHECK-NEXT:    bit B = 1;
+// CHECK-NEXT:    bit C = 0;
+// CHECK-NEXT:  }
+
+def Z1Strings {
+  bit A = !iscomplete(Inputs.StringA);
+  bit B = !iscomplete(Inputs.StringB);
+  bit C = !iscomplete(Inputs.StringC);
+}
+
+// CHECK-LABEL: def Z2Codes {
+// CHECK-NEXT:    bit A = 1;
+// CHECK-NEXT:    bit B = 1;
+// CHECK-NEXT:    bit C = 0;
+// CHECK-NEXT:  }
+
+def Z2Codes {
+  bit A = !iscomplete(Inputs.CodeA);
+  bit B = !iscomplete(Inputs.CodeB);
+  bit C = !iscomplete(Inputs.CodeC);
+}
+
+// CHECK-LABEL: def Z3Bits {
+// CHECK-NEXT:    bit A = 1;
+// CHECK-NEXT:    bit B = 1;
+// CHECK-NEXT:    bit C = 0;
+// CHECK-NEXT:  }
+
+def Z3Bits {
+  bit A = !iscomplete(Inputs.BitA);
+  bit B = !iscomplete(Inputs.BitB);
+  bit C = !iscomplete(Inputs.BitC);
+}
+
+// CHECK-LABEL: def Z4Ints {
+// CHECK-NEXT:    bit A = 1;
+// CHECK-NEXT:    bit B = 1;
+// CHECK-NEXT:    bit C = 0;
+// CHECK-NEXT:  }
+
+def Z4Ints {
+  bit A = !iscomplete(Inputs.IntA);
+  bit B = !iscomplete(Inputs.IntB);
+  bit C = !iscomplete(Inputs.IntC);
+}
+
+// CHECK-LABEL: def Z5Dags {
+// CHECK-NEXT:    bit A = 1;
+// CHECK-NEXT:    bit B = 1;
+// CHECK-NEXT:    bit C = 0;
+// CHECK-NEXT:  }
+
+def Z5Dags {
+  bit A = !iscomplete(Inputs.DagA);
+  bit B = !iscomplete(Inputs.DagB);
+  bit C = !iscomplete(Inputs.DagC);
+}
+
+// CHECK-LABEL: def Z6Bit2s {
+// CHECK-NEXT:    bit A = 1;
+// CHECK-NEXT:    bit B = 1;
+// CHECK-NEXT:    bit C = 0;
+// CHECK-NEXT:    bit D = 0;
+// CHECK-NEXT:    bit E = 1;
+// CHECK-NEXT:  }
+
+def Z6Bit2s {
+  bit A = !iscomplete(Inputs.Bit2A);
+  bit B = !iscomplete(Inputs.Bit2B);
+  bit C = !iscomplete(Inputs.Bit2C);
+  // Test these two explicitly because unset is not convertible to int but an
+  // unset bit extracted from a a bits<> is convertible to int.
+  bit D = !iscomplete(Inputs.Bit2D{0});
+  bit E = !iscomplete(Inputs.Bit2D{1});
+}
+
+// CHECK-LABEL: def Z7LstBits {
+// CHECK-NEXT:    bit A = 1;
+// CHECK-NEXT:    bit B = 1;
+// CHECK-NEXT:    bit C = 0;
+// CHECK-NEXT:    bit D = 0;
+// CHECK-NEXT:    bit E = 1;
+// CHECK-NEXT:  }
+
+def Z7LstBits {
+  bit A = !iscomplete(Inputs.LstBitA);
+  bit B = !iscomplete(Inputs.LstBitB);
+  bit C = !iscomplete(Inputs.LstBitC);
+  // Test these two explicitly because unset is not convertible to int but an
+  // unset bit extracted from a a bits<> is convertible to int.
+  bit D = !iscomplete(Inputs.LstBitD[0]);
+  bit E = !iscomplete(Inputs.LstBitD[1]);
+}
+
+// CHECK-LABEL: def Z8Defs {
+// CHECK-NEXT:    bit A = 1;
+// CHECK-NEXT:    bit B = 1;
+// CHECK-NEXT:    bit C = 0;
+// CHECK-NEXT:  }
+
+def Z8Defs {
+  bit A = !iscomplete(Inputs.DefA);
+  bit B = !iscomplete(Inputs.DefB);
+  bit C = !iscomplete(Inputs.DefC);
+}
+
+// CHECK-LABEL: def Z9OriginalBug {
+// CHECK-NEXT:    bit A = 1;
+// CHECK-NEXT:    bit B = 1;
+// CHECK-NEXT:    bit C = 1;
+// CHECK-NEXT:  }
+
+def Z9OriginalBug {
+  // These are the expressions that caused the original problem. Because bits<>
+  // is convertible to int and squashes the `?`s to 0, these both evaluate to 1.
+  bit A = !if(!eq(Inputs.Bit2A{0}, 0), 1, 0);
+  bit B = !if(!eq(Inputs.Bit2A{1}, 0), 1, 0);
+  // This one also evaluates to 1 because BitA's value is a BitInit
+  bit C = !if(!eq(Inputs.BitA, 0), 1, 0);
+  // This one however, caused an assertion because BitC's value an UnsetInit
+  // and !eq() can only accept TypedInit's. This still doesn't work but it
+  // at least emits a proper error.
+  // bit D = !if(!eq(Inputs.BitC, 0), 1, 0);
+}
+