Skip to content

Conversation

@F-Stuckmann
Copy link
Collaborator

@F-Stuckmann F-Stuckmann commented Oct 24, 2025

Creating a combined register class for vector and accumulator registers changes the machine scheduling behavior because the combined register class is now used for register pressure calculation in the machine scheduler.
Therefore, vector and accumulator registers are considered as a single register pressure class and thus the machine scheduler ever so slightly changes the scheduling. This scheduling change changes register liveliness and causes spilling (sometimes even in the innermost loop).

To mitigate this:

  1. Remove the combined register class for register pressure calculation during machine scheduling, so the machine scheduling does not change.
  2. Updating WAW rewriter strategy to reach good IIs.

Things to update in this PR:

  • unit tests
  • fine tuning on the latest QoR

P.S. For my sanity I continue #678 here

@F-Stuckmann F-Stuckmann changed the base branch from aie-public to andreu.waw.rewriter.latency.aware October 24, 2025 12:02
@andcarminati andcarminati force-pushed the andreu.waw.rewriter.latency.aware branch 3 times, most recently from 401e86b to 30888fc Compare October 24, 2025 14:59
@F-Stuckmann
Copy link
Collaborator Author

QoR numbers.
The comparison is [this PR + https://github.com//pull/682 ] vs #682
-0.33% performance improvement
Core_Compute_Insn_Count

Core_PMSize_perf_sorted_absolute Core_StackSize_perf_sorted_absolute

return {{AIE2P::VST_dmx_sts_x_spill, AIE2P::sub_bfp16_x},
{AIE2P::VST_E_SPILL, AIE2P::sub_bfp16_e}};
case AIE2P::VLDA_512_COMPOSED_REG_SPILL:
return {};
Copy link
Collaborator

Choose a reason for hiding this comment

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

This says 'nothing to expand' All other cases return something, so I'm wondering whether this should fall under
if (!MI.isPseudo()) in the top of the function


// // Ignore Reg Pressure Set ID
const unsigned PSID = i;
if (std::find(IgnorePressureSetIDs.begin(), IgnorePressureSetIDs.end(),
Copy link
Collaborator

Choose a reason for hiding this comment

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

Discussed offline, maybe we should try a single target hook, to which we pass the ID. All IDs a given by an enum, so it should be easy for the target to handle them.

Opcode = AIE2P::ST_R_SPILL;
SrcReg = ScratchReg;
IsKill = true;
} else if (&AIE2P::spill_vec512_to_compositeRegClass == RC) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm wondering why we don't use regClassMatches() here.

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.

4 participants