[AIEX] Fix invalid remat after super-reg rewrite/unbundle#987
Merged
Conversation
niwinanto
requested review from
F-Stuckmann,
SagarMaheshwari99,
abhinay-anubola,
abnikant,
andcarminati,
katerynamuts,
khallouh,
konstantinschwarz,
martien-de-jong,
mludevid and
stephenneuendorffer
as code owners
niwinanto
commented
May 5, 2026
| ; CHECK-NEXT: {{ $}} | ||
| ; CHECK-NEXT: [[MOV_RLC_imm11_pseudo2:%[0-9]+]]:er = MOV_RLC_imm11_pseudo 1 | ||
| ; CHECK-NEXT: [[AND1:%[0-9]+]]:er = AND [[COPY]], [[MOV_RLC_imm11_pseudo2]] | ||
| ; CHECK-NEXT: [[MOV_PD_imm11_pseudo4:%[0-9]+]]:eds = MOV_PD_imm11_pseudo 128 |
Collaborator
Author
There was a problem hiding this comment.
This was an illegal copy that we have fixed now.
niwinanto
force-pushed
the
niwin.splitmap.repair
branch
4 times, most recently
from
c68212a to
f2b1ad1
Compare
konstantinschwarz
previously approved these changes
May 7, 2026
| DebugVars); | ||
|
|
||
| // Prevent SplitKit from rematerializing through stale ancestor LIs. | ||
| LLVM_DEBUG(dbgs() << "Clearing stale split-from mappings...\n"); |
Collaborator
There was a problem hiding this comment.
nit: this will emit debug prints twice, once here, once in clearStaleSplitFromMappings itself.
Drop the print here?
niwinanto
force-pushed
the
niwin.splitmap.repair
branch
from
f2b1ad1 to
31d4767
Compare
Collaborator
|
Hi Niwin, could you please share the current QoR status of this fix? |
| # RUN: llc -mtriple=aie2p -start-before=greedy %s -o /dev/null --filetype=obj | ||
|
|
||
| # RUN: : not --crash llc -mtriple=aie2p -start-before=greedy %s -o /dev/null --filetype=obj | ||
| # RUN: llc -mtriple=aie2p -start-before=greedy -stop-before=virtregrewriter %s -o - | FileCheck %s |
Collaborator
There was a problem hiding this comment.
Check: with -stop-before=virtregrewrite we currently prevent the crash, right?
Collaborator
Author
There was a problem hiding this comment.
You are right, we do the wrong remat in greedy and crash during code emission. Reason I stopped before virtregrewriter is to minimize the change to MIR(still use virtual registers) so that wrong remat is clear. Also, I kept the run line to reproduce the crash.
Collaborator
Author
|
@andcarminati Topk1d tests are fragile, as their inputs change from run to run and they strongly affect the execution profile, generated code is literally same between 2 runs. And regarding the improvements it reports, RA has not really reclaimed all the regressions but it changed the allocation and that act as an enabler for the pipeliner and we have better code in the end. On the other side, Core_PM regress for these kernels. No change to Core_StackSize. |
niwinanto
force-pushed
the
niwin.splitmap.repair
branch
from
754b4d6 to
3f4ae1b
Compare
added 3 commits
May 19, 2026 05:18
LiveRange::clear() destructively mutates a shared LiveInterval; any cached reference held by greedy (spill weights, queue entries, hint reconciliation) is left dangling.
AIESuperRegRewriter and AIEUnallocatedSuperRegRewriter rewrite or unbundle instructions in place but leave the LiveInterval of the VRM "Original" ancestor untouched. Its VNInfos still point at slots whose MIs have changed, so a later Greedy split that consults OrigLI via SplitEditor::defFromParent can rematerialize the wrong instruction (e.g. a sublane-only MOV becomes a full-composite def). Add AIESuperRegUtils::clearStaleSplitFromMappings and call it from both passes. It clears the VRM split-from mapping for every descendant of a touched Original via the new VirtRegMap::clearSplitFromReg API, restoring each descendant to the canonical "no split parent" state of a freshly created vreg. SplitKit then reads the descendant's own repaired LI rather than the stale ancestor's, and predicates that examine Virt2SplitMap directly (e.g. isAssignedReg) no longer treat the descendant as a split product. OrigLI is left untouched.
The earlier "Sever stale VRM split-from chain after super-reg rewrites" commit calls VRM.clearSplitFromReg() on descendants whose ancestor LiveInterval has gone stale, so SplitKit::defFromParent no longer rematerializes through it. A side effect is that InlineSpiller can no longer recognise sibling vregs as belonging to the same logical group (VRM.getOriginal() now returns the descendant itself), so each sibling is assigned its own stack slot instead of sharing one. Add a target hook TargetSubtargetInfo::getSpillGroupOriginal that lets a target advertise a "logical group original" for stack-slot sharing in InlineSpiller, independent of the VRM split-from chain. Defaults to no override. Plumb it through AIE: - AIEMachineFunctionInfo gains a side map (descendant -> pre-severance Original) with record/get accessors. - AIESuperRegUtils::clearStaleSplitFromMappings records the chain into that side map before calling VRM.clearSplitFromReg(). - AIEBaseSubtarget overrides getSpillGroupOriginal to forward to AIEMachineFunctionInfo. - Callers (AIE[Unallocated]SuperRegRewriter) updated to pass MF.
niwinanto
force-pushed
the
niwin.splitmap.repair
branch
from
3f4ae1b to
23b474e
Compare
andcarminati
approved these changes
May 19, 2026
Collaborator
andcarminati
left a comment
andcarminati
left a comment
There was a problem hiding this comment.
LGTM. Thanks for fixing this!
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Greedy register allocation can produce an invalid rematerialization after the AIE register-rewriter passes run, because those passes modify the IR in place but leave the LiveInterval of the VRM "Original" ancestor stale.
SplitKit::defFromParentwalks back throughVRM.getOriginal()and uses that staleOrigLIto pick a remat candidate, which can clone an instruction whose semantics no longer match the destination's register class.