[TableGen] Add field kind to the RecordVal class.

[Paul-C-Anagnostopoulos]

Summary

This revision adds a field kind to the RecordVal class, replacing the prefix bit in the TyAndKind member (formerly TyAndPrefix). A field can now have one of three kinds: normal, nonconcrete OK (indicated by the 'field' keyword), and template argument.

This revision prepares for the next one, where I will improve the algorithms that deal with class and multiclass template arguments.

[Paul-C-Anagnostopoulos]

Bumping this for a "looks good."

[lattner]

I'm ok with this, your expertise surpasses mine Paul :-)

[Paul-C-Anagnostopoulos]

@lattner Did you mean to accept this revision?

[craig.topper]

llvm/lib/TableGen/Record.cpp

2147

Can you fix this as clang-format suggests? I know it was wrong before you got here and I wouldn't care if the constructor at line 2155 didn't have 4 space indentation. So might as well make them consistent while we're here.

[Paul-C-Anagnostopoulos]

Fixed indentation requested by @craig.topper.

[craig.topper]

LGTM

[fpichet]

The llvm tablegen reference guide says the keyword *field* is deprecated.
Is this still valid?

https://llvm.org/docs/TableGen/ProgRef.html

[Paul-C-Anagnostopoulos]

It is deprecated, yes. I'm still investigating whether any TableGen files actually rely on its behavior, which is to allow nonconcrete field values.

Do you have additional information about it?

[fpichet]

I had to add 'field' for something like: ( out of tree target)

field string BaseOpcodeSTSrcKind;

...

let BaseOpcodeSTSrcKind = asmstr # opVal # DataSize # PtrMode #

OffsetMode # isSub # isDS # "_ri7";

BaseOpcodeSTSrcKind is then used in InstructionMapping.
Because the initialization of "let BaseOpcodeSTSrcKind =.." is of
type BinOpInit for which isConcrete() returns false.
I am not familiar with tablegen internal enough to understand what is going
on.

[Paul-C-Anagnostopoulos]

Could you post a simple file that reproduces the problem? Without the context of BaseOpcodeSTSrcKind and all the identifiers being pasted, I cannot tell what's going on. It certainly isn't necessary to declare a field 'field' just to set it to a concatenated value.

[fpichet]

The identifier come from tablegen class parameter:

class InstD_LD<bits<8> opVal, string DataSize, string PtrMode, string
OffsetMode,

bit isSExt, bit isSub, string SubOrAdd, bit isDS, bit

isTemp,

                 dag outs, dag ins, string asmstr, list<dag> pattern>

let BaseOpcodeLDST = asmstr # DataSize # isSExt # isDS # isTemp;

So this will concat string and bit together.
error: Initializer of 'BaseOpcodeLDST' in 'LD_byte_ptr8_ds_add_sext_ri7'
could not be fully resolved: !strconcat("ldl $rsd2, $addr",
!strconcat("byte", !strconcat("1", !strconcat("1", !cast<string>(0)))))

it seems like isTemp cannot be converted to string.

Sorry I can't reduce this more.. its very difficult to extract the problem
in a self contained test case.

[Paul-C-Anagnostopoulos]

