TableGen: Use DefInit::getDef() instead of the type's getRecord()

[nhaehnle]

Summary

The former simply makes more sense: we want to access the data here in
the backend, not information about the type.

More importantly, removing users of RecordRecTy::getRecord() allows us
more freedom to refactor the frontend.

Change-Id: Iee8905fd22cdb9b11c42ca03246c03d8fe4dd77f

[tra]

The change looks good to me, but you should get OK from someone familiar with the back-end.

[nhaehnle]

Thanks. It's hard to say who really groks utils/TableGen/FixedLenCodeEmitter.cpp, adding some folks (+ @arsenm) who at least wrote the bits near my changes.

I've verified that this change doesn't change the generated code in any of the backends in trunk at all.

The basic pattern is simple: use cast<DefInit>(TI)->getDef() instead of cast<RecordRecTy>(TI->getType())->getRecord(). If TI is a DefInit, those will yield the same result. If TI is not a DefInit, then the former will crash and the latter will return the record representing some uninstantiated class, which is not meaningful.

So I'd say this is a low-risk change.

[tra]

In D43679#1018714, @nhaehnle wrote:

Thanks. It's hard to say who really groks utils/TableGen/FixedLenCodeEmitter.cpp, adding some folks (+ @arsenm) who at least wrote the bits near my changes.

I've verified that this change doesn't change the generated code in any of the backends in trunk at all.

Great. At least we're not breaking correct code.

The basic pattern is simple: use cast<DefInit>(TI)->getDef() instead of cast<RecordRecTy>(TI->getType())->getRecord(). If TI is a DefInit, those will yield the same result. If TI is not a DefInit, then the former will crash and the latter will return the record representing some uninstantiated class, which is not meaningful.

If you can think of a way for user input to trigger this scenario, we should probably issue an error. I'm not sure if that's the case, though.
My attempts to trigger the problem were thwarted by tablegen -- it aborted with an error before TI with a wrong type could get to populateInstruction().

So I'd say this is a low-risk change.

Agreed.
If nobody chimes in today, I'm OK with landing this change.

[nhaehnle]

r326699

utils/TableGen/FixedLenDecoderEmitter.cpp

dumpBits(errs(),
              getBitsField(*AllInstructions[Opcodes[i]]->TheDef, "Inst"));
     errs() << '\n';
   }
 }
 
 static std::string findOperandDecoderMethod(TypedInit *TI) {
   std::string Decoder;
 
-  RecordRecTy *Type = cast<RecordRecTy>(TI->getType());
-  Record *TypeRecord = Type->getRecord();
+  Record *Record = cast<DefInit>(TI)->getDef();
 
-  RecordVal *DecoderString = TypeRecord->getValue("DecoderMethod");
+  RecordVal *DecoderString = Record->getValue("DecoderMethod");
   StringInit *String = DecoderString ?
     dyn_cast<StringInit>(DecoderString->getValue()) : nullptr;
   if (String) {
     Decoder = String->getValue();
     if (!Decoder.empty())
       return Decoder;
   }
 
-  if (TypeRecord->isSubClassOf("RegisterOperand"))
-    TypeRecord = TypeRecord->getValueAsDef("RegClass");
+  if (Record->isSubClassOf("RegisterOperand"))
+    Record = Record->getValueAsDef("RegClass");
 
-  if (TypeRecord->isSubClassOf("RegisterClass")) {
-    Decoder = "Decode" + TypeRecord->getName().str() + "RegisterClass";
-  } else if (TypeRecord->isSubClassOf("PointerLikeRegClass")) {
+  if (Record->isSubClassOf("RegisterClass")) {
+    Decoder = "Decode" + Record->getName().str() + "RegisterClass";
+  } else if (Record->isSubClassOf("PointerLikeRegClass")) {
     Decoder = "DecodePointerLikeRegClass" +
-      utostr(TypeRecord->getValueAsInt("RegClassKind"));
+        utostr(Record->getValueAsInt("RegClassKind"));
   }
 
   return Decoder;
 }
 
 static bool populateInstruction(CodeGenTarget &Target,
                        const CodeGenInstruction &CGI, unsigned Opc,
                        std::map<unsigned, std::vector<OperandInfo>> &Operands){
for (unsigned i = 0, e = Vals.size(); i != e; ++i) {
       if (String && String->getValue() != "")
         Decoder = String->getValue();
 
       if (Decoder == "" &&
           CGI.Operands[SO.first].MIOperandInfo &&
           CGI.Operands[SO.first].MIOperandInfo->getNumArgs()) {
         Init *Arg = CGI.Operands[SO.first].MIOperandInfo->
                       getArg(SO.second);
-        if (TypedInit *TI = cast<TypedInit>(Arg)) {
-          RecordRecTy *Type = cast<RecordRecTy>(TI->getType());
-          TypeRecord = Type->getRecord();
-        }
+        if (DefInit *DI = cast<DefInit>(Arg))
+          TypeRecord = DI->getDef();
       }
 
       bool isReg = false;
       if (TypeRecord->isSubClassOf("RegisterOperand"))
         TypeRecord = TypeRecord->getValueAsDef("RegClass");
       if (TypeRecord->isSubClassOf("RegisterClass")) {
         Decoder = "Decode" + TypeRecord->getName().str() + "RegisterClass";
         isReg = true;
for (const auto &Op : InOutOperands) {
     }
 
     TypedInit *TI = cast<TypedInit>(Op.first);
 
     // At this point, we can locate the decoder field, but we need to know how
     // to interpret it.  As a first step, require the target to provide
     // callbacks for decoding register classes.
     std::string Decoder = findOperandDecoderMethod(TI);
-    Record *TypeRecord = cast<RecordRecTy>(TI->getType())->getRecord();
+    Record *TypeRecord = cast<DefInit>(TI)->getDef();
 
     RecordVal *HasCompleteDecoderVal =
       TypeRecord->getValue("hasCompleteDecoder");
     BitInit *HasCompleteDecoderBit = HasCompleteDecoderVal ?
       dyn_cast<BitInit>(HasCompleteDecoderVal->getValue()) : nullptr;
     bool HasCompleteDecoder = HasCompleteDecoderBit ?
       HasCompleteDecoderBit->getValue() : true;