[TableGen] Add "InitValue": Handle operands with set bit values in decoder methods

[nlguillemot]

Summary

The problem:

When an operand had bits explicitly set to "1" (as in the InitValue.td test case attached), the decoder was ignoring those bits, and the DecoderMethod was receiving an input where the bits were still zero.

The solution:

We added an "InitValue" variable that stores the initial value of the operand based on what bits were explicitly initialized to 1 in TableGen code. The generated decoder code then uses that initial value to initialize the "tmp" variable, then calls fieldFromInstruction to read the values for the remaining bits that were left unknown in TableGen.

This is mainly useful when there are variations of an instruction that differ based on what bits are set in the operands, since this change makes it possible to access those bits in a DecoderMethod. The DecoderMethod can use those bits to know how to handle the input.

Patch by Nicolas Guillemot

[nlguillemot]

ping

[dsanders]

LGTM with a couple nits fixed

test/TableGen/FixedLenDecoderEmitter/InitValue.td

11 ↗

(On Diff #206305)

This is a bit confusing and isn't really doing anything helpful. For this test case, I'd prefer to just have the lets duplicated inside the defs.

Also, the formatting in this file isn't following the conventions used by most tablegen files (e.g. closing } being in the same column as the 'd' from def)

[nlguillemot]

test/TableGen/FixedLenDecoderEmitter/InitValue.td

11 ↗

(On Diff #206305)

The formatting is copy/pasted from test/TableGen/BitOffsetDecoder.td, so that's my "claim of innocence". Of course we could change it anyways, and clean up BitOffsetDecoder.td as well in a separate patch. Thoughts?

[nlguillemot]

test/TableGen/FixedLenDecoderEmitter/InitValue.td

11 ↗

(On Diff #206305)

ping - what would you like to do?

[dsanders]

test/TableGen/FixedLenDecoderEmitter/InitValue.td

11 ↗

(On Diff #206305)

I'd prefer it to be formatted in the conventional way but since the same formatting is used elsewhere it's ok to keep it as-is

[nlguillemot]

I don't have commit privileges yet. Could somebody commit this for me?

llvm/trunk/test/TableGen/FixedLenDecoderEmitter/InitValue.td

+// RUN: llvm-tblgen -gen-disassembler -I %p/../../../include %s | FileCheck %s
+
+include "llvm/Target/Target.td"
+
+def archInstrInfo : InstrInfo { }
+
+def arch : Target {
+    let InstructionSet = archInstrInfo;
+}
+
+let OutOperandList = (outs), Size = 2 in {
+
+def foo : Instruction {
+    let InOperandList = (ins i32imm:$factor);
+    field bits<16> Inst;
+    field bits<16> SoftFail = 0;
+    bits<8> factor;
+    let factor{0} = 0; // zero initial value
+    let Inst{15-8} = factor{7-0};
+    }
+
+def bar : Instruction {
+    let InOperandList = (ins i32imm:$factor);
+    field bits<16> Inst;
+    field bits<16> SoftFail = 0;
+    bits<8> factor;
+    let factor{0} = 1; // non-zero initial value
+    let Inst{15-8} = factor{7-0};
+    }
+
+}
+
+// CHECK: tmp = fieldFromInstruction(insn, 9, 7) << 1;
+// CHECK: tmp = 0x1;
+// CHECK: tmp |= fieldFromInstruction(insn, 9, 7) << 1;

llvm/trunk/utils/TableGen/FixedLenDecoderEmitter.cpp

struct EncodingField {
   EncodingField(unsigned B, unsigned W, unsigned O)
     : Base(B), Width(W), Offset(O) { }
 };
 
 struct OperandInfo {
   std::vector<EncodingField> Fields;
   std::string Decoder;
   bool HasCompleteDecoder;
+  uint64_t InitValue;
 
   OperandInfo(std::string D, bool HCD)
-      : Decoder(std::move(D)), HasCompleteDecoder(HCD) {}
+      : Decoder(std::move(D)), HasCompleteDecoder(HCD), InitValue(0) {}
 
   void addField(unsigned Base, unsigned Width, unsigned Offset) {
     Fields.push_back(EncodingField(Base, Width, Offset));
   }
 
   unsigned numFields() const { return Fields.size(); }
 
   typedef std::vector<EncodingField>::const_iterator const_iterator;
unsigned FilterChooser::getIslands(std::vector<unsigned> &StartBits,
   return Num;
 }
 
 void FilterChooser::emitBinaryParser(raw_ostream &o, unsigned &Indentation,
                                      const OperandInfo &OpInfo,
                                      bool &OpHasCompleteDecoder) const {
   const std::string &Decoder = OpInfo.Decoder;
 
-  if (OpInfo.numFields() != 1)
-    o.indent(Indentation) << "tmp = 0;\n";
+  if (OpInfo.numFields() != 1 || OpInfo.InitValue != 0) {
+    o.indent(Indentation) << "tmp = 0x";
+    o.write_hex(OpInfo.InitValue);
+    o << ";\n";
+  }
 
   for (const EncodingField &EF : OpInfo) {
     o.indent(Indentation) << "tmp ";
-    if (OpInfo.numFields() != 1) o << '|';
+    if (OpInfo.numFields() != 1 || OpInfo.InitValue != 0) o << '|';
     o << "= fieldFromInstruction"
       << "(insn, " << EF.Base << ", " << EF.Width << ')';
     if (OpInfo.numFields() != 1 || EF.Offset != 0)
       o << " << " << EF.Offset;
     o << ";\n";
   }
 
   if (Decoder != "") {
for (const auto &Op : InOutOperands) {
     RecordVal *HasCompleteDecoderVal =
       TypeRecord->getValue("hasCompleteDecoder");
     BitInit *HasCompleteDecoderBit = HasCompleteDecoderVal ?
       dyn_cast<BitInit>(HasCompleteDecoderVal->getValue()) : nullptr;
     bool HasCompleteDecoder = HasCompleteDecoderBit ?
       HasCompleteDecoderBit->getValue() : true;
 
     OperandInfo OpInfo(Decoder, HasCompleteDecoder);
+
+    // Some bits of the operand may be required to be 1 depending on the
+    // instruction's encoding. Collect those bits.
+    if (const RecordVal *EncodedValue = EncodingDef.getValue(Op.second))
+      if (const BitsInit *OpBits = dyn_cast<BitsInit>(EncodedValue->getValue()))
+        for (unsigned I = 0; I < OpBits->getNumBits(); ++I)
+          if (const BitInit *OpBit = dyn_cast<BitInit>(OpBits->getBit(I)))
+            if (OpBit->getValue())
+              OpInfo.InitValue |= 1 << I;
+
     unsigned Base = ~0U;
     unsigned Width = 0;
     unsigned Offset = 0;
 
     for (unsigned bi = 0; bi < Bits.getNumBits(); ++bi) {
       VarInit *Var = nullptr;
       VarBitInit *BI = dyn_cast<VarBitInit>(Bits.getBit(bi));
       if (BI)