[TableGen][DecoderEmitter] Report all decoding conflicts

[jurahul]

Do not exit when the first decoding conflict is encountered. Instead record the conflict and continue to report any additional decoding conflicts and exit fatally after all instructions have been processed.

[llvmbot]

@llvm/pr-subscribers-tablegen

Author: Rahul Joshi (jurahul)

Changes

Do not exit when the first decoding conflict is encountered. Instead record the conflict and continue to report any additional decoding conflicts and exit fatally after all instructions have been processed.

---

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

3 Files Affected:

• (modified) llvm/test/TableGen/FixedLenDecoderEmitter/conflict.td (+43-7)
• (modified) llvm/test/TableGen/FixedLenDecoderEmitter/var-len-conflict-2.td (+7-9)
• (modified) llvm/utils/TableGen/DecoderEmitter.cpp (+24-5)

diff --git a/llvm/test/TableGen/FixedLenDecoderEmitter/conflict.td b/llvm/test/TableGen/FixedLenDecoderEmitter/conflict.td
index 853a68d22d1d9..e3666ebbf942a 100644
--- a/llvm/test/TableGen/FixedLenDecoderEmitter/conflict.td
+++ b/llvm/test/TableGen/FixedLenDecoderEmitter/conflict.td
@@ -9,27 +9,63 @@ def MyTarget : Target { let InstructionSet = MyTargetISA; }
 def R0 : Register<"r0"> { let Namespace = "MyTarget"; }
 def GPR32 : RegisterClass<"MyTarget", [i32], 32, (add R0)>;
 
+def X0 : Register<"x0"> { let Namespace = "MyTarget"; }
+def GPR64 : RegisterClass<"MyTarget", [i64], 64, (add X0)>;
+
 class I<dag OOps, dag IOps, list<dag> Pat>
   : Instruction {
   let Namespace = "MyTarget";
   let OutOperandList = OOps;
   let InOperandList = IOps;
   let Pattern = Pat;
+  let Size = 4;
   bits<32> Inst;
   bits<32> SoftFail;
 }
 
+// Assume there is a 2 bit encoding for the dst and src register.
 def A : I<(outs GPR32:$dst), (ins GPR32:$src1), []> {
-  let Size = 4;
-  let Inst{31...0} = 0;
+  bits<2> dst;
+  bits<2> src1;
+  let Inst{31...4} = 0;
+  let Inst{1...0} = dst;
+  let Inst{3...2} = src1;
 }
+
 def B : I<(outs GPR32:$dst), (ins GPR32:$src1), []> {
-  let Size = 4;
-  let Inst{31...0} = 0;
+  bits<2> dst;
+  bits<2> src1;
+  let Inst{31...4} = 0;
+  let Inst{1...0} = dst;
+  let Inst{3...2} = src1;
+}
+
+def C : I<(outs GPR64:$dst), (ins GPR64:$src1), []> {
+  bits<2> dst;
+  bits<2> src1;
+  let Inst{31...4} = 1;
+  let Inst{1...0} = dst;
+  let Inst{3...2} = src1;
 }
 
+def D : I<(outs GPR64:$dst), (ins GPR64:$src1), []> {
+  bits<2> dst;
+  bits<2> src1;
+  let Inst{31...4} = 1;
+  let Inst{1...0} = dst;
+  let Inst{3...2} = src1;
+}
+
+// CHECK: Decoding Conflict:
+// CHECK:     ................................
+// CHECK:     0000000000000000000000000000....
+// CHECK:     0000000000000000000000000000____  A
+// CHECK:     0000000000000000000000000000____  B
+
 // CHECK: Decoding Conflict:
 // CHECK:     ................................
