Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 7 commits into from
Sep 17, 2025
Merged

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
311a4b8
[TableGen] Add mapping from processor ID to resource index for packet…
Sep 11, 2025
9d25fe7
[TableGen] Address Matt and Haohai's comments
Sep 15, 2025
26ac7b6
[TableGen] Address Haohai's comments
Sep 15, 2025
64e6793
[TableGen] Fix clang format issue
Sep 15, 2025
a04ae23
[TableGen] Address Haohai's comments
Sep 16, 2025
409b19f
[Packetizer] Fix lit test failure
Sep 16, 2025
65159da
[TableGen] Address Haohai's comments
Sep 16, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions llvm/lib/Target/AMDGPU/R600Packetizer.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -319,6 +319,11 @@ bool R600Packetizer::runOnMachineFunction(MachineFunction &Fn) {

MachineLoopInfo &MLI = getAnalysis<MachineLoopInfoWrapperPass>().getLI();

const InstrItineraryData *II = ST.getInstrItineraryData();
// If there is no itineraries information, abandon.
if (II->Itineraries == nullptr)
return false;

// Instantiate the packetizer.
R600PacketizerList Packetizer(Fn, ST, MLI);

Expand Down
39 changes: 39 additions & 0 deletions llvm/test/TableGen/DFAPacketizer.td
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
// 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: int TestTargetGetResourceIndex(unsigned ProcID) {
// CHECK-NEXT: static const unsigned TestTargetProcIdToProcResourceIdxTable[][2] = {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

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

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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));
}

Copy link
Contributor

@HaohaiWen HaohaiWen Sep 16, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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);
}

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

revised.

// CHECK-NEXT: { 2, 1 }, // TestItinerariesModel
// CHECK-NEXT: };
// CHECK-NEXT: auto It = llvm::lower_bound(TestTargetProcIdToProcResourceIdxTable, ProcID,
// CHECK-NEXT: [](const unsigned LHS[], unsigned Val) { return LHS[0] < Val; });
// CHECK-NEXT: assert(*It[0] == ProcID);
// CHECK-NEXT: return (*It)[1];
Copy link
Contributor

@HaohaiWen HaohaiWen Sep 16, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good suggestion. Added assert.

// CHECK-NEXT: }

// CHECK: unsigned Index = TestTargetGetResourceIndex(IID->SchedModel.ProcID);

def TestItineraries: ProcessorItineraries<[], [], Itin.ItinList>;
def TestProcessor2 : Processor<"testprocessor2", TestItineraries, []>;
28 changes: 24 additions & 4 deletions llvm/utils/TableGen/DFAPacketizerEmitter.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -266,6 +266,25 @@ void DFAPacketizerEmitter::emitForItineraries(
}
OS << " " << ScheduleClasses.size() << "\n};\n\n";

// Output the mapping from proc ID to ResourceIndexStart
Idx = 1;
OS << "int " << TargetName << DFAName
<< "GetResourceIndex(unsigned ProcID) { \n"
<< " static const unsigned " << TargetName << DFAName
<< "ProcIdToProcResourceIdxTable[][2] = {\n";
for (const CodeGenProcModel *Model : ProcModels) {
OS << " { " << Model->Index << ", " << Idx++ << " }, // "
<< Model->ModelName << "\n";
}
OS << " };\n"
<< " auto It = llvm::lower_bound(" << TargetName << DFAName
<< "ProcIdToProcResourceIdxTable, ProcID,\n"
<< " [](const unsigned LHS[], unsigned Val) { return LHS[0] < Val; "
"});\n"
<< " assert(*It[0] == ProcID);\n"
<< " return (*It)[1];\n"
<< "}\n\n";

// The type of a state in the nondeterministic automaton we're defining.
using NfaStateTy = uint64_t;

Expand Down Expand Up @@ -339,16 +358,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
Copy link
Contributor

@HaohaiWen HaohaiWen Sep 15, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more. 

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?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

<< "GetResourceIndex(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"
Expand Down