Skip to content

[AIE] Consolidate itinerary queries over register supersets#999

Open
martien-de-jong wants to merge 1 commit into
aie-publicfrom
martien.consolidate-regitinpairs
Open

[AIE] Consolidate itinerary queries over register supersets#999
martien-de-jong wants to merge 1 commit into
aie-publicfrom
martien.consolidate-regitinpairs

Conversation

@martien-de-jong
Copy link
Copy Markdown
Collaborator

Lo-priority

Generated reg itin pairs tend to split register sets in their subsets, all yielding the same itinerary.
By recognising those equivalences and reverting to the largest superset we gain several advantages:
1- There are less entries to check. This saves table size and compile time
spent in table lookup.
2- For reverse-lookup purposes, i.e. finding register classes compatible with
physical register occurrences, we end up at wider sets with more allocation
freedom.

StringRef TimingModelName;

bool operator==(const StageCycleInfo &Other) const {
return Cycles == Other.Cycles && UnitNames == Other.UnitNames &&
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

nit: check cheaper first, also, could we have early returns? Cycles -> TimingModelName -> UnitNames? Maybe we can save some table build time.

bool operator<(const StageCycleInfo &Other) const {
if (Cycles != Other.Cycles)
return Cycles < Other.Cycles;
if (UnitNames != Other.UnitNames)
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This could be the last test.

/// \param ItinName The name of the itinerary to look up.
/// \returns The signature for the itinerary. If not found, returns an empty
/// signature.
ItinerarySignature getSignature(StringRef ItinName) const {
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

optional?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Actually, unused function...


// First check that all input classes have compatible attributes.
for (size_t I = 1; I < RCs.size(); ++I) {
if (!haveCompatibleAttributes(*RCs[0], *RCs[I])) {
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Check: this is intended to speedup the whole strategy by discarding obvious cases.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

For me it's correctness. I shouldn't merge reg classes that aren't equivalent in e.g. spill size. But perhaps there are sanity rules that I don't know of.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

More detail: I check the result class only against RC[0].

Generated reg itin pairs tend to split register sets in their subsets, all
yielding the same itinerary.
By recognising those equivalences and reverting to the largest superset we
gain several advantages:
1- There are less entries to check. This saves table size and compile time
   spent in table lookup.
2- For reverse-lookup purposes, i.e. finding register classes compatible with
   physical register occurrences, we end up at wider sets with more allocation
   freedom.
@martien-de-jong martien-de-jong force-pushed the martien.consolidate-regitinpairs branch from 0a0709d to 66f1b53 Compare May 18, 2026 10:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants