[TableGen][DecoderEmitter] Decode operands with "all zeros" encoding

[s-barannikov]

Follow-up to #156358. The original change didn't take into account operands with "all zeros" encoding, now fixed.

[llvmbot]

@llvm/pr-subscribers-tablegen

Author: Sergei Barannikov (s-barannikov)

Changes

Follow-up to #156358. The original change didn't take into account operands with "all zeros" encoding, now fixed.

---

Full diff: https://github.com/llvm/llvm-project/pull/158163.diff

3 Files Affected:

• (removed) llvm/test/TableGen/FixedLenDecoderEmitter/InitValue.td (-46)
• (added) llvm/test/TableGen/FixedLenDecoderEmitter/operand-decoder.td (+66)
• (modified) llvm/utils/TableGen/DecoderEmitter.cpp (+22-23)

diff --git a/llvm/test/TableGen/FixedLenDecoderEmitter/InitValue.td b/llvm/test/TableGen/FixedLenDecoderEmitter/InitValue.td
deleted file mode 100644
index 03847439ffc2e..0000000000000
--- a/llvm/test/TableGen/FixedLenDecoderEmitter/InitValue.td
+++ /dev/null
@@ -1,46 +0,0 @@
-// 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};
-    }
-
-def bax : Instruction {
-    let InOperandList = (ins i32imm:$factor);
-    field bits<16> Inst;
-    field bits<16> SoftFail = 0;
-    bits<33> factor;
-    let factor{32} = 1; // non-zero initial value
-    let Inst{15...8} = factor{32...25};
-    }
-
-}
-
-// CHECK: tmp = fieldFromInstruction(insn, 9, 7) << 1;
-// CHECK: tmp = 0x1;
-// CHECK: insertBits(tmp, fieldFromInstruction(insn, 9, 7), 1, 7);
-// CHECK: tmp = 0x100000000;
-// CHECK: insertBits(tmp, fieldFromInstruction(insn, 8, 7), 25, 7);
diff --git a/llvm/test/TableGen/FixedLenDecoderEmitter/operand-decoder.td b/llvm/test/TableGen/FixedLenDecoderEmitter/operand-decoder.td
new file mode 100644
index 0000000000000..f281996cf9a86
--- /dev/null
+++ b/llvm/test/TableGen/FixedLenDecoderEmitter/operand-decoder.td
@@ -0,0 +1,66 @@
+// RUN: llvm-tblgen -gen-disassembler -I %p/../../../include %s | FileCheck %s
+
+include "llvm/Target/Target.td"
+
+def R0 : Register<"r0">;
+def RC : RegisterClass<"MyTarget", [i32], 32, (add R0)>;
+
+def MyInstrInfo : InstrInfo;
+
+def MyTarget : Target {
+  let InstructionSet = MyInstrInfo;
+}
+
+// CHECK-LABEL: case 0:
+// CHECK-NEXT:    if (!Check(S, DecodeRCRegisterClass(MI, Decoder)))
+// CHECK-NEXT:      return MCDisassembler::Fail;
+// CHECK-NEXT:    tmp = fieldFromInstruction(insn, 2, 4);
+// CHECK-NEXT:    MI.addOperand(MCOperand::createImm(tmp));
+// CHECK-NEXT:    tmp = 0x0;
+// CHECK-NEXT:    insertBits(tmp, fieldFromInstruction(insn, 0, 2), 0, 2);
+// CHECK-NEXT:    insertBits(tmp, fieldFromInstruction(insn, 6, 2), 2, 2);
+// CHECK-NEXT:    MI.addOperand(MCOperand::createImm(tmp));
+// CHECK-NEXT:    tmp = 0x0;
+// CHECK-NEXT:    MI.addOperand(MCOperand::createImm(tmp));
+// CHECK-NEXT:    tmp = fieldFromInstruction(insn, 13, 2) << 1;
+// CHECK-NEXT:    MI.addOperand(MCOperand::createImm(tmp));
+// CHECK-NEXT:    tmp = 0x0;
+// CHECK-NEXT:    insertBits(tmp, fieldFromInstruction(insn, 17, 1), 1, 1);
+// CHECK-NEXT:    insertBits(tmp, fieldFromInstruction(insn, 19, 1), 3, 1);
+// CHECK-NEXT:    MI.addOperand(MCOperand::createImm(tmp));
+// CHECK-NEXT:    tmp = 0x5;
+// CHECK-NEXT:    MI.addOperand(MCOperand::createImm(tmp));
+// CHECK-NEXT:    tmp = 0x2;
+// CHECK-NEXT:    insertBits(tmp, fieldFromInstruction(insn, 26, 2), 2, 2);
+// CHECK-NEXT:    MI.addOperand(MCOperand::createImm(tmp));
+// CHECK-NEXT:    tmp = 0xa;
+// CHECK-NEXT:    insertBits(tmp, fieldFromInstruction(insn, 28, 1), 0, 1);
+// CHECK-NEXT:    insertBits(tmp, fieldFromInstruction(insn, 30, 1), 2, 1);
+// CHECK-NEXT:    MI.addOperand(MCOperand::createImm(tmp));
+// CHECK-NEXT:    return S;
+
+def I : Instruction {
+  let OutOperandList = (outs RC:$op0);
+  let InOperandList = (ins i32imm:$op1, i32imm:$op2, i32imm:$op3, i32imm:$op4,
+                           i32imm:$op5, i32imm:$op6, i32imm:$op7, i32imm:$op8);
+  let Size = 4;
+  bits<32> Inst;
+  bits<0> op0;                  // no init, no variable parts
+  bits<4> op1;                  // no init, 1 variable part
+  bits<4> op2;                  // no init, 2 variable parts
+  bits<4> op3 = 0b0000;         // zero init, no variable parts
+  bits<4> op4 = {0, ?, ?, 0};   // zero init, 1 variable part
+  bits<4> op5 = {?, 0, ?, 0};   // zero init, 2 variable parts
+  bits<4> op6 = 0b0101;         // non-zero init, no variable parts
+  bits<4> op7 = {?, ?, 1, 0};   // non-zero init, 1 variable part
+  bits<4> op8 = {1, ?, 1, ?};   // non-zero init, 2 variable parts
+  let Inst{5...2} = op1;
+  let Inst{1...0} = op2{1...0};
+  let Inst{7...6} = op2{3...2};
+  let Inst{11...8} = op3;
+  let Inst{15...12} = op4;
+  let Inst{19...16} = op5;
+  let Inst{23...20} = op6;
+  let Inst{27...24} = op7;
+  let Inst{31...28} = op8;
+}
diff --git a/llvm/utils/TableGen/DecoderEmitter.cpp b/llvm/utils/TableGen/DecoderEmitter.cpp
index 8747d02ac892b..ef3823721c54f 100644
--- a/llvm/utils/TableGen/DecoderEmitter.cpp
+++ b/llvm/utils/TableGen/DecoderEmitter.cpp
@@ -33,6 +33,7 @@
 #include "llvm/Support/CommandLine.h"
 #include "llvm/Support/Debug.h"
 #include "llvm/Support/ErrorHandling.h"