That is plenty of information, thanks. The problem is that the paste operator (#) is not willing to convert a bit to a string. I have no idea why and I will fix this if there is no good reason to prevent it. Meanwhile, you can use:

!cast<int>(isDS)
!cast<int>(isTemp)

And the 'field' keyword is unnecessary.

[fpichet]

Thanks, adding the !cast<int> remove the problem. But it would be nice to be able to convert from bit to string automatically otherwise I would need to add the !cast<int> at dozen of places.

[Paul-C-Anagnostopoulos]

I will look at this issue tomorrow and should have a patch soon.

llvm/include/llvm/TableGen/Record.h

 //  High-Level Classes
 //===----------------------------------------------------------------------===//
 
 /// This class represents a field in a record, including its name, type,
 /// value, and source location.
 class RecordVal {
   friend class Record;
 
+public:
+  enum FieldKind {
+    FK_Normal,        // A normal record field.
+    FK_NonconcreteOK, // A field that can be nonconcrete ('field' keyword).
+    FK_TemplateArg,   // A template argument.
+  };
+
+private:
   Init *Name;
   SMLoc Loc; // Source location of definition of name.
-  PointerIntPair<RecTy *, 1, bool> TyAndPrefix;
+  PointerIntPair<RecTy *, 2, FieldKind> TyAndKind;
   Init *Value;
 
 public:
-  RecordVal(Init *N, RecTy *T, bool P);
-  RecordVal(Init *N, SMLoc Loc, RecTy *T, bool P);
+  RecordVal(Init *N, RecTy *T, FieldKind K);
+  RecordVal(Init *N, SMLoc Loc, RecTy *T, FieldKind K);
 
   /// Get the name of the field as a StringRef.
   StringRef getName() const;
 
   /// Get the name of the field as an Init.
   Init *getNameInit() const { return Name; }
 
   /// Get the name of the field as a std::string.
   std::string getNameInitAsString() const {
     return getNameInit()->getAsUnquotedString();
   }
 
   /// Get the source location of the point where the field was defined.
   const SMLoc &getLoc() const { return Loc; }
 
-  bool getPrefix() const { return TyAndPrefix.getInt(); }
+  /// Is this a field where nonconcrete values are okay?
+  bool isNonconcreteOK() const {
+    return TyAndKind.getInt() == FK_NonconcreteOK;
+  }
+
+  /// Is this a template argument?
+  bool isTemplateArg() const {
+    return TyAndKind.getInt() == FK_TemplateArg;
+  }
 
   /// Get the type of the field value as a RecTy.
-  RecTy *getType() const { return TyAndPrefix.getPointer(); }
+  RecTy *getType() const { return TyAndKind.getPointer(); }
 
   /// Get the type of the field for printing purposes.
   std::string getPrintType() const;
 
   /// Get the value of the field as an Init.
   Init *getValue() const { return Value; }
 
   /// Set the value of the field from an Init.

llvm/lib/TableGen/JSONBackend.cpp

for (const auto &D : Records.getDefs()) {
     auto &Def = *D.second;
 
     json::Object obj;
     json::Array fields;
 
     for (const RecordVal &RV : Def.getValues()) {
       if (!Def.isTemplateArg(RV.getNameInit())) {
         auto Name = RV.getNameInitAsString();
-        if (RV.getPrefix())
+        if (RV.isNonconcreteOK())
           fields.push_back(Name);
         obj[Name] = translateInit(*RV.getValue());
       }
     }
 
     obj["!fields"] = std::move(fields);
 
     json::Array superclasses;

llvm/lib/TableGen/Record.cpp

std::string DagInit::getAsString() const {
   }
   return Result + ")";
 }
 
 //===----------------------------------------------------------------------===//
 //    Other implementations
 //===----------------------------------------------------------------------===//
 
-RecordVal::RecordVal(Init *N, RecTy *T, bool P)
-  : Name(N), TyAndPrefix(T, P) {
+RecordVal::RecordVal(Init *N, RecTy *T, FieldKind K)
+  : Name(N), TyAndKind(T, K) {
   setValue(UnsetInit::get());
   assert(Value && "Cannot create unset value for current type!");
 }
 
 // This constructor accepts the same arguments as the above, but also
 // a source location.
-RecordVal::RecordVal(Init *N, SMLoc Loc, RecTy *T, bool P)
-    : Name(N), Loc(Loc), TyAndPrefix(T, P) {
+RecordVal::RecordVal(Init *N, SMLoc Loc, RecTy *T, FieldKind K)
+    : Name(N), Loc(Loc), TyAndKind(T, K) {
   setValue(UnsetInit::get());
   assert(Value && "Cannot create unset value for current type!");
 }
 
 StringRef RecordVal::getName() const {
   return cast<StringInit>(getNameInit())->getValue();
 }
 
 std::string RecordVal::getPrintType() const {
   if (getType() == StringRecTy::get()) {
     if (auto *StrInit = dyn_cast<StringInit>(Value)) {
       if (StrInit->hasCodeFormat())
         return "code";
       else
         return "string";
     } else {
       return "string";
     }
   } else {
-    return TyAndPrefix.getPointer()->getAsString();
+    return TyAndKind.getPointer()->getAsString();
   }
 }
 
 bool RecordVal::setValue(Init *V) {
   if (V) {
     Value = V->getCastTo(getType());
     if (Value) {
       assert(!isa<TypedInit>(Value) ||

 }
 
 #include "llvm/TableGen/Record.h"
 #if !defined(NDEBUG) || defined(LLVM_ENABLE_DUMP)
 LLVM_DUMP_METHOD void RecordVal::dump() const { errs() << *this; }
 #endif
 
 void RecordVal::print(raw_ostream &OS, bool PrintSem) const {
-  if (getPrefix()) OS << "field ";
+  if (isNonconcreteOK()) OS << "field ";
   OS << getPrintType() << " " << getNameInitAsString();
 
   if (getValue())
     OS << " = " << *getValue();
 
   if (PrintSem) OS << ";\n";
 }
 
raw_ostream &llvm::operator<<(raw_ostream &OS, const Record &R) {
   if (!SC.empty()) {
     OS << "\t//";
     for (const auto &SuperPair : SC)
       OS << " " << SuperPair.first->getNameInitAsString();
   }
   OS << "\n";
 
   for (const RecordVal &Val : R.getValues())
-    if (Val.getPrefix() && !R.isTemplateArg(Val.getNameInit()))
+    if (Val.isNonconcreteOK() && !R.isTemplateArg(Val.getNameInit()))
       OS << Val;
   for (const RecordVal &Val : R.getValues())
-    if (!Val.getPrefix() && !R.isTemplateArg(Val.getNameInit()))
+    if (!Val.isNonconcreteOK() && !R.isTemplateArg(Val.getNameInit()))
       OS << Val;
 
   return OS << "}\n";
 }
 
 SMLoc Record::getFieldLoc(StringRef FieldName) const {
   const RecordVal *R = getValue(FieldName);
   if (!R)

llvm/lib/TableGen/TGParser.cpp

 }
 
 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())
+    if (RV.isNonconcreteOK())
       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() +
case tgtok::XFoldl: {
     // two variables.
     std::unique_ptr<Record> ParseRecTmp;
     Record *ParseRec = CurRec;
     if (!ParseRec) {
       ParseRecTmp = std::make_unique<Record>(".parse", ArrayRef<SMLoc>{}, Records);
       ParseRec = ParseRecTmp.get();
     }
 
-    ParseRec->addValue(RecordVal(A, Start->getType(), false));
-    ParseRec->addValue(RecordVal(B, ListType->getElementType(), false));
+    ParseRec->addValue(RecordVal(A, Start->getType(), RecordVal::FK_Normal));
+    ParseRec->addValue(RecordVal(B, ListType->getElementType(),
+                                 RecordVal::FK_Normal));
     Init *ExprUntyped = ParseValue(ParseRec);
     ParseRec->removeValue(A);
     ParseRec->removeValue(B);
     if (!ExprUntyped)
       return nullptr;
 
     TypedInit *Expr = dyn_cast<TypedInit>(ExprUntyped);
     if (!Expr) {
Init *TGParser::ParseOperationForEachFilter(Record *CurRec, RecTy *ItemType) {
   std::unique_ptr<Record> ParseRecTmp;
   Record *ParseRec = CurRec;
   if (!ParseRec) {
     ParseRecTmp =
         std::make_unique<Record>(".parse", ArrayRef<SMLoc>{}, Records);
     ParseRec = ParseRecTmp.get();
   }
 
-  ParseRec->addValue(RecordVal(LHS, InEltType, false));
+  ParseRec->addValue(RecordVal(LHS, InEltType, RecordVal::FK_Normal));
   Init *RHS = ParseValue(ParseRec, ExprEltType);
   ParseRec->removeValue(LHS);
   if (!RHS)
     return nullptr;
 
   if (!consume(tgtok::r_paren)) {
     TokError("expected ')' in !foreach/!filter");
     return nullptr;
if (CurRec)
       DeclName = QualifyName(*CurRec, CurMultiClass, DeclName, ":");
     else
       assert(CurMultiClass);
     if (CurMultiClass)
       DeclName = QualifyName(CurMultiClass->Rec, CurMultiClass, DeclName,
                              "::");
   }
 
-  // Add the value.
-  if (AddValue(CurRec, IdLoc, RecordVal(DeclName, IdLoc, Type, HasField)))
+  // Add the field to the record.
+  if (AddValue(CurRec, IdLoc, RecordVal(DeclName, IdLoc, Type,
+                                        HasField ? RecordVal::FK_NonconcreteOK
+                                                 : RecordVal::FK_Normal)))
     return nullptr;
 
   // If a value is present, parse it.
   if (consume(tgtok::equal)) {
     SMLoc ValLoc = Lex.getLoc();
     Init *Val = ParseValue(CurRec, Type);
     if (!Val ||
         SetValue(CurRec, ValLoc, DeclName, None, Val))

llvm/utils/TableGen/CodeEmitterGen.cpp

if (Target.getInstructionSet()->
     }
   }
 
   // Loop over all of the fields in the instruction, determining which are the
   // operands to the instruction.
   for (const RecordVal &RV : EncodingDef->getValues()) {
     // Ignore fixed fields in the record, we're looking for values like:
     //    bits<5> RST = { ?, ?, ?, ?, ? };
-    if (RV.getPrefix() || RV.getValue()->isComplete())
+    if (RV.isNonconcreteOK() || RV.getValue()->isComplete())
       continue;
 
     AddCodeToMergeInOperand(R, BI, std::string(RV.getName()), NumberedOp,
                             NamedOpIndices, Case, Target);
   }
 
   StringRef PostEmitter = R->getValueAsString("PostEncoderMethod");
   if (!PostEmitter.empty()) {

llvm/utils/TableGen/FixedLenDecoderEmitter.cpp

if (Target.getInstructionSet()->
           continue;
 
         NamedOpIndices.insert(OpIdx);
       }
 
     for (unsigned i = 0, e = Vals.size(); i != e; ++i) {
       // Ignore fixed fields in the record, we're looking for values like:
       //    bits<5> RST = { ?, ?, ?, ?, ? };
-      if (Vals[i].getPrefix() || Vals[i].getValue()->isComplete())
+      if (Vals[i].isNonconcreteOK() || Vals[i].getValue()->isComplete())
         continue;
 
       // Determine if Vals[i] actually contributes to the Inst encoding.
       unsigned bi = 0;
       for (; bi < Bits.getNumBits(); ++bi) {
         VarInit *Var = nullptr;
         VarBitInit *BI = dyn_cast<VarBitInit>(Bits.getBit(bi));
         if (BI)