-// CHECK:     00000000000000000000000000000000
-// CHECK:     00000000000000000000000000000000  A
-// CHECK:     00000000000000000000000000000000  B
+// CHECK:     0000000000000000000000000001....
+// CHECK:     0000000000000000000000000001____  C
+// CHECK:     0000000000000000000000000001____  D
+
+// CHECK: Decoding conflict encountered
diff --git a/llvm/test/TableGen/FixedLenDecoderEmitter/var-len-conflict-2.td b/llvm/test/TableGen/FixedLenDecoderEmitter/var-len-conflict-2.td
index 8a848a986ce24..7ca13b8debd9d 100644
--- a/llvm/test/TableGen/FixedLenDecoderEmitter/var-len-conflict-2.td
+++ b/llvm/test/TableGen/FixedLenDecoderEmitter/var-len-conflict-2.td
@@ -10,16 +10,14 @@ class I : Instruction {
 
 // Check pretty printing of decoding conflicts.
 
-// CHECK:      {{^}}Decoding Conflict:
-// CHECK-NEXT: {{^}}                    ........
-// CHECK-NEXT: {{^}}            ..............11
-// CHECK-NEXT: {{^}}            .......0......11
-// CHECK-NEXT: {{^}}            _______0______11  I16_0
-// CHECK-NEXT: {{^}}            _______0______11  I16_1
-// CHECK-NEXT: {{^}}    _______0_______0______11  I24_0
+// CHECK:      Decoding Conflict:
+// CHECK-NEXT:             ................
+// CHECK-NEXT:             ..............11
+// CHECK-NEXT:             .......0......11
+// CHECK-NEXT:             _______0______11  I16_0
+// CHECK-NEXT:             _______0______11  I16_1
+// CHECK-NEXT:     _______0_______0______11  I24_0
 
-def I8_0  : I { dag Inst = (descend (operand "$op", 6), 0b00); }
-def I8_1  : I { dag Inst = (descend (operand "$op", 6), 0b01); }
 def I16_0 : I { dag Inst = (descend (operand "$op2", 7), 0b0,
                                     (operand "$op", 6), 0b11); }
 def I16_1 : I { dag Inst = (descend (operand "$op2", 7), 0b0,
diff --git a/llvm/utils/TableGen/DecoderEmitter.cpp b/llvm/utils/TableGen/DecoderEmitter.cpp
index 95e7408ea88c7..ce7734b04cb36 100644
--- a/llvm/utils/TableGen/DecoderEmitter.cpp
+++ b/llvm/utils/TableGen/DecoderEmitter.cpp
@@ -517,6 +517,9 @@ class FilterChooser {
   /// The "field value" here refers to the encoding bits in the filtered range.
   std::map<uint64_t, std::unique_ptr<const FilterChooser>> FilterChooserMap;
 
+  /// Set to true if decoding conflict was encountered.
+  bool HasConflict = false;
+
   struct Island {
     unsigned StartBit;
     unsigned NumBits;
@@ -600,18 +603,22 @@ class DecoderTableBuilder {
   ArrayRef<InstructionEncoding> Encodings;
   DecoderTableInfo &TableInfo;
 
+  bool HasConflict = false;
+
 public:
   DecoderTableBuilder(const CodeGenTarget &Target,
                       ArrayRef<InstructionEncoding> Encodings,
                       DecoderTableInfo &TableInfo)
       : Target(Target), Encodings(Encodings), TableInfo(TableInfo) {}
 
-  void buildTable(const FilterChooser &FC, unsigned BitWidth) const {
+  /// Returns true if a decoding conflict was encountered.
+  bool buildTable(const FilterChooser &FC, unsigned BitWidth) {
     // When specializing decoders per bit width, each decoder table will begin
     // with the bitwidth for that table.
     if (SpecializeDecodersPerBitwidth)
       TableInfo.Table.insertULEB128(BitWidth);
     emitTableEntries(FC);
+    return HasConflict;
   }
 
 private:
@@ -637,7 +644,7 @@ class DecoderTableBuilder {
 
   void emitSingletonTableEntry(const FilterChooser &FC) const;
 
-  void emitTableEntries(const FilterChooser &FC) const;
+  void emitTableEntries(const FilterChooser &FC);
 };
 
 } // end anonymous namespace
@@ -1592,7 +1599,7 @@ void FilterChooser::doFilter() {
   // Print out useful conflict information for postmortem analysis.
   errs() << "Decoding Conflict:\n";
   dump();
-  PrintFatalError("Decoding conflict encountered");
+  HasConflict = true;
 }
 
 void FilterChooser::dump() const {
@@ -1612,7 +1619,12 @@ void FilterChooser::dump() const {
   }
 }
 
-void DecoderTableBuilder::emitTableEntries(const FilterChooser &FC) const {
+void DecoderTableBuilder::emitTableEntries(const FilterChooser &FC) {
+  if (FC.HasConflict) {
+    HasConflict = true;
+    return;
+  }
+
   DecoderTable &Table = TableInfo.Table;
 
   // If there are other encodings that could match if those with all bits
@@ -2570,6 +2582,7 @@ template <typename T> constexpr uint32_t InsnBitWidth = 0;
   DecoderTableBuilder TableBuilder(Target, Encodings, TableInfo);
   unsigned OpcodeMask = 0;
 
+  bool HasConflict = false;
   for (const auto &[BitWidth, BWMap] : EncMap) {
     for (const auto &[Key, EncodingIDs] : BWMap) {
       auto [DecoderNamespace, HwModeID] = Key;
@@ -2585,7 +2598,10 @@ template <typename T> constexpr uint32_t InsnBitWidth = 0;
       //    across all decoder tables.
       //  - predicates are shared across all decoder tables.
       TableInfo.Table.clear();
-      TableBuilder.buildTable(FC, BitWidth);
+      HasConflict |= TableBuilder.buildTable(FC, BitWidth);
+      // Skip emitting table entries if a conflict has been detected.
+      if (HasConflict)
+        continue;
 
       // Print the table to the output stream.
       OpcodeMask |= emitTable(OS, TableInfo.Table, DecoderNamespace, HwModeID,
@@ -2600,6 +2616,9 @@ template <typename T> constexpr uint32_t InsnBitWidth = 0;
     }
   }
 
+  if (HasConflict)
+    PrintFatalError("Decoding conflict encountered");
+
   // Emit the decoder function for the last bucket. This will also emit the
   // single decoder function if SpecializeDecodersPerBitwidth = false.
   if (!SpecializeDecodersPerBitwidth)