[AArch64] Prioritize regalloc hints over movprfx hints #167480
Merged
Conversation
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
Ensures the hints are only added once, and ensures that hints inserted by the register allocator take priority over hints to reduce movprfx.
Member
|
@llvm/pr-subscribers-backend-aarch64 Author: Sander de Smalen (sdesmalen-arm) ChangesEnsures the hints are only added once, and ensures that hints inserted by the register allocator take priority over hints to reduce movprfx. Full diff: https://github.com/llvm/llvm-project/pull/167480.diff 2 Files Affected:
diff --git a/llvm/lib/Target/AArch64/AArch64RegisterInfo.cpp b/llvm/lib/Target/AArch64/AArch64RegisterInfo.cpp
index eaf8723094797..a6f48cdc6c19a 100644
--- a/llvm/lib/Target/AArch64/AArch64RegisterInfo.cpp
+++ b/llvm/lib/Target/AArch64/AArch64RegisterInfo.cpp
@@ -1157,6 +1157,9 @@ bool AArch64RegisterInfo::getRegAllocationHints(
// a movprfx.
const TargetRegisterClass *RegRC = MRI.getRegClass(VirtReg);
if (AArch64::ZPRRegClass.hasSubClassEq(RegRC)) {
+ bool ConsiderOnlyHints = TargetRegisterInfo::getRegAllocationHints(
+ VirtReg, Order, Hints, MF, VRM);
+
for (const MachineOperand &DefOp : MRI.def_operands(VirtReg)) {
const MachineInstr &Def = *DefOp.getParent();
if (DefOp.isImplicit() ||
@@ -1168,26 +1171,33 @@ bool AArch64RegisterInfo::getRegAllocationHints(
TII->get(AArch64::getSVEPseudoMap(Def.getOpcode())).TSFlags;
for (MCPhysReg R : Order) {
- auto AddHintIfSuitable = [&](MCPhysReg R, const MachineOperand &MO) {
- // R is a suitable register hint if there exists an operand for the
- // instruction that is not yet allocated a register or if R matches
- // one of the other source operands.
- if (!VRM->hasPhys(MO.getReg()) || VRM->getPhys(MO.getReg()) == R)
+ auto AddHintIfSuitable = [&](MCPhysReg R,
+ const MachineOperand &MO) -> bool {
+ // R is a suitable register hint if:
+ // * R is one of the source operands.
+ // * The register allocator has not suggested any hints and one of the
+ // instruction's source operands does not yet have a register
+ // allocated for it.
+ if (VRM->getPhys(MO.getReg()) == R ||
+ (!VRM->hasPhys(MO.getReg()) && Hints.empty())) {
Hints.push_back(R);
+ return true;
+ }
+ return false;
};
switch (InstFlags & AArch64::DestructiveInstTypeMask) {
default:
break;
case AArch64::DestructiveTernaryCommWithRev:
- AddHintIfSuitable(R, Def.getOperand(2));
- AddHintIfSuitable(R, Def.getOperand(3));
- AddHintIfSuitable(R, Def.getOperand(4));
+ AddHintIfSuitable(R, Def.getOperand(2)) ||
+ AddHintIfSuitable(R, Def.getOperand(3)) ||
+ AddHintIfSuitable(R, Def.getOperand(4));
break;
case AArch64::DestructiveBinaryComm:
case AArch64::DestructiveBinaryCommWithRev:
- AddHintIfSuitable(R, Def.getOperand(2));
- AddHintIfSuitable(R, Def.getOperand(3));
+ AddHintIfSuitable(R, Def.getOperand(2)) ||
+ AddHintIfSuitable(R, Def.getOperand(3));
break;
case AArch64::DestructiveBinary:
case AArch64::DestructiveBinaryImm:
@@ -1198,8 +1208,7 @@ bool AArch64RegisterInfo::getRegAllocationHints(
}
if (Hints.size())
- return TargetRegisterInfo::getRegAllocationHints(VirtReg, Order, Hints,
- MF, VRM);
+ return ConsiderOnlyHints;
}
if (!ST.hasSME() || !ST.isStreaming())
diff --git a/llvm/test/CodeGen/AArch64/regalloc-hint-movprfx.mir b/llvm/test/CodeGen/AArch64/regalloc-hint-movprfx.mir
new file mode 100644
index 0000000000000..c2d8f8e73772d
--- /dev/null
+++ b/llvm/test/CodeGen/AArch64/regalloc-hint-movprfx.mir
@@ -0,0 +1,66 @@
+# RUN: llc -mtriple=aarch64 -mattr=+sve -start-before=greedy -stop-after=virtregrewriter -debug-only=regalloc %s -o /dev/null 2>&1 | FileCheck %s --check-prefix=DBG
+
+# Check that the register allocator gets hints to reuse registers of one of it's operands.
+---
+name: prioritize_movprfx_hints
+tracksRegLiveness: true
+isSSA: false
+noVRegs: false
+body: |
+ bb.0.entry:
+ liveins: $z0, $z1, $z2, $z3, $p0
+
+ ; DBG: Machine code for function prioritize_movprfx_hints
+ ;
+ ; DBG: selectOrSplit ZPR:%4
+ ; DBG-NEXT: hints: $z0 $z1{{$}}
+ ;
+ ; DBG: selectOrSplit ZPR:%5
+ ; DBG-NEXT: hints: $z2 $z3{{$}}
+ ;
+ ; DBG: [%0 -> $z3] ZPR
+ ; DBG: [%1 -> $z2] ZPR
+ ; DBG: [%2 -> $z1] ZPR
+ ; DBG: [%3 -> $z0] ZPR
+ ; DBG: [%4 -> $z0] ZPR
+ ; DBG: [%5 -> $z2] ZPR
+ ; DBG: [%6 -> $z0] ZPR
+ %0:zpr = COPY $z3
+ %1:zpr = COPY $z2
+ %2:zpr = COPY $z1
+ %3:zpr = COPY $z0
+ %4:zpr = SDIV_ZPZZ_D_UNDEF $p0, %3:zpr, %2:zpr
+ %5:zpr = MUL_ZPZZ_D_UNDEF $p0, %1:zpr, %0:zpr
+ %6:zpr = MUL_ZPZZ_D_UNDEF $p0, %5:zpr, %4:zpr
+ $z0 = COPY %6:zpr
+ RET_ReallyLR implicit $z0
+...
+
+# Check that the register allocator prioritises hints that are set by the register
+# allocator itself (i.e. to use z4 for the result register).
+---
+name: prioritize_regalloc_hints
+isSSA: false
+noVRegs: false
+body: |
+ bb.0.entry:
+ %0:zpr = FDUP_ZI_S 0, implicit $vg
+ %1:zpr = FDUP_ZI_S 16, implicit $vg
+ %2:zpr = FDUP_ZI_S 32, implicit $vg
+ %3:ppr_3b = PTRUE_S 31, implicit $vg
+
+ ; DBG: Machine code for function prioritize_regalloc_hints
+ ;
+ ; DBG: selectOrSplit ZPR:%4
+ ; DBG-NEXT: hints: $z4{{$}}
+ ;
+ ; DBG: [%0 -> $z0] ZPR
+ ; DBG: [%1 -> $z1] ZPR
+ ; DBG: [%2 -> $z2] ZPR
+ ; DBG: [%3 -> $p0] PPR_3b
+ ; DBG: [%4 -> $z4] ZPR
+
+ %4:zpr = FMLA_ZPZZZ_S_UNDEF %3, %0, %1, %2
+ $z4 = COPY %4
+ RET_ReallyLR implicit $z4
+...
|
paulwalker-arm
approved these changes
Nov 11, 2025
Collaborator
paulwalker-arm
left a comment
paulwalker-arm
left a comment
There was a problem hiding this comment.
Personally I'd prefer a title that describes the change, but otherwise this looks good to me.
rj-jesus
reviewed
Nov 11, 2025
rj-jesus
reviewed
Nov 11, 2025
Comment on lines
+1193
to
+1195
| AddHintIfSuitable(R, Def.getOperand(2)) || | ||
| AddHintIfSuitable(R, Def.getOperand(3)) || | ||
| AddHintIfSuitable(R, Def.getOperand(4)); |
Contributor
There was a problem hiding this comment.
Is this code trying to ensure we add at most one hint in this function? I think it's reasonable to add one hint for each destructive operand with a physical register allocated, in case their live ranges differ.
In any case, I believe this code may still insert a duplicate hint when the physical register of a destructive operand happens to match a hint previously inserted by the target-independent call.
Collaborator
Author
There was a problem hiding this comment.
This change is trying to avoid for e.g. R = z0 adding [z0, z0, z0] to the list if operands 2, 3 and 4 are equal to z0.
It could indeed still add a duplicate hint if the same hint was added by TargetRegisterInfo::getRegAllocationHints, but I don't believe that's an issue. Do you?
Contributor
There was a problem hiding this comment.
I don't think it's an issue as such, but maybe you could use something like !is_contained(Hints, R) in AddHintIfSuitable to avoid adding duplicates while still being able to add a hint per unique destructive operand? What do you think?
Contributor
There was a problem hiding this comment.
Nevermind, I had misread the code, I see what you mean now. Feel free to ignore my previous suggestion.
sdesmalen-arm
changed the title
rj-jesus
approved these changes
Nov 11, 2025
Contributor
rj-jesus
left a comment
rj-jesus
left a comment
There was a problem hiding this comment.
LGTM as far as I can tell, cheers. :)
There's a CI test failing, but it looks unrelated.
Collaborator
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/11/builds/27869 Here is the relevant piece of the build log for the reference |
Collaborator
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/23/builds/15473 Here is the relevant piece of the build log for the reference |
Member
Seems like the test added here is also failing on green dragon: https://ci.swift.org/job/llvm.org/job/clang-stage2-cmake-RgSan/1180/testReport/ -- could you have a look? |
Collaborator
Author
|
I think that failure should be resolved by 07cd105 ? |
Member
|
Ah, nice, the build with that included is still running (https://ci.swift.org/job/llvm.org/job/clang-stage2-cmake-RgSan/1181/), let's see what falls out, thanks :) |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
6 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.
This is a follow-up from #166926 that ensures the hints are only added once, and ensures that hints inserted by the register allocator take priority over hints to reduce movprfx.