[llvm] [llvm-tblgen] Increase Coverage Index Size (PR #118329)

Sam Elliott via llvm-commits llvm-commits at lists.llvm.org
Mon Dec 2 09:54:29 PST 2024


https://github.com/lenary created https://github.com/llvm/llvm-project/pull/118329

The tablegen backend for DAGISel can, using the `-instrument-coverage` option, save information about which patterns are used. This saves an index into two tables, one that contains a string dump of the pattern, and another that contains the path the pattern came from.

Before this change, these indexes were an unsigned 16-bit value.

The RISC-V backend, however, goes beyond this limit. This change increases the indexes to be represented by a 32-bit unsigned value.

`-instrument-coverage` is not enabled by default, so this should have minimal effect on the default configuration's code size. This is partly why I also just chose to double the size (from 16-bit to 32-bit) rather than going to 24-bit. Hopefully this size will never need to be changed again.

Fixes #118259

>From 301e321620a49659648b433771e341c6c16aa819 Mon Sep 17 00:00:00 2001
From: Sam Elliott <quic_aelliott at quicinc.com>
Date: Mon, 2 Dec 2024 09:43:24 -0800
Subject: [PATCH] [llvm-tblgen] Increase Coverage Index Size

The tablegen backend for DAGISel can, using the `-instrument-coverage`
option, save information about which patterns are used. This saves an
index into two tables, one that contains a string dump of the pattern,
and another that contains the path the pattern came from.

Before this change, these indexes were an unsigned 16-bit value.

The RISC-V backend, however, goes beyond this limit. This change
increases the indexes to be represented by a 32-bit unsigned value.

`-instrument-coverage` is not enabled by default, so this should have
minimal effect on the default configuration's code size. This is partly
why I also just chose to double the size (from 16-bit to 32-bit) rather
than going to 24-bit. Hopefully this size will never need to be changed
again.

Fixes #118259
---
 llvm/include/llvm/CodeGen/SelectionDAGISel.h  |  2 +-
 .../CodeGen/SelectionDAG/SelectionDAGISel.cpp |  2 ++
 llvm/test/TableGen/dag-isel-instrument.td     | 32 +++++++++++++++++++
 llvm/utils/TableGen/DAGISelMatcherEmitter.cpp | 14 +++++---
 4 files changed, 44 insertions(+), 6 deletions(-)
 create mode 100644 llvm/test/TableGen/dag-isel-instrument.td

diff --git a/llvm/include/llvm/CodeGen/SelectionDAGISel.h b/llvm/include/llvm/CodeGen/SelectionDAGISel.h
index f99ec4651009a0..43ba8f4c44cf9c 100644
--- a/llvm/include/llvm/CodeGen/SelectionDAGISel.h
+++ b/llvm/include/llvm/CodeGen/SelectionDAGISel.h
@@ -315,7 +315,7 @@ class SelectionDAGISel {
     OPC_MorphNodeTo1GlueOutput,
     OPC_MorphNodeTo2GlueOutput,
     OPC_CompleteMatch,
-    // Contains offset in table for pattern being selected
+    // Contains 32-bit offset in table for pattern being selected
     OPC_Coverage
   };
 
diff --git a/llvm/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp b/llvm/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp
index 0d99ae9cdebd50..3000dfda1bea03 100644
--- a/llvm/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp
+++ b/llvm/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp
@@ -4045,6 +4045,8 @@ void SelectionDAGISel::SelectCodeCommon(SDNode *NodeToMatch,
       // So it should be safe to assume that this node has been selected
       unsigned index = MatcherTable[MatcherIndex++];
       index |= (MatcherTable[MatcherIndex++] << 8);
+      index |= (MatcherTable[MatcherIndex++] << 16);
+      index |= (MatcherTable[MatcherIndex++] << 24);
       dbgs() << "COVERED: " << getPatternForIndex(index) << "\n";
       dbgs() << "INCLUDED: " << getIncludePathForIndex(index) << "\n";
       continue;
diff --git a/llvm/test/TableGen/dag-isel-instrument.td b/llvm/test/TableGen/dag-isel-instrument.td
new file mode 100644
index 00000000000000..2862af03cf16da
--- /dev/null
+++ b/llvm/test/TableGen/dag-isel-instrument.td
@@ -0,0 +1,32 @@
+// RUN: llvm-tblgen -gen-dag-isel -instrument-coverage -I %p/../../include %s | FileCheck %s
+
+include "llvm/Target/Target.td"
+
+def TestTargetInstrInfo : InstrInfo;
+
+def TestTarget : Target {
+  let InstructionSet = TestTargetInstrInfo;
+}
+
+def REG : Register<"REG">;
+def GPR : RegisterClass<"TestTarget", [i32], 32, (add REG)>;
+
+// CHECK-LABEL: OPC_CheckOpcode, TARGET_VAL(ISD::UDIVREM)
+// CHECK: OPC_EmitNode2None, TARGET_VAL(::INSTR)
+// CHECK-NEXT: Results = #2 #3
+// CHECK-NEXT: OPC_Coverage, COVERAGE_IDX_VAL(0),
+// CHECK-NEXT: OPC_CompleteMatch, 2, 3, 2
+def INSTR : Instruction {
+  let OutOperandList = (outs GPR:$r1, GPR:$r0);
+  let InOperandList = (ins GPR:$t0, GPR:$t1);
+  let Pattern = [(set i32:$r0, i32:$r1, (udivrem i32:$t0, i32:$t1))];
+}
+
+
+// CHECK: getPatternForIndex(unsigned Index)
+// CHECK: static const char *PATTERN_MATCH_TABLE[]
+// CHECK: return StringRef(PATTERN_MATCH_TABLE[Index]);
+
+// CHECK: getIncludePathForIndex(unsigned Index)
+// CHECK: static const char *INCLUDE_PATH_TABLE[]
+// CHECK: return StringRef(INCLUDE_PATH_TABLE[Index]);
diff --git a/llvm/utils/TableGen/DAGISelMatcherEmitter.cpp b/llvm/utils/TableGen/DAGISelMatcherEmitter.cpp
index 74c399ee423176..2145284ab29e22 100644
--- a/llvm/utils/TableGen/DAGISelMatcherEmitter.cpp
+++ b/llvm/utils/TableGen/DAGISelMatcherEmitter.cpp
@@ -381,8 +381,8 @@ static void EndEmitFunction(raw_ostream &OS) {
 
 void MatcherTableEmitter::EmitPatternMatchTable(raw_ostream &OS) {
 
-  assert(isUInt<16>(VecPatterns.size()) &&
-         "Using only 16 bits to encode offset into Pattern Table");
+  assert(isUInt<32>(VecPatterns.size()) &&
+         "Using only 32 bits to encode offset into Pattern Table");
   assert(VecPatterns.size() == VecIncludeStrings.size() &&
          "The sizes of Pattern and include vectors should be the same");
 
@@ -947,7 +947,7 @@ unsigned MatcherTableEmitter::EmitMatcher(const Matcher *N,
         std::string include_src = getIncludePath(PatRecord);
         unsigned Offset =
             getPatternIdxFromTable(src + " -> " + dst, std::move(include_src));
-        OS << "TARGET_VAL(" << Offset << "),\n";
+        OS << "COVERAGE_IDX_VAL(" << Offset << "),\n";
         OS.indent(FullIndexWidth + Indent);
       }
     }
@@ -1060,7 +1060,7 @@ unsigned MatcherTableEmitter::EmitMatcher(const Matcher *N,
       std::string include_src = getIncludePath(PatRecord);
       unsigned Offset =
           getPatternIdxFromTable(src + " -> " + dst, std::move(include_src));
-      OS << "TARGET_VAL(" << Offset << "),\n";
+      OS << "COVERAGE_IDX_VAL(" << Offset << "),\n";
       OS.indent(FullIndexWidth + Indent);
     }
     OS << "OPC_CompleteMatch, " << CM->getNumResults() << ", ";
@@ -1393,8 +1393,11 @@ void llvm::EmitMatcherTable(Matcher *TheMatcher, const CodeGenDAGPatterns &CGP,
   // final stream.
   OS << "{\n";
   OS << "  // Some target values are emitted as 2 bytes, TARGET_VAL handles\n";
-  OS << "  // this.\n";
+  OS << "  // this. Coverage indexes are emitted as 4 bytes,\n";
+  OS << "  // COVERAGE_IDX_VAL handles this.\n";
   OS << "  #define TARGET_VAL(X) X & 255, unsigned(X) >> 8\n";
+  OS << "  #define COVERAGE_IDX_VAL(X) X & 255, (unsigned(X) >> 8) & 255, ";
+  OS << "(unsigned(X) >> 16) & 255, (unsigned(X) >> 24) & 255\n";
   OS << "  static const unsigned char MatcherTable[] = {\n";
   TotalSize = MatcherEmitter.EmitMatcherList(TheMatcher, 1, 0, OS);
   OS << "    0\n  }; // Total Array size is " << (TotalSize + 1)
@@ -1402,6 +1405,7 @@ void llvm::EmitMatcherTable(Matcher *TheMatcher, const CodeGenDAGPatterns &CGP,
 
   MatcherEmitter.EmitHistogram(TheMatcher, OS);
 
+  OS << "  #undef COVERAGE_IDX_VAL\n";
   OS << "  #undef TARGET_VAL\n";
   OS << "  SelectCodeCommon(N, MatcherTable, sizeof(MatcherTable));\n";
   OS << "}\n";



More information about the llvm-commits mailing list