[MachineOutliner] Avoid ranges that cross bundle boundary #148977
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
|
@llvm/pr-subscribers-backend-aarch64 Author: Ellis Hoag (ellishg) ChangesWe found some code that was hitting this assert because llvm-project/llvm/include/llvm/CodeGen/MachineInstrBundleIterator.h Lines 133 to 135 in ae3bba4 Avoid creating those ranges and add a test that hit the assert. Full diff: https://github.com/llvm/llvm-project/pull/148977.diff 2 Files Affected:
diff --git a/llvm/lib/Target/AArch64/AArch64InstrInfo.cpp b/llvm/lib/Target/AArch64/AArch64InstrInfo.cpp
index 5420545cc3cec..a23ed60d0325b 100644
--- a/llvm/lib/Target/AArch64/AArch64InstrInfo.cpp
+++ b/llvm/lib/Target/AArch64/AArch64InstrInfo.cpp
@@ -9585,10 +9585,13 @@ AArch64InstrInfo::getOutlinableRanges(MachineBasicBlock &MBB,
};
auto SaveRangeIfNonEmpty = [&RangeLen, &Ranges, &RangeBegin, &RangeEnd]() {
// At least one unsafe register is not dead. We do not want to outline at
- // this point. If it is long enough to outline from, save the range
- // [RangeBegin, RangeEnd).
- if (RangeLen > 1)
- Ranges.push_back(std::make_pair(RangeBegin, RangeEnd));
+ // this point. If it is long enough to outline from and does not cross a
+ // bundle boundary, save the range [RangeBegin, RangeEnd).
+ if (RangeLen <= 1)
+ return;
+ if (RangeBegin->isBundledWithPred())
+ return;
+ Ranges.push_back(std::make_pair(RangeBegin, RangeEnd));
};
// Find the first point where all unsafe registers are dead.
// FIND: <safe instr> <-- end of first potential range
diff --git a/llvm/test/CodeGen/AArch64/machine-outliner-safe-range-in-middle.mir b/llvm/test/CodeGen/AArch64/machine-outliner-safe-range-in-middle.mir
index 23811425101fd..ed4a8fe76c221 100644
--- a/llvm/test/CodeGen/AArch64/machine-outliner-safe-range-in-middle.mir
+++ b/llvm/test/CodeGen/AArch64/machine-outliner-safe-range-in-middle.mir
@@ -40,3 +40,52 @@ body: |
$x9 = ADDXri $x9, 16, 0
$x16 = ADDXri killed $x16, 16, 0
RET undef $x9
+...
+---
+name: unsafe_range_bundle
+tracksRegLiveness: true
+machineFunctionInfo:
+ hasRedZone: false
+body: |
+ bb.0:
+ liveins: $x0
+ ; Begin safe range of 3 instructions
+ ; CHECK-LABEL: name: unsafe_range_bundle
+ ; CHECK: liveins: $x0
+ ; CHECK-NEXT: {{ $}}
+ ; CHECK-NEXT: BL @OUTLINED_FUNCTION_0, implicit-def $lr, implicit $sp, implicit-def $lr, implicit-def $x0, implicit-def $x1, implicit-def $x2, implicit-def $x3, implicit-def $x16, implicit $x0, implicit $sp
+ ; CHECK-NEXT: $x9 = ADDXri $x16, 16, 0
+ ; CHECK-NEXT: BUNDLE {
+ ; CHECK-NEXT: $x16 = ADDXri killed $x16, 16, 0
+ ; CHECK-NEXT: $x0 = ADDXri $x0, 0, 0
+ ; CHECK-NEXT: $x1 = ADDXri $x0, 1, 0
+ ; CHECK-NEXT: }
+ ; CHECK-NEXT: $x2 = ADDXri $x0, 2, 0
+ ; CHECK-NEXT: $x3 = ADDXri $x0, 3, 0
+ ; CHECK-NEXT: $x16 = ADDXri $x0, 16, 0
+ ; CHECK-NEXT: $x9 = ADDXri $x9, 16, 0
+ ; CHECK-NEXT: $x16 = ADDXri killed $x16, 16, 0
+ ; CHECK-NEXT: RET undef $x9
+ $x0 = ADDXri $x0, 0, 0
+ $x1 = ADDXri $x0, 1, 0
+ $x2 = ADDXri $x0, 2, 0
+ $x3 = ADDXri $x0, 3, 0
+
+ ; End safe range
+ $x16 = ADDXri $x0, 16, 0
+ $x9 = ADDXri $x16, 16, 0
+ ; Bundle crosses a safe range
+ BUNDLE {
+ $x16 = ADDXri killed $x16, 16, 0
+
+ $x0 = ADDXri $x0, 0, 0
+ $x1 = ADDXri $x0, 1, 0
+ }
+ $x2 = ADDXri $x0, 2, 0
+ $x3 = ADDXri $x0, 3, 0
+ ; End safe range
+ $x16 = ADDXri $x0, 16, 0
+ $x9 = ADDXri $x9, 16, 0
+ $x16 = ADDXri killed $x16, 16, 0
+ RET undef $x9
+
|
kyulee-com
approved these changes
Jul 15, 2025
statham-arm
reviewed
Jul 16, 2025
| body: | | ||
| bb.0: | ||
| liveins: $x0 | ||
| ; Begin safe range of 3 instructions |
There was a problem hiding this comment.
I'm confused by the "safe range" comments. This "Begin safe range of 3 instructions" comment seems to go with an "End safe range" comment on line 74 (in this version of the patch) four instructions later, not three. Then there's another "End safe range" further down (line 86) without any "Begin" comment. Finally, it's confusing that the CHECK comments are after the "Begin safe range"; putting them the other way round would surely have been clearer?
It looks to me as if the real point of this test is that the four ADDXri to x0,x1,x2,x3 are duplicated in this function (lines 69-72 and 81-85), but in the second copy two of them are inside a bundle, and therefore they mustn't be outlined. If I've understood that correctly, perhaps describing them as something like an "outlining candidate" instead of a safe range might be less confusing – especially since the whole point of this example is that this range of instructions isn't safe to outline :-)
There was a problem hiding this comment.
These comments were copied from the test unsafe_range_in_middle above, and I didn't check if they were correct or not. I simply found some tests that had separate ranges and inserted a bundle that triggered the crash.
There was a problem hiding this comment.
I ended up updating the comments to reflect the actual ranges and simplified my test a bit. I also found that we need some extra checks, too.
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/54/builds/10937 Here is the relevant piece of the build log for the reference |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/123/builds/23515 Here is the relevant piece of the build log for the reference |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
We found some code that was hitting this assert because
getOutlinableRanges()was trying to create a range that crossed a bundle boundary.llvm-project/llvm/include/llvm/CodeGen/MachineInstrBundleIterator.h
Lines 133 to 135 in ae3bba4
Avoid creating those ranges and add a test that hit the assert.