-
Notifications
You must be signed in to change notification settings - Fork 13.6k
[RISCV] Avoid using x7/t2 for indirect branches which need landing pad. #68292
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 ChangesWhen Zicfilp enabled, this avoids selecting brind to PseudoBRIND, since PseudoBRIND may uses X7 as rs1 and be identified as a software guarded jump. There is an another PR #66762 to use software guarded jump for jumptable branch. Full diff: https://github.com/llvm/llvm-project/pull/68292.diff 4 Files Affected:
diff --git a/llvm/lib/Target/RISCV/RISCVFeatures.td b/llvm/lib/Target/RISCV/RISCVFeatures.td
index 3d3486b7fa89563..7ba09d65e54d136 100644
--- a/llvm/lib/Target/RISCV/RISCVFeatures.td
+++ b/llvm/lib/Target/RISCV/RISCVFeatures.td
@@ -672,6 +672,8 @@ def FeatureStdExtZicfilp
def HasStdExtZicfilp : Predicate<"Subtarget->hasStdExtZicfilp()">,
AssemblerPredicate<(all_of FeatureStdExtZicfilp),
"'Zicfilp' (Landing pad)">;
+def NoStdExtZicfilp : Predicate<"!Subtarget->hasStdExtZicfilp()">,
+ AssemblerPredicate<(all_of (not FeatureStdExtZicfilp))>;
def FeatureStdExtZicond
: SubtargetFeature<"experimental-zicond", "HasStdExtZicond", "true",
diff --git a/llvm/lib/Target/RISCV/RISCVInstrInfo.td b/llvm/lib/Target/RISCV/RISCVInstrInfo.td
index abbeff78b6e2864..175b0e3a722cf07 100644
--- a/llvm/lib/Target/RISCV/RISCVInstrInfo.td
+++ b/llvm/lib/Target/RISCV/RISCVInstrInfo.td
@@ -1641,9 +1641,25 @@ let isBarrier = 1, isBranch = 1, isIndirectBranch = 1, isTerminator = 1 in
def PseudoBRIND : Pseudo<(outs), (ins GPRJALR:$rs1, simm12:$imm12), []>,
PseudoInstExpansion<(JALR X0, GPR:$rs1, simm12:$imm12)>;
+let Predicates = [HasStdExtZicfilp],
+ isBarrier = 1, isBranch = 1, isIndirectBranch = 1, isTerminator = 1 in {
+def PseudoBRINDNonX7 : Pseudo<(outs), (ins GPRJALRNonX7:$rs1, simm12:$imm12), []>,
+ PseudoInstExpansion<(JALR X0, GPR:$rs1, simm12:$imm12)>;
+}
+
+// For Zicfilp, need to avoid using X7/T2 for indirect branches which need
+// landing pad.
+let Predicates = [HasStdExtZicfilp] in {
+def : Pat<(brind GPRJALRNonX7:$rs1), (PseudoBRINDNonX7 GPRJALRNonX7:$rs1, 0)>;
+def : Pat<(brind (add GPRJALRNonX7:$rs1, simm12:$imm12)),
+ (PseudoBRINDNonX7 GPRJALRNonX7:$rs1, simm12:$imm12)>;
+}
+
+let Predicates = [NoStdExtZicfilp] in {
def : Pat<(brind GPRJALR:$rs1), (PseudoBRIND GPRJALR:$rs1, 0)>;
def : Pat<(brind (add GPRJALR:$rs1, simm12:$imm12)),
(PseudoBRIND GPRJALR:$rs1, simm12:$imm12)>;
+}
// PseudoCALLReg is a generic pseudo instruction for calls which will eventually
// expand to auipc and jalr while encoding, with any given register used as the
diff --git a/llvm/lib/Target/RISCV/RISCVRegisterInfo.td b/llvm/lib/Target/RISCV/RISCVRegisterInfo.td
index ab0d354967b34c7..98763a41b7caf3a 100644
--- a/llvm/lib/Target/RISCV/RISCVRegisterInfo.td
+++ b/llvm/lib/Target/RISCV/RISCVRegisterInfo.td
@@ -148,6 +148,8 @@ def GPRNoX0X2 : GPRRegisterClass<(sub GPR, X0, X2)>;
// by tablegen.
def GPRJALR : GPRRegisterClass<(sub GPR, (sequence "X%u", 0, 5))>;
+def GPRJALRNonX7 : GPRRegisterClass<(sub GPRJALR, X7)>;
+
def GPRC : GPRRegisterClass<(add (sequence "X%u", 10, 15),
(sequence "X%u", 8, 9))>;
diff --git a/llvm/test/CodeGen/RISCV/zicfilp-brind.ll b/llvm/test/CodeGen/RISCV/zicfilp-brind.ll
new file mode 100644
index 000000000000000..2f26d886b4d4438
--- /dev/null
+++ b/llvm/test/CodeGen/RISCV/zicfilp-brind.ll
@@ -0,0 +1,49 @@
+; NOTE: Assertions have been autogenerated by utils/update_mir_test_checks.py UTC_ARGS: --version 3
+; RUN: llc -mtriple=riscv64 -stop-after=finalize-isel < %s | FileCheck %s
+; RUN: llc -mtriple=riscv64 -mattr=+experimental-zicfilp -stop-after=finalize-isel < %s | FileCheck -check-prefixes=ZICFILP %s
+
+@foo.arr = internal unnamed_addr constant [2 x ptr] [ptr blockaddress(@foo, %5), ptr blockaddress(@foo, %8)], align 8
+@x = dso_local global i32 0, align 4
+
+define void @foo(i32 noundef signext %0) {
+ ; CHECK: PseudoBRIND killed [[VAR:%.*]], 0
+ ; ZICFILP: PseudoBRINDNonX7 killed [[VAR:%.*]], 0
+ %2 = sext i32 %0 to i64
+ %3 = getelementptr inbounds [2 x ptr], ptr @foo.arr, i64 0, i64 %2
+ %4 = load ptr, ptr %3, align 8
+ indirectbr ptr %4, [label %5, label %8]
+
+5: ; preds = %1
+ %6 = load i32, ptr @x, align 4
+ %7 = add nsw i32 %6, 2
+ store i32 %7, ptr @x, align 4
+ br label %8
+
+8: ; preds = %5, %1
+ %9 = load i32, ptr @x, align 4
+ %10 = add nsw i32 %9, 1
+ store i32 %10, ptr @x, align 4
+ ret void
+}
+
+define void @bar(i32 noundef signext %0) {
+ ; CHECK: PseudoBRIND killed [[VAR:%.*]], 4
+ ; ZICFILP: PseudoBRINDNonX7 killed [[VAR:%.*]], 4
+ %2 = sext i32 %0 to i64
+ %3 = getelementptr inbounds [2 x ptr], ptr @foo.arr, i64 0, i64 %2
+ %4 = load ptr, ptr %3, align 8
+ %t = getelementptr inbounds i8, ptr @foo.arr, i64 4
+ indirectbr ptr %t, [label %5, label %8]
+
+5: ; preds = %1
+ %6 = load i32, ptr @x, align 4
+ %7 = add nsw i32 %6, 2
+ store i32 %7, ptr @x, align 4
+ br label %8
+
+8: ; preds = %5, %1
+ %9 = load i32, ptr @x, align 4
+ %10 = add nsw i32 %9, 1
+ store i32 %10, ptr @x, align 4
+ ret void
+}
|
I have concerns about this aspect of the spec, as it has implications for the mapping of pseudo instructions to real instructions, and requires changing which register |
435de82
to
621be9e
Compare
When Zicfilp enabled, this avoids selecting brind/call/tail to PseudoBRIND/CALLIndirect/TAILIndirect, since it may use X7 as its scratch register and be identified as a software guarded jump. There is an another PR llvm#66762 to use software guarded jump for jumptable branch.
621be9e
to
140927e
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
When Zicfilp enabled, this avoids selecting brind to PseudoBRIND, since PseudoBRIND may uses X7 as rs1 and be identified as a software guarded jump. There is an another PR #66762 to use software guarded jump for jumptable branch.