release/20.x: [RISCV] [MachineOutliner] Analyze all candidates (#127659) #128146
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
|
@wangpc-pp What do you think about merging this PR to the release branch? |
|
@llvm/pr-subscribers-backend-risc-v Author: None (llvmbot) ChangesBackport 6757cf4 Requested by: @svs-quic Full diff: https://github.com/llvm/llvm-project/pull/128146.diff 2 Files Affected:
diff --git a/llvm/lib/Target/RISCV/RISCVInstrInfo.cpp b/llvm/lib/Target/RISCV/RISCVInstrInfo.cpp
index 12a7af0750813..87f1f35835cbe 100644
--- a/llvm/lib/Target/RISCV/RISCVInstrInfo.cpp
+++ b/llvm/lib/Target/RISCV/RISCVInstrInfo.cpp
@@ -3010,30 +3010,25 @@ static bool cannotInsertTailCall(const MachineBasicBlock &MBB) {
return false;
}
-static std::optional<MachineOutlinerConstructionID>
-analyzeCandidate(outliner::Candidate &C) {
+static bool analyzeCandidate(outliner::Candidate &C) {
// If last instruction is return then we can rely on
// the verification already performed in the getOutliningTypeImpl.
if (C.back().isReturn()) {
assert(!cannotInsertTailCall(*C.getMBB()) &&
"The candidate who uses return instruction must be outlined "
"using tail call");
- return MachineOutlinerTailCall;
+ return false;
}
- auto CandidateUsesX5 = [](outliner::Candidate &C) {
- const TargetRegisterInfo *TRI = C.getMF()->getSubtarget().getRegisterInfo();
- if (std::any_of(C.begin(), C.end(), [TRI](const MachineInstr &MI) {
- return isMIModifiesReg(MI, TRI, RISCV::X5);
- }))
- return true;
- return !C.isAvailableAcrossAndOutOfSeq(RISCV::X5, *TRI);
- };
-
- if (!CandidateUsesX5(C))
- return MachineOutlinerDefault;
+ // Filter out candidates where the X5 register (t0) can't be used to setup
+ // the function call.
+ const TargetRegisterInfo *TRI = C.getMF()->getSubtarget().getRegisterInfo();
+ if (std::any_of(C.begin(), C.end(), [TRI](const MachineInstr &MI) {
+ return isMIModifiesReg(MI, TRI, RISCV::X5);
+ }))
+ return true;
- return std::nullopt;
+ return !C.isAvailableAcrossAndOutOfSeq(RISCV::X5, *TRI);
}
std::optional<std::unique_ptr<outliner::OutlinedFunction>>
@@ -3042,35 +3037,32 @@ RISCVInstrInfo::getOutliningCandidateInfo(
std::vector<outliner::Candidate> &RepeatedSequenceLocs,
unsigned MinRepeats) const {
- // Each RepeatedSequenceLoc is identical.
- outliner::Candidate &Candidate = RepeatedSequenceLocs[0];
- auto CandidateInfo = analyzeCandidate(Candidate);
- if (!CandidateInfo)
- RepeatedSequenceLocs.clear();
+ // Analyze each candidate and erase the ones that are not viable.
+ llvm::erase_if(RepeatedSequenceLocs, analyzeCandidate);
// If the sequence doesn't have enough candidates left, then we're done.
if (RepeatedSequenceLocs.size() < MinRepeats)
return std::nullopt;
+ // Each RepeatedSequenceLoc is identical.
+ outliner::Candidate &Candidate = RepeatedSequenceLocs[0];
unsigned InstrSizeCExt =
Candidate.getMF()->getSubtarget<RISCVSubtarget>().hasStdExtCOrZca() ? 2
: 4;
unsigned CallOverhead = 0, FrameOverhead = 0;
- MachineOutlinerConstructionID MOCI = CandidateInfo.value();
- switch (MOCI) {
- case MachineOutlinerDefault:
- // call t0, function = 8 bytes.
- CallOverhead = 8;
- // jr t0 = 4 bytes, 2 bytes if compressed instructions are enabled.
- FrameOverhead = InstrSizeCExt;
- break;
- case MachineOutlinerTailCall:
+ MachineOutlinerConstructionID MOCI = MachineOutlinerDefault;
+ if (Candidate.back().isReturn()) {
+ MOCI = MachineOutlinerTailCall;
// tail call = auipc + jalr in the worst case without linker relaxation.
CallOverhead = 4 + InstrSizeCExt;
// Using tail call we move ret instruction from caller to callee.
FrameOverhead = 0;
- break;
+ } else {
+ // call t0, function = 8 bytes.
+ CallOverhead = 8;
+ // jr t0 = 4 bytes, 2 bytes if compressed instructions are enabled.
+ FrameOverhead = InstrSizeCExt;
}
for (auto &C : RepeatedSequenceLocs)
diff --git a/llvm/test/CodeGen/RISCV/machine-outliner-call-x5-liveout.mir b/llvm/test/CodeGen/RISCV/machine-outliner-call-x5-liveout.mir
new file mode 100644
index 0000000000000..f7bea33e52885
--- /dev/null
+++ b/llvm/test/CodeGen/RISCV/machine-outliner-call-x5-liveout.mir
@@ -0,0 +1,136 @@
+# NOTE: Assertions have been autogenerated by utils/update_mir_test_checks.py UTC_ARGS: --version 5
+# RUN: llc -mtriple=riscv32 -x mir -run-pass=machine-outliner -simplify-mir -verify-machineinstrs < %s \
+# RUN: | FileCheck -check-prefixes=RV32I-MO %s
+# RUN: llc -mtriple=riscv64 -x mir -run-pass=machine-outliner -simplify-mir -verify-machineinstrs < %s \
+# RUN: | FileCheck -check-prefixes=RV64I-MO %s
+
+# MIR has been edited by hand to have x5 as live out in @dont_outline
+
+---
+
+name: outline_0
+tracksRegLiveness: true
+isOutlined: false
+body: |
+ bb.0:
+ liveins: $x10, $x11
+
+ ; RV32I-MO-LABEL: name: outline_0
+ ; RV32I-MO: liveins: $x10, $x11
+ ; RV32I-MO-NEXT: {{ $}}
+ ; RV32I-MO-NEXT: $x5 = PseudoCALLReg target-flags(riscv-call) @OUTLINED_FUNCTION_0, implicit-def $x5, implicit-def $x10, implicit-def $x11, implicit-def $x12, implicit $x10, implicit $x11
+ ; RV32I-MO-NEXT: PseudoRET implicit $x11
+ ;
+ ; RV64I-MO-LABEL: name: outline_0
+ ; RV64I-MO: liveins: $x10, $x11
+ ; RV64I-MO-NEXT: {{ $}}
+ ; RV64I-MO-NEXT: $x5 = PseudoCALLReg target-flags(riscv-call) @OUTLINED_FUNCTION_0, implicit-def $x5, implicit-def $x10, implicit-def $x11, implicit-def $x12, implicit $x10, implicit $x11
+ ; RV64I-MO-NEXT: PseudoRET implicit $x11
+ $x11 = ORI $x11, 1023
+ $x12 = ADDI $x10, 17
+ $x11 = AND $x12, $x11
+ $x10 = SUB $x10, $x11
+ PseudoRET implicit $x11
+
+...
+---
+
+name: dont_outline
+tracksRegLiveness: true
+isOutlined: false
+body: |
+ ; RV32I-MO-LABEL: name: dont_outline
+ ; RV32I-MO: bb.0:
+ ; RV32I-MO-NEXT: liveins: $x10, $x11
+ ; RV32I-MO-NEXT: {{ $}}
+ ; RV32I-MO-NEXT: $x11 = ORI $x11, 1023
+ ; RV32I-MO-NEXT: $x12 = ADDI $x10, 17
+ ; RV32I-MO-NEXT: $x11 = AND $x12, $x11
+ ; RV32I-MO-NEXT: $x10 = SUB $x10, $x11
+ ; RV32I-MO-NEXT: BEQ $x10, $x11, %bb.1
+ ; RV32I-MO-NEXT: {{ $}}
+ ; RV32I-MO-NEXT: bb.1:
+ ; RV32I-MO-NEXT: liveins: $x10, $x5
+ ; RV32I-MO-NEXT: {{ $}}
+ ; RV32I-MO-NEXT: PseudoRET implicit $x10, implicit $x5
+ ;
+ ; RV64I-MO-LABEL: name: dont_outline
+ ; RV64I-MO: bb.0:
+ ; RV64I-MO-NEXT: liveins: $x10, $x11
+ ; RV64I-MO-NEXT: {{ $}}
+ ; RV64I-MO-NEXT: $x11 = ORI $x11, 1023
+ ; RV64I-MO-NEXT: $x12 = ADDI $x10, 17
+ ; RV64I-MO-NEXT: $x11 = AND $x12, $x11
+ ; RV64I-MO-NEXT: $x10 = SUB $x10, $x11
+ ; RV64I-MO-NEXT: BEQ $x10, $x11, %bb.1
+ ; RV64I-MO-NEXT: {{ $}}
+ ; RV64I-MO-NEXT: bb.1:
+ ; RV64I-MO-NEXT: liveins: $x10, $x5
+ ; RV64I-MO-NEXT: {{ $}}
+ ; RV64I-MO-NEXT: PseudoRET implicit $x10, implicit $x5
+ bb.0:
+ liveins: $x10, $x11
+
+ $x11 = ORI $x11, 1023
+ $x12 = ADDI $x10, 17
+ $x11 = AND $x12, $x11
+ $x10 = SUB $x10, $x11
+ BEQ $x10, $x11, %bb.1
+
+ bb.1:
+ liveins: $x10, $x5
+ PseudoRET implicit $x10, implicit $x5
+
+...
+---
+
+name: outline_1
+tracksRegLiveness: true
+isOutlined: false
+body: |
+ bb.0:
+ liveins: $x10, $x11
+
+ ; RV32I-MO-LABEL: name: outline_1
+ ; RV32I-MO: liveins: $x10, $x11
+ ; RV32I-MO-NEXT: {{ $}}
+ ; RV32I-MO-NEXT: $x5 = PseudoCALLReg target-flags(riscv-call) @OUTLINED_FUNCTION_0, implicit-def $x5, implicit-def $x10, implicit-def $x11, implicit-def $x12, implicit $x10, implicit $x11
+ ; RV32I-MO-NEXT: PseudoRET implicit $x10
+ ;
+ ; RV64I-MO-LABEL: name: outline_1
+ ; RV64I-MO: liveins: $x10, $x11
+ ; RV64I-MO-NEXT: {{ $}}
+ ; RV64I-MO-NEXT: $x5 = PseudoCALLReg target-flags(riscv-call) @OUTLINED_FUNCTION_0, implicit-def $x5, implicit-def $x10, implicit-def $x11, implicit-def $x12, implicit $x10, implicit $x11
+ ; RV64I-MO-NEXT: PseudoRET implicit $x10
+ $x11 = ORI $x11, 1023
+ $x12 = ADDI $x10, 17
+ $x11 = AND $x12, $x11
+ $x10 = SUB $x10, $x11
+ PseudoRET implicit $x10
+
+...
+---
+name: outline_2
+tracksRegLiveness: true
+isOutlined: false
+body: |
+ bb.0:
+ liveins: $x10, $x11
+
+ ; RV32I-MO-LABEL: name: outline_2
+ ; RV32I-MO: liveins: $x10, $x11
+ ; RV32I-MO-NEXT: {{ $}}
+ ; RV32I-MO-NEXT: $x5 = PseudoCALLReg target-flags(riscv-call) @OUTLINED_FUNCTION_0, implicit-def $x5, implicit-def $x10, implicit-def $x11, implicit-def $x12, implicit $x10, implicit $x11
+ ; RV32I-MO-NEXT: PseudoRET implicit $x12
+ ;
+ ; RV64I-MO-LABEL: name: outline_2
+ ; RV64I-MO: liveins: $x10, $x11
+ ; RV64I-MO-NEXT: {{ $}}
+ ; RV64I-MO-NEXT: $x5 = PseudoCALLReg target-flags(riscv-call) @OUTLINED_FUNCTION_0, implicit-def $x5, implicit-def $x10, implicit-def $x11, implicit-def $x12, implicit $x10, implicit $x11
+ ; RV64I-MO-NEXT: PseudoRET implicit $x12
+ $x11 = ORI $x11, 1023
+ $x12 = ADDI $x10, 17
+ $x11 = AND $x12, $x11
+ $x10 = SUB $x10, $x11
+ PseudoRET implicit $x12
+...
|
llvm#117700 made a change from analyzing all the candidates to analyzing just the first candidate before deciding to either delete or keep all of them. Even though the candidates all have the same instructions, the basic blocks in which they are present are different and we will need to check each of them before deciding whether to keep or erase them. Particularly, `isAvailableAcrossAndOutOfSeq` checks to see if the register (x5 in this case) is available from the end of the MBB to the beginning of the candidate and not checking this for each candidate led to incorrect candidates being outlined resulting in correctness issues in a few downstream benchmarks. Similarly, deleting all the candidates if the first one is not viable will result in missed outlining opportunities. (cherry picked from commit 6757cf4)
|
@svs-quic (or anyone else). If you would like to add a note about this fix in the release notes (completely optional). Please reply to this comment with a one or two sentence description of the fix. When you are done, please add the release:note label to this PR. |
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.
Backport 6757cf4
Requested by: @svs-quic