-
Notifications
You must be signed in to change notification settings - Fork 15k
Greedy: Take hints from copy to physical subreg #160467
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.
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
Greedy: Take hints from copy to physical subreg #160467
Conversation
This stack of pull requests is managed by Graphite. Learn more about stacking. |
|
@llvm/pr-subscribers-backend-x86 Author: Matt Arsenault (arsenm) ChangesPreviously this took hints from subregister extract of physreg, This now also handles the rarer case: Also make an accidental bug here before explicit; this was Full diff: https://github.com/llvm/llvm-project/pull/160467.diff 2 Files Affected:
diff --git a/llvm/lib/CodeGen/RegAllocGreedy.cpp b/llvm/lib/CodeGen/RegAllocGreedy.cpp
index 6e0585b2e9e55..0df8713dd892b 100644
--- a/llvm/lib/CodeGen/RegAllocGreedy.cpp
+++ b/llvm/lib/CodeGen/RegAllocGreedy.cpp
@@ -2439,25 +2439,28 @@ void RAGreedy::collectHintInfo(Register Reg, HintsInfo &Out) {
unsigned SubReg = Opnd.getSubReg();
// Get the current assignment.
- MCRegister OtherPhysReg =
- OtherReg.isPhysical() ? OtherReg.asMCReg() : VRM->getPhys(OtherReg);
- if (OtherSubReg) {
- if (OtherReg.isPhysical()) {
- MCRegister Tuple =
- TRI->getMatchingSuperReg(OtherPhysReg, OtherSubReg, RC);
- if (!Tuple)
- continue;
- OtherPhysReg = Tuple;
- } else {
- // TODO: There should be a hinting mechanism for subregisters
- if (SubReg != OtherSubReg)
- continue;
- }
+ MCRegister OtherPhysReg;
+ if (OtherReg.isPhysical()) {
+ if (OtherSubReg)
+ OtherPhysReg = TRI->getMatchingSuperReg(OtherReg, OtherSubReg, RC);
+ else if (SubReg)
+ OtherPhysReg = TRI->getMatchingSuperReg(OtherReg, SubReg, RC);
+ else
+ OtherPhysReg = OtherReg;
+ } else {
+ OtherPhysReg = VRM->getPhys(OtherReg);
+ // TODO: Should find matching superregister, but applying this in the
+ // non-hint case currently causes regressions
+
+ if (SubReg && OtherSubReg && SubReg != OtherSubReg)
+ continue;
}
// Push the collected information.
- Out.push_back(HintInfo(MBFI->getBlockFreq(Instr.getParent()), OtherReg,
- OtherPhysReg));
+ if (OtherPhysReg) {
+ Out.push_back(HintInfo(MBFI->getBlockFreq(Instr.getParent()), OtherReg,
+ OtherPhysReg));
+ }
}
}
diff --git a/llvm/test/CodeGen/X86/shift-i128.ll b/llvm/test/CodeGen/X86/shift-i128.ll
index 7462c77482827..049ee47af9681 100644
--- a/llvm/test/CodeGen/X86/shift-i128.ll
+++ b/llvm/test/CodeGen/X86/shift-i128.ll
@@ -613,8 +613,7 @@ define void @test_shl_v2i128(<2 x i128> %x, <2 x i128> %a, ptr nocapture %r) nou
; i686-NEXT: shldl %cl, %esi, %ebx
; i686-NEXT: movl {{[-0-9]+}}(%e{{[sb]}}p), %edi # 4-byte Reload
; i686-NEXT: movl %edi, %esi
-; i686-NEXT: movl {{[-0-9]+}}(%e{{[sb]}}p), %eax # 4-byte Reload
-; i686-NEXT: movl %eax, %ecx
+; i686-NEXT: movl {{[-0-9]+}}(%e{{[sb]}}p), %ecx # 4-byte Reload
; i686-NEXT: shll %cl, %esi
; i686-NEXT: shldl %cl, %edi, {{[-0-9]+}}(%e{{[sb]}}p) # 4-byte Folded Spill
; i686-NEXT: negl %edx
|
|
@llvm/pr-subscribers-llvm-regalloc Author: Matt Arsenault (arsenm) ChangesPreviously this took hints from subregister extract of physreg, This now also handles the rarer case: Also make an accidental bug here before explicit; this was Full diff: https://github.com/llvm/llvm-project/pull/160467.diff 2 Files Affected:
diff --git a/llvm/lib/CodeGen/RegAllocGreedy.cpp b/llvm/lib/CodeGen/RegAllocGreedy.cpp
index 6e0585b2e9e55..0df8713dd892b 100644
--- a/llvm/lib/CodeGen/RegAllocGreedy.cpp
+++ b/llvm/lib/CodeGen/RegAllocGreedy.cpp
@@ -2439,25 +2439,28 @@ void RAGreedy::collectHintInfo(Register Reg, HintsInfo &Out) {
unsigned SubReg = Opnd.getSubReg();
// Get the current assignment.
- MCRegister OtherPhysReg =
- OtherReg.isPhysical() ? OtherReg.asMCReg() : VRM->getPhys(OtherReg);
- if (OtherSubReg) {
- if (OtherReg.isPhysical()) {
- MCRegister Tuple =
- TRI->getMatchingSuperReg(OtherPhysReg, OtherSubReg, RC);
- if (!Tuple)
- continue;
- OtherPhysReg = Tuple;
- } else {
- // TODO: There should be a hinting mechanism for subregisters
- if (SubReg != OtherSubReg)
- continue;
- }
+ MCRegister OtherPhysReg;
+ if (OtherReg.isPhysical()) {
+ if (OtherSubReg)
+ OtherPhysReg = TRI->getMatchingSuperReg(OtherReg, OtherSubReg, RC);
+ else if (SubReg)
+ OtherPhysReg = TRI->getMatchingSuperReg(OtherReg, SubReg, RC);
+ else
+ OtherPhysReg = OtherReg;
+ } else {
+ OtherPhysReg = VRM->getPhys(OtherReg);
+ // TODO: Should find matching superregister, but applying this in the
+ // non-hint case currently causes regressions
+
+ if (SubReg && OtherSubReg && SubReg != OtherSubReg)
+ continue;
}
// Push the collected information.
- Out.push_back(HintInfo(MBFI->getBlockFreq(Instr.getParent()), OtherReg,
- OtherPhysReg));
+ if (OtherPhysReg) {
+ Out.push_back(HintInfo(MBFI->getBlockFreq(Instr.getParent()), OtherReg,
+ OtherPhysReg));
+ }
}
}
diff --git a/llvm/test/CodeGen/X86/shift-i128.ll b/llvm/test/CodeGen/X86/shift-i128.ll
index 7462c77482827..049ee47af9681 100644
--- a/llvm/test/CodeGen/X86/shift-i128.ll
+++ b/llvm/test/CodeGen/X86/shift-i128.ll
@@ -613,8 +613,7 @@ define void @test_shl_v2i128(<2 x i128> %x, <2 x i128> %a, ptr nocapture %r) nou
; i686-NEXT: shldl %cl, %esi, %ebx
; i686-NEXT: movl {{[-0-9]+}}(%e{{[sb]}}p), %edi # 4-byte Reload
; i686-NEXT: movl %edi, %esi
-; i686-NEXT: movl {{[-0-9]+}}(%e{{[sb]}}p), %eax # 4-byte Reload
-; i686-NEXT: movl %eax, %ecx
+; i686-NEXT: movl {{[-0-9]+}}(%e{{[sb]}}p), %ecx # 4-byte Reload
; i686-NEXT: shll %cl, %esi
; i686-NEXT: shldl %cl, %edi, {{[-0-9]+}}(%e{{[sb]}}p) # 4-byte Folded Spill
; i686-NEXT: negl %edx
|
|
@llvm/pr-subscribers-backend-systemz Author: Matt Arsenault (arsenm) ChangesPreviously this took hints from subregister extract of physreg, This now also handles the rarer case: Also make an accidental bug here before explicit; this was Full diff: https://github.com/llvm/llvm-project/pull/160467.diff 2 Files Affected:
diff --git a/llvm/lib/CodeGen/RegAllocGreedy.cpp b/llvm/lib/CodeGen/RegAllocGreedy.cpp
index 6e0585b2e9e55..0df8713dd892b 100644
--- a/llvm/lib/CodeGen/RegAllocGreedy.cpp
+++ b/llvm/lib/CodeGen/RegAllocGreedy.cpp
@@ -2439,25 +2439,28 @@ void RAGreedy::collectHintInfo(Register Reg, HintsInfo &Out) {
unsigned SubReg = Opnd.getSubReg();
// Get the current assignment.
- MCRegister OtherPhysReg =
- OtherReg.isPhysical() ? OtherReg.asMCReg() : VRM->getPhys(OtherReg);
- if (OtherSubReg) {
- if (OtherReg.isPhysical()) {
- MCRegister Tuple =
- TRI->getMatchingSuperReg(OtherPhysReg, OtherSubReg, RC);
- if (!Tuple)
- continue;
- OtherPhysReg = Tuple;
- } else {
- // TODO: There should be a hinting mechanism for subregisters
- if (SubReg != OtherSubReg)
- continue;
- }
+ MCRegister OtherPhysReg;
+ if (OtherReg.isPhysical()) {
+ if (OtherSubReg)
+ OtherPhysReg = TRI->getMatchingSuperReg(OtherReg, OtherSubReg, RC);
+ else if (SubReg)
+ OtherPhysReg = TRI->getMatchingSuperReg(OtherReg, SubReg, RC);
+ else
+ OtherPhysReg = OtherReg;
+ } else {
+ OtherPhysReg = VRM->getPhys(OtherReg);
+ // TODO: Should find matching superregister, but applying this in the
+ // non-hint case currently causes regressions
+
+ if (SubReg && OtherSubReg && SubReg != OtherSubReg)
+ continue;
}
// Push the collected information.
- Out.push_back(HintInfo(MBFI->getBlockFreq(Instr.getParent()), OtherReg,
- OtherPhysReg));
+ if (OtherPhysReg) {
+ Out.push_back(HintInfo(MBFI->getBlockFreq(Instr.getParent()), OtherReg,
+ OtherPhysReg));
+ }
}
}
diff --git a/llvm/test/CodeGen/X86/shift-i128.ll b/llvm/test/CodeGen/X86/shift-i128.ll
index 7462c77482827..049ee47af9681 100644
--- a/llvm/test/CodeGen/X86/shift-i128.ll
+++ b/llvm/test/CodeGen/X86/shift-i128.ll
@@ -613,8 +613,7 @@ define void @test_shl_v2i128(<2 x i128> %x, <2 x i128> %a, ptr nocapture %r) nou
; i686-NEXT: shldl %cl, %esi, %ebx
; i686-NEXT: movl {{[-0-9]+}}(%e{{[sb]}}p), %edi # 4-byte Reload
; i686-NEXT: movl %edi, %esi
-; i686-NEXT: movl {{[-0-9]+}}(%e{{[sb]}}p), %eax # 4-byte Reload
-; i686-NEXT: movl %eax, %ecx
+; i686-NEXT: movl {{[-0-9]+}}(%e{{[sb]}}p), %ecx # 4-byte Reload
; i686-NEXT: shll %cl, %esi
; i686-NEXT: shldl %cl, %edi, {{[-0-9]+}}(%e{{[sb]}}p) # 4-byte Folded Spill
; i686-NEXT: negl %edx
|
6f2be9a to
0581982
Compare
f98b2e0 to
205e4cd
Compare
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
205e4cd to
5f93f5c
Compare
0581982 to
ab52103
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
For subregister copies, do a subregister live check instead of checking the main range. Doesn't do much yet, the split analysis still does not track live ranges.
Previously this took hints from subregister extract of physreg, like %vreg.sub = COPY $physreg This now also handles the rarer case: $physreg_sub = COPY %vreg Also make an accidental bug here before explicit; this was only using the superregister as a hint if it was already in the copy, and not if using the existing assignment. There are a handful of regressions in that case, so leave that extension for a future change.
ab52103 to
9604dab
Compare
5f93f5c to
661b22d
Compare
Previously this took hints from subregister extract of physreg, like %vreg.sub = COPY $physreg This now also handles the rarer case: $physreg_sub = COPY %vreg Also make an accidental bug here before explicit; this was only using the superregister as a hint if it was already in the copy, and not if using the existing assignment. There are a handful of regressions in that case, so leave that extension for a future change.

Previously this took hints from subregister extract of physreg,
like %vreg.sub = COPY $physreg
This now also handles the rarer case:
$physreg_sub = COPY %vreg
Also make an accidental bug here before explicit; this was
only using the superregister as a hint if it was already
in the copy, and not if using the existing assignment. There are
a handful of regressions in that case, so leave that extension
for a future change.