-
Notifications
You must be signed in to change notification settings - Fork 15.4k
[RISCV] Exclude LPAD in machine outliner #157220
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) ChangesAfter #139993, the RISCVIndirectBranchTracking pass is also ran before the Machine Outliner pass, this yield a possibility that the outliner could also outlined the LPAD instruction that should be placed at the target of an indirect branch. This patch excludes LPAD instruction from the candidate of machine outliner. Full diff: https://github.com/llvm/llvm-project/pull/157220.diff 2 Files Affected:
diff --git a/llvm/lib/Target/RISCV/RISCVInstrInfo.cpp b/llvm/lib/Target/RISCV/RISCVInstrInfo.cpp
index ee6d6cdb00096..0d15ca27edb00 100644
--- a/llvm/lib/Target/RISCV/RISCVInstrInfo.cpp
+++ b/llvm/lib/Target/RISCV/RISCVInstrInfo.cpp
@@ -3511,6 +3511,11 @@ RISCVInstrInfo::getOutliningTypeImpl(const MachineModuleInfo &MMI,
return outliner::InstrType::Illegal;
}
+ // LPADs should not be outlined too
+ if (MI.getOpcode() == RISCV::AUIPC &&
+ MI.getOperand(0).getReg() == RISCV::X0)
+ return outliner::InstrType::Illegal;
+
return outliner::InstrType::Legal;
}
diff --git a/llvm/test/CodeGen/RISCV/machine-outliner-lpad.ll b/llvm/test/CodeGen/RISCV/machine-outliner-lpad.ll
new file mode 100644
index 0000000000000..9d5187c8c5f7c
--- /dev/null
+++ b/llvm/test/CodeGen/RISCV/machine-outliner-lpad.ll
@@ -0,0 +1,61 @@
+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py UTC_ARGS: --version 5
+; RUN: llc -mtriple riscv64 -mattr=+experimental-zicfilp < %s | FileCheck %s --check-prefixes=CHECK,RV64
+
+define i32 @test1() #0 {
+; CHECK-LABEL: test1:
+; CHECK: # %bb.0: # %entry
+; CHECK-NEXT: lpad 0
+; CHECK-NEXT: li a0, 0
+; CHECK-NEXT: ret
+entry:
+ ret i32 0
+}
+
+define i32 @test2() #0 {
+; CHECK-LABEL: test2:
+; CHECK: # %bb.0: # %entry
+; CHECK-NEXT: lpad 0
+; CHECK-NEXT: li a0, 0
+; CHECK-NEXT: ret
+entry:
+ ret i32 0
+}
+
+define i32 @test3() #0 {
+; CHECK-LABEL: test3:
+; CHECK: # %bb.0: # %entry
+; CHECK-NEXT: lpad 0
+; CHECK-NEXT: li a0, 0
+; CHECK-NEXT: ret
+entry:
+ ret i32 0
+}
+
+define i32 @test4() #0 {
+; CHECK-LABEL: test4:
+; CHECK: # %bb.0: # %entry
+; CHECK-NEXT: lpad 0
+; CHECK-NEXT: li a0, 0
+; CHECK-NEXT: ret
+entry:
+ ret i32 0
+}
+
+define i32 @main() #0 {
+; CHECK-LABEL: main:
+; CHECK: # %bb.0: # %entry
+; CHECK-NEXT: lpad 0
+; CHECK-NEXT: li a0, 0
+; CHECK-NEXT: ret
+entry:
+ ret i32 0
+}
+
+attributes #0 = { minsize }
+
+!llvm.module.flags = !{!0, !1}
+
+!0 = !{i32 8, !"cf-protection-branch", i32 1}
+!1 = !{i32 1, !"cf-branch-label-scheme", !"unlabeled"}
+;; NOTE: These prefixes are unused and the list is autogenerated. Do not add tests below this line:
+; RV64: {{.*}}
|
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
| ; RV32: {{.*}} | ||
| ; RV64: {{.*}} |
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.
Why distinguish these 2?
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.
Oh thanks
I originally used i32 and the codegen was different, I forgot to remove them
| ; RUN: llc -mtriple riscv64 -mattr=+experimental-zicfilp < %s | FileCheck %s --check-prefixes=CHECK | ||
| ; RUN: llc -mtriple riscv32 -mattr=+experimental-zicfilp < %s | FileCheck %s --check-prefixes=CHECK |
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.
Remove --check-prefixes
| CheckNot<CheckImmOperand<2, 0>> | ||
| ]>>>; | ||
|
|
||
| // Returns true if this is LPAD (auipc with rd = x0) |
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 think this comment is a bit redundant (it only says what's done instead of why it's done this way). I suggest to just remove it.
| return outliner::InstrType::Illegal; | ||
| } | ||
|
|
||
| // LPADs should not be outlined too |
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.
This comment is also redundant.
mylai-mtk
left a comment
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
wangpc-pp
left a comment
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.
After #139993, the RISCVIndirectBranchTracking pass is also ran before the Machine Outliner pass, this yield a possibility that the outliner could also outlined the LPAD instruction that should be placed at the target of an indirect branch. This patch excludes LPAD instruction from the candidate of machine outliner.