Skip to content
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

[RISCV] Avoid using x7/t2 for indirect branches which need landing pad. #68292

Merged
merged 2 commits into from
May 2, 2024

Conversation

yetingk
Copy link
Contributor

@yetingk yetingk commented Oct 5, 2023

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.

@llvmbot
Copy link
Collaborator

llvmbot commented Oct 5, 2023

@llvm/pr-subscribers-backend-risc-v

Changes

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.


Full diff: https://github.com/llvm/llvm-project/pull/68292.diff

4 Files Affected:

  • (modified) llvm/lib/Target/RISCV/RISCVFeatures.td (+2)
  • (modified) llvm/lib/Target/RISCV/RISCVInstrInfo.td (+16)
  • (modified) llvm/lib/Target/RISCV/RISCVRegisterInfo.td (+2)
  • (added) llvm/test/CodeGen/RISCV/zicfilp-brind.ll (+49)
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
+}

@jrtc27
Copy link
Collaborator

jrtc27 commented Oct 5, 2023

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 tail clobbers, something which is, today, very clearly specified in the RISC-V Assembly Programmer's Manual. See riscv/riscv-cfi#125 (comment).

@yetingk yetingk force-pushed the zicfilp-indirect-branch branch 2 times, most recently from 435de82 to 621be9e Compare April 24, 2024 08:07
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.
Copy link
Collaborator

@topperc topperc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@yetingk yetingk merged commit 0644909 into llvm:main May 2, 2024
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants