-
Notifications
You must be signed in to change notification settings - Fork 14k
[RISCV] Move RISCVIndirectBranchTracking before Branch Relaxation #139993
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
Conversation
@llvm/pr-subscribers-backend-risc-v Author: Jesse Huang (jaidTw) ChangesThe Full diff: https://github.com/llvm/llvm-project/pull/139993.diff 3 Files Affected:
diff --git a/llvm/lib/Target/RISCV/RISCVTargetMachine.cpp b/llvm/lib/Target/RISCV/RISCVTargetMachine.cpp
index 7fb64be3975d5..28283c9fd5b6d 100644
--- a/llvm/lib/Target/RISCV/RISCVTargetMachine.cpp
+++ b/llvm/lib/Target/RISCV/RISCVTargetMachine.cpp
@@ -570,6 +570,7 @@ void RISCVPassConfig::addPreEmitPass() {
addPass(createMachineCopyPropagationPass(true));
if (TM->getOptLevel() >= CodeGenOptLevel::Default)
addPass(createRISCVLateBranchOptPass());
+ addPass(createRISCVIndirectBranchTrackingPass());
addPass(&BranchRelaxationPassID);
addPass(createRISCVMakeCompressibleOptPass());
}
@@ -581,7 +582,6 @@ void RISCVPassConfig::addPreEmitPass2() {
// ensuring return instruction is detected correctly.
addPass(createRISCVPushPopOptimizationPass());
}
- addPass(createRISCVIndirectBranchTrackingPass());
addPass(createRISCVExpandPseudoPass());
// Schedule the expansion of AMOs at the last possible moment, avoiding the
diff --git a/llvm/test/CodeGen/RISCV/O0-pipeline.ll b/llvm/test/CodeGen/RISCV/O0-pipeline.ll
index 694662eab1681..8714b286374a5 100644
--- a/llvm/test/CodeGen/RISCV/O0-pipeline.ll
+++ b/llvm/test/CodeGen/RISCV/O0-pipeline.ll
@@ -62,6 +62,7 @@
; CHECK-NEXT: Insert fentry calls
; CHECK-NEXT: Insert XRay ops
; CHECK-NEXT: Implement the 'patchable-function' attribute
+; CHECK-NEXT: RISC-V Indirect Branch Tracking
; CHECK-NEXT: Branch relaxation pass
; CHECK-NEXT: RISC-V Make Compressible
; CHECK-NEXT: Contiguously Lay Out Funclets
@@ -73,7 +74,6 @@
; CHECK-NEXT: Lazy Machine Block Frequency Analysis
; CHECK-NEXT: Machine Optimization Remark Emitter
; CHECK-NEXT: Stack Frame Layout Analysis
-; CHECK-NEXT: RISC-V Indirect Branch Tracking
; CHECK-NEXT: RISC-V pseudo instruction expansion pass
; CHECK-NEXT: RISC-V atomic pseudo instruction expansion pass
; CHECK-NEXT: Unpack machine instruction bundles
diff --git a/llvm/test/CodeGen/RISCV/O3-pipeline.ll b/llvm/test/CodeGen/RISCV/O3-pipeline.ll
index 19de864422bc5..c7f70a9d266c2 100644
--- a/llvm/test/CodeGen/RISCV/O3-pipeline.ll
+++ b/llvm/test/CodeGen/RISCV/O3-pipeline.ll
@@ -195,6 +195,7 @@
; CHECK-NEXT: Implement the 'patchable-function' attribute
; CHECK-NEXT: Machine Copy Propagation Pass
; CHECK-NEXT: RISC-V Late Branch Optimisation Pass
+; CHECK-NEXT: RISC-V Indirect Branch Tracking
; CHECK-NEXT: Branch relaxation pass
; CHECK-NEXT: RISC-V Make Compressible
; CHECK-NEXT: Contiguously Lay Out Funclets
@@ -210,7 +211,6 @@
; CHECK-NEXT: Stack Frame Layout Analysis
; CHECK-NEXT: RISC-V Zcmp move merging pass
; CHECK-NEXT: RISC-V Zcmp Push/Pop optimization pass
-; CHECK-NEXT: RISC-V Indirect Branch Tracking
; CHECK-NEXT: RISC-V pseudo instruction expansion pass
; CHECK-NEXT: RISC-V atomic pseudo instruction expansion pass
; CHECK-NEXT: Unpack machine instruction bundles
|
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.
I'm for this in principle, based on your reasoning.
Do you have any kind of testcase you could add to show branch relaxation getting something wrong?
This is an idea of @topperc, theoretically it is possible and I am trying to make a failing test case but with no luck yet. |
✅ With the latest revision this PR passed the C/C++ code formatter. |
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
…vm#139993) The `RISCVIndirectBranchTracking` pass inserts `lpad` instruction and could change the basic block alignment, so this should not happen after the branch relaxation as the adjusted offset is possible to exceed the branch range.
The
RISCVIndirectBranchTracking
pass insertslpad
instruction and could change the basic block alignment, so this should not happen after the branch relaxation as the adjusted offset is possible to exceed the branch range.