+#include "llvm/Support/Format.h"
 #include "llvm/Support/FormatVariadic.h"
 #include "llvm/Support/FormattedStream.h"
 #include "llvm/Support/KnownBits.h"
@@ -1104,31 +1105,29 @@ void DecoderTableBuilder::emitBinaryParser(raw_ostream &OS, indent Indent,
     return;
   }
 
-  if (OpInfo.Fields.empty() && OpInfo.InitValue && IgnoreFullyDefinedOperands)
-    return;
-
-  // We need to construct the encoding of the operand from pieces if it is not
-  // encoded sequentially or has a non-zero constant part in the encoding.
-  bool UseInsertBits = OpInfo.numFields() > 1 || OpInfo.InitValue.value_or(0);
-
-  if (UseInsertBits) {
-    OS << Indent << "tmp = 0x";
-    OS.write_hex(OpInfo.InitValue.value_or(0));
-    OS << ";\n";
-  }
-
-  for (const auto &[Base, Width, Offset] : OpInfo.fields()) {
-    OS << Indent;
-    if (UseInsertBits)
-      OS << "insertBits(tmp, ";
-    else
-      OS << "tmp = ";
-    OS << "fieldFromInstruction(insn, " << Base << ", " << Width << ')';
-    if (UseInsertBits)
-      OS << ", " << Offset << ", " << Width << ')';
-    else if (Offset != 0)
+  if (OpInfo.fields().empty()) {
+    // Only a constant part. The old behavior is to not decode this operand.
+    if (IgnoreFullyDefinedOperands)
+      return;
+    // Initialize `tmp` with the constant part.
+    OS << Indent << "tmp = " << format_hex(*OpInfo.InitValue, 0) << ";\n";
+  } else if (OpInfo.fields().size() == 1 && !OpInfo.InitValue.value_or(0)) {
+    // One variable part and no/zero constant part. Initialize `tmp` with the
+    // variable part.
+    auto [Base, Width, Offset] = OpInfo.fields().front();
+    OS << Indent << "tmp = fieldFromInstruction(insn, " << Base << ", " << Width
+       << ')';
+    if (Offset)
       OS << " << " << Offset;
     OS << ";\n";
+  } else {
+    // General case. Initialize `tmp` with the constant part, if any, and
+    // insert the variable parts into it.
+    OS << Indent << "tmp = " << format_hex(OpInfo.InitValue.value_or(0), 0)
+       << ";\n";
+    for (auto [Base, Width, Offset] : OpInfo.fields())
+      OS << Indent << "insertBits(tmp, fieldFromInstruction(insn, " << Base
+         << ", " << Width << "), " << Offset << ", " << Width << ");\n";
   }
 
   StringRef Decoder = OpInfo.Decoder;