[TableGen] Add mapping from processor ID to resource index for packetizer #158182

LuoYuanke · 2025-09-12T03:49:26Z

Tablegen would generate code to access TargetResourceIndices with processor ID.
The TargetProcResourceIndexStart[] array is generated for each processor which
has itineraries. The processor which doesn't has itineraries is excluded from
the array. When a target has mixed processors, the processor ID may exceed the
array size and cause the error.
This patch is to generate a table mapping processor with itineraries to resource
index, so that scheduler can get the correct resource index with processor ID.

llvmbot · 2025-09-12T03:49:55Z

@llvm/pr-subscribers-backend-amdgpu

@llvm/pr-subscribers-tablegen

Author: Luo, Yuanke (LuoYuanke)

Changes

Tablegen would generate code to access TargetResourceIndices with processor ID.
The TargetProcResourceIndexStart[] array is generated for each processor which
has itineraries. The processor which doesn't has itineraries is excluded from
the array. When a target has mixed processors, the processor ID may exceed the
array size and cause the error.
This patch is to generate a table mapping processor with itineraries to resource
index, so that scheduler can get the correct resource index with processor ID.

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

2 Files Affected:

(added) llvm/test/TableGen/DFAPacketizer.td (+31)
(modified) llvm/utils/TableGen/DFAPacketizerEmitter.cpp (+15-4)

diff --git a/llvm/test/TableGen/DFAPacketizer.td b/llvm/test/TableGen/DFAPacketizer.td
new file mode 100644
index 0000000000000..b4c05c63811b5
--- /dev/null
+++ b/llvm/test/TableGen/DFAPacketizer.td
@@ -0,0 +1,31 @@
+// RUN: llvm-tblgen -gen-dfa-packetizer -I %p/../../include %s | FileCheck %s
+
+include "llvm/Target/Target.td"
+
+def TestTarget : Target;
+
+def TestSchedModel : SchedMachineModel {
+  let CompleteModel = 0;
+}
+
+def TestProcessor1 : ProcessorModel<"testprocessor1", TestSchedModel, []>;
+
+def FU0 : FuncUnit;
+def FU1 : FuncUnit;
+
+def OP0 : InstrItinClass;
+def OP1 : InstrItinClass;
+
+def Itin {
+  list<InstrItinData> ItinList = [
+    InstrItinData<OP0, [InstrStage<1, [FU0]>]>,
+    InstrItinData<OP1, [InstrStage<1, [FU1]>]>,
+  ];
+}
+
+// CHECK: std::map<unsigned, unsigned> TestTargetProcIdToResourceIndexStartMapping = {
+// CHECK-NEXT:   { 2,  1 }, // TestItinerariesModel
+// CHECK-NEXT: };
+
+def TestItineraries: ProcessorItineraries<[], [], Itin.ItinList>;
+def TestProcessor2 : Processor<"testprocessor2", TestItineraries, []>;
diff --git a/llvm/utils/TableGen/DFAPacketizerEmitter.cpp b/llvm/utils/TableGen/DFAPacketizerEmitter.cpp
index 8cb2c22736f8a..0fd30411451a2 100644
--- a/llvm/utils/TableGen/DFAPacketizerEmitter.cpp
+++ b/llvm/utils/TableGen/DFAPacketizerEmitter.cpp
@@ -256,6 +256,16 @@ void DFAPacketizerEmitter::emitForItineraries(
   }
   OS << "\n};\n\n";
 
+  // Output the mapping from proc ID to ResourceIndexStart
+  Idx = 1;
+  OS << "std::map<unsigned, unsigned> " << TargetName << DFAName
+     << "ProcIdToResourceIndexStartMapping = {\n";
+  for (const CodeGenProcModel *Model : ProcModels) {
+    OS << "  { " << Model->Index << ",  " << Idx++ << " }, // "
+       << Model->ModelName << "\n";
+  }
+  OS << "};\n\n";
+
   // And the mapping from Itinerary index into the previous table.
   OS << "constexpr unsigned " << TargetName << DFAName
      << "ProcResourceIndexStart[] = {\n";
@@ -339,16 +349,17 @@ void DFAPacketizerEmitter::emitForItineraries(
 
   std::string SubTargetClassName = TargetName + "GenSubtargetInfo";
   OS << "namespace llvm {\n";
-  OS << "DFAPacketizer *" << SubTargetClassName << "::"
-     << "create" << DFAName
+  OS << "DFAPacketizer *" << SubTargetClassName << "::" << "create" << DFAName
      << "DFAPacketizer(const InstrItineraryData *IID) const {\n"
      << "  static Automaton<uint64_t> A(ArrayRef<" << TargetAndDFAName
      << "Transition>(" << TargetAndDFAName << "Transitions), "
      << TargetAndDFAName << "TransitionInfo);\n"
+     << "  unsigned Index = " << TargetName << DFAName
+     << "ProcIdToResourceIndexStartMapping[IID->SchedModel.ProcID];\n"
      << "  unsigned ProcResIdxStart = " << TargetAndDFAName
-     << "ProcResourceIndexStart[IID->SchedModel.ProcID];\n"
+     << "ProcResourceIndexStart[Index];\n"
      << "  unsigned ProcResIdxNum = " << TargetAndDFAName
-     << "ProcResourceIndexStart[IID->SchedModel.ProcID + 1] - "
+     << "ProcResourceIndexStart[Index + 1] - "
         "ProcResIdxStart;\n"
      << "  return new DFAPacketizer(IID, A, {&" << TargetAndDFAName
      << "ResourceIndices[ProcResIdxStart], ProcResIdxNum});\n"

arsenm · 2025-09-15T01:22:58Z

llvm/test/TableGen/DFAPacketizer.td

This is emitting a static constructor for a std::map. Can you emit this as a sorted static table instead which doesn't need construction

HaohaiWen · 2025-09-15T03:12:33Z

llvm/utils/TableGen/DFAPacketizerEmitter.cpp

Should this table be placed after ProcResourceIndexStart?

HaohaiWen · 2025-09-15T05:27:05Z

llvm/utils/TableGen/DFAPacketizerEmitter.cpp

The TargetProcResourceIndexStart[] array is generated for each processor which has itineraries. The processor which doesn't has itineraries is excluded from the array.

Is it possible that createDFAPacketizer accesses an IID that has no itinerary. (i.e. No such key exists in ProcIdToResourceIndexStartMapping?

I think it is not possible. DFAPacketizer relies on itinerary and API createDFAPacketizer(InstrItineraryData *) require the itinerary data.

HaohaiWen · 2025-09-15T08:30:32Z

llvm/utils/TableGen/DFAPacketizerEmitter.cpp

llvm::lower_bound/upper_bound?

github-actions · 2025-09-15T11:30:13Z

✅ With the latest revision this PR passed the C/C++ code formatter.

HaohaiWen · 2025-09-16T00:59:08Z

llvm/test/TableGen/DFAPacketizer.td

NIT: I guess llvm::lower_bound supports unsigned[][].

llvm::lower_bound(TestTargetProcIdToProcResourceIdxTable, [](const unsigned LHS[], unsigned Val) { return LHS[0] < Val; });

I got compiling error with below code. I'd like keep this code in this patch and revise it when there is a better way to call lower_bound() for unsigned arr[][].

#include <algorithm> #include <utility> #include <vector> #include <stdio.h> #include <assert.h> int TestTargetGetResourceIndex(unsigned ProcID) { static const unsigned TestTargetProcIdToProcResourceIdxTable[][2] = { { 2, 1 }, { 4, 2 }, { 8, 3 }, { 9, 4 }, }; auto It = std::lower_bound( std::begin(TestTargetProcIdToProcResourceIdxTable), std::end(TestTargetProcIdToProcResourceIdxTable), ProcID); // auto It = std::lower_bound( // std::begin(TestTargetProcIdToProcResourceIdxTable), // std::end(TestTargetProcIdToProcResourceIdxTable), ProcID, // [](const unsigned LHS[], unsigned Val) { return LHS[0] < Val; }); assert(*It[0] == ProcID); return (*It)[1]; } int main() { printf("2 : %d\n", TestTargetGetResourceIndex(2)); printf("9 : %d\n", TestTargetGetResourceIndex(9)); }

I mean something like:

auto It = llvm::lower_bound(TestTargetProcIdToProcResourceIdxTable, ProcID, [](const unsigned LHS[], unsigned Val) { return LHS[0] < Val; });

See

llvm-project/llvm/include/llvm/ADT/STLExtras.h

Lines 1982 to 1985 in cedceeb

auto lower_bound(R &&Range, T &&Value, Compare C) {

return std::lower_bound(adl_begin(Range), adl_end(Range),

std::forward<T>(Value), C);

}

HaohaiWen

LGTM

HaohaiWen · 2025-09-16T01:05:45Z

llvm/test/TableGen/DFAPacketizer.td

Add assert(*It[0] == ProcID) here to avoid unnecessary bug.?

Good suggestion. Added assert.

…izer Tablegen would generate code to access TargetResourceIndices with processor ID. The TargetProcResourceIndexStart[] array is generated for each processor which has itineraries. The processor which doesn't has itineraries is excluded from the array. When a target has mixed processors, the processor ID may exceed the array size and cause the error. This patch is to generate a table mapping processor with itineraries to resource index, so that scheduler can get the correct resource index with processor ID.

LuoYuanke · 2025-09-17T00:35:25Z

All comments have been addressed. If there is no more comment, @HaohaiWen, could you help to land the patch?

llvmbot added the tablegen label Sep 12, 2025

LuoYuanke requested review from phoebewang, KanRobert, jmolloy, HaohaiWen, jurahul and arsenm September 12, 2025 03:52

arsenm reviewed Sep 15, 2025

View reviewed changes

HaohaiWen reviewed Sep 15, 2025

View reviewed changes

LuoYuanke requested review from arsenm and HaohaiWen September 15, 2025 08:11

HaohaiWen reviewed Sep 15, 2025

View reviewed changes

LuoYuanke requested a review from HaohaiWen September 15, 2025 11:33

HaohaiWen reviewed Sep 16, 2025

View reviewed changes

HaohaiWen approved these changes Sep 16, 2025

View reviewed changes

HaohaiWen reviewed Sep 16, 2025

View reviewed changes

LuoYuanke force-pushed the DFAPacketizer-fix branch from ab868eb to 9ae9df6 Compare September 16, 2025 02:06

Yuanke Luo added 6 commits September 16, 2025 10:52

[TableGen] Address Matt and Haohai's comments

9d25fe7

[TableGen] Address Haohai's comments

26ac7b6

[TableGen] Fix clang format issue

64e6793

[TableGen] Address Haohai's comments

a04ae23

[Packetizer] Fix lit test failure

409b19f

LuoYuanke force-pushed the DFAPacketizer-fix branch from 9ae9df6 to 409b19f Compare September 16, 2025 06:38

llvmbot added the backend:AMDGPU label Sep 16, 2025

[TableGen] Address Haohai's comments

65159da

HaohaiWen merged commit cbd99c5 into llvm:main Sep 17, 2025
9 checks passed

LuoYuanke mentioned this pull request Sep 20, 2025

Request Commit Access For LuoYuanke #159920

Closed

	auto lower_bound(R &&Range, T &&Value, Compare C) {
	return std::lower_bound(adl_begin(Range), adl_end(Range),
	std::forward<T>(Value), C);
	}

[TableGen] Add mapping from processor ID to resource index for packetizer #158182

[TableGen] Add mapping from processor ID to resource index for packetizer #158182

Uh oh!

Conversation

LuoYuanke commented Sep 12, 2025

Uh oh!

llvmbot commented Sep 12, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

HaohaiWen Sep 15, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

github-actions bot commented Sep 15, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

HaohaiWen Sep 16, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

HaohaiWen left a comment

Choose a reason for hiding this comment

Uh oh!

HaohaiWen Sep 16, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

LuoYuanke commented Sep 17, 2025

Uh oh!

Uh oh!

Uh oh!

llvmbot commented Sep 12, 2025 •

edited

Loading

HaohaiWen Sep 15, 2025 •

edited

Loading

github-actions bot commented Sep 15, 2025 •

edited

Loading

HaohaiWen Sep 16, 2025 •

edited

Loading

HaohaiWen Sep 16, 2025 •

edited

Loading