[shrinkwrap] PowerPC's FP register should be honored when processing the save point for prologue. #129855

tonykuttai · 2025-03-05T09:19:43Z

When generating code for functions that have __builtin_frame_address calls and noinline attribute, prologue was not emitted correctly leading to an assertion failure in PowerPC. The issue was due to improper insertion of prologue for a function that contain llvm __builtin_frame_address.
Shrink-wrap pass computes the save and restore points of a function. Default points are the entry and exit points of the function. During shrink-wrapping the frame-pointer was not honored like the stack pointer and it was considered as a callee-saved register. This change will treat the FP similar to SP and will insert the prolog on top the instruction containing FP.

github-actions · 2025-03-05T09:20:04Z

Thank you for submitting a Pull Request (PR) to the LLVM Project!

This PR will be automatically labeled and the relevant teams will be notified.

If you wish to, you can add reviewers by using the "Reviewers" section on this page.

If this is not working for you, it is probably because you do not have write permissions for the repository. In which case you can instead tag reviewers by name in a comment by using @ followed by their GitHub username.

If you have received no comments on your PR for a week, you can request a review by "ping"ing the PR by adding a comment “Ping”. The common courtesy "ping" rate is once a week. Please remember that you are asking for valuable time from other developers.

If you have further questions, they may be answered by the LLVM GitHub User Guide.

You can also ask questions in a comment on this PR, on the LLVM Discord or on the forums.

llvmbot · 2025-03-05T09:20:38Z

@llvm/pr-subscribers-backend-powerpc

Author: Tony Varghese (tonykuttai)

Changes

When generating code for functions that have __builtin_frame_address calls and noinline attribute, prologue was not emitted correctly leading to an assertion failure in PowerPC. The issue was due to improper insertion of prologue for a function that contain llvm __builtin_frame_address.
Shrink-wrap pass computes the save and restore points of a function. Default points are the entry and exit points of the function. During shrink-wrapping the frame-pointer was not honored like the stack pointer and it was considered as a callee-saved register. This change will treat the FP similar to SP and will insert the prolog on top the instruction containing FP.

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

4 Files Affected:

(modified) llvm/include/llvm/CodeGen/TargetRegisterInfo.h (+7)
(modified) llvm/lib/CodeGen/ShrinkWrap.cpp (+5-1)
(modified) llvm/lib/Target/PowerPC/PPCRegisterInfo.h (+4)
(added) llvm/test/CodeGen/PowerPC/shrink-wrap-frame-pointer.ll (+103)

diff --git a/llvm/include/llvm/CodeGen/TargetRegisterInfo.h b/llvm/include/llvm/CodeGen/TargetRegisterInfo.h
index 3206cc4518821..b339bcd7fa5fb 100644
--- a/llvm/include/llvm/CodeGen/TargetRegisterInfo.h
+++ b/llvm/include/llvm/CodeGen/TargetRegisterInfo.h
@@ -1243,6 +1243,13 @@ class TargetRegisterInfo : public MCRegisterInfo {
     return false;
   }
 
+  /// Some targets delay assigning the frame until late and use a placeholder
+  /// to represent it earlier. This method can be used to identify the frame
+  /// register placeholder.
+  virtual bool isVirtualFrameRegister(MCRegister Reg) const {
+    return false;
+  }
+
   virtual std::optional<uint8_t> getVRegFlagValue(StringRef Name) const {
     return {};
   }
diff --git a/llvm/lib/CodeGen/ShrinkWrap.cpp b/llvm/lib/CodeGen/ShrinkWrap.cpp
index fa57eb30fac43..c723a1aabd552 100644
--- a/llvm/lib/CodeGen/ShrinkWrap.cpp
+++ b/llvm/lib/CodeGen/ShrinkWrap.cpp
@@ -348,10 +348,14 @@ bool ShrinkWrap::useOrDefCSROrFI(const MachineInstr &MI, RegScavenger *RS,
       // calling convention definitions, so we need to watch for it, too. An LR
       // mentioned implicitly by a return (or "branch to link register")
       // instruction we can ignore, otherwise we may pessimize shrinkwrapping.
+      // PPC's Frame pointer (FP) is also not described as a callee-saved register.
+      // Until the FP is assigned a Physical Register PPC's FP needs to be checked
+      // separately.
       UseOrDefCSR =
           (!MI.isCall() && PhysReg == SP) ||
           RCI.getLastCalleeSavedAlias(PhysReg) ||
-          (!MI.isReturn() && TRI->isNonallocatableRegisterCalleeSave(PhysReg));
+          (!MI.isReturn() && TRI->isNonallocatableRegisterCalleeSave(PhysReg)) ||
+          (!MI.isReturn() && TRI->isVirtualFrameRegister(PhysReg));
     } else if (MO.isRegMask()) {
       // Check if this regmask clobbers any of the CSRs.
       for (unsigned Reg : getCurrentCSRs(RS)) {
diff --git a/llvm/lib/Target/PowerPC/PPCRegisterInfo.h b/llvm/lib/Target/PowerPC/PPCRegisterInfo.h
index 274c7cb68ae0a..103059d0e29ab 100644
--- a/llvm/lib/Target/PowerPC/PPCRegisterInfo.h
+++ b/llvm/lib/Target/PowerPC/PPCRegisterInfo.h
@@ -176,6 +176,10 @@ class PPCRegisterInfo : public PPCGenRegisterInfo {
   bool isNonallocatableRegisterCalleeSave(MCRegister Reg) const override {
     return Reg == PPC::LR || Reg == PPC::LR8;
   }
+
+  bool isVirtualFrameRegister(MCRegister Reg) const override {
+    return Reg == PPC::FP || Reg == PPC::FP8;
+  }
 };
 
 } // end namespace llvm
diff --git a/llvm/test/CodeGen/PowerPC/shrink-wrap-frame-pointer.ll b/llvm/test/CodeGen/PowerPC/shrink-wrap-frame-pointer.ll
new file mode 100644
index 0000000000000..df76cf965b51b
--- /dev/null
+++ b/llvm/test/CodeGen/PowerPC/shrink-wrap-frame-pointer.ll
@@ -0,0 +1,103 @@
+; Test file to check shrink-wrap pass
+
+; RUN: llc -verify-machineinstrs < %s -mtriple=powerpc-ibm-aix-xcoff -mcpu=pwr9 | FileCheck %s --check-prefixes=POWERPC32-AIX
+; RUN: llc -verify-machineinstrs < %s -mtriple=powerpc64-ibm-aix-xcoff -mcpu=pwr9 | FileCheck %s --check-prefixes=POWERPC64-AIX
+
+@.str = private unnamed_addr constant [50 x i8] c"parent_frame_pointer > __builtin_frame_address(0)\00", align 1
+@.str.1 = private unnamed_addr constant [8 x i8] c"bad.cpp\00", align 1
+
+; Function Attrs: mustprogress noinline nounwind
+define void @_Z3fooPv(ptr noundef readnone %parent_frame_pointer) local_unnamed_addr #0 {
+
+; POWERPC32-AIX-LABEL: ._Z3fooPv:
+; POWERPC32-AIX:       # %bb.0:
+; POWERPC32-AIX-NEXT:  mflr 0
+; POWERPC32-AIX-NEXT:  stwu 1, -64(1)
+; POWERPC32-AIX-NEXT:  cmplw 3, 1
+; POWERPC32-AIX-NEXT:  stw 0, 72(1)
+; POWERPC32-AIX-NEXT:  ble- 0, L..BB0_2
+; POWERPC32-AIX-NEXT:  # %bb.1:
+; POWERPC32-AIX-NEXT:  addi 1, 1, 64
+; POWERPC32-AIX-NEXT:  lwz 0, 8(1)
+; POWERPC32-AIX-NEXT:  mtlr 0
+; POWERPC32-AIX-NEXT:  blr
+; POWERPC32-AIX-NEXT: L..BB0_2:
+; POWERPC32-AIX-NEXT:  lwz 4, L..C0(2)
+; POWERPC32-AIX-NEXT:  li 5, 6
+; POWERPC32-AIX-NEXT:  addi 3, 4, 8
+; POWERPC32-AIX-NEXT:  bl .__assert[PR]
+; POWERPC32-AIX-NEXT:  nop
+
+; POWERPC64-AIX-LABEL: ._Z3fooPv:
+; POWERPC64-AIX:       # %bb.0:
+; POWERPC64-AIX-NEXT:  mflr 0
+; POWERPC64-AIX-NEXT:  stdu 1, -112(1)
+; POWERPC64-AIX-NEXT:  cmpld 3, 1
+; POWERPC64-AIX-NEXT:  std 0, 128(1)
+; POWERPC64-AIX-NEXT:  ble- 0, L..BB0_2
+; POWERPC64-AIX-NEXT:  # %bb.1:
+; POWERPC64-AIX-NEXT:  addi 1, 1, 112
+; POWERPC64-AIX-NEXT:  ld 0, 16(1)
+; POWERPC64-AIX-NEXT:  mtlr 0
+; POWERPC64-AIX-NEXT:  blr
+; POWERPC64-AIX-NEXT: L..BB0_2:
+; POWERPC64-AIX-NEXT:  ld 4, L..C0(2)
+; POWERPC64-AIX-NEXT:  li 5, 6
+; POWERPC64-AIX-NEXT:  addi 3, 4, 8
+; POWERPC64-AIX-NEXT:  bl .__assert[PR]
+; POWERPC64-AIX-NEXT:  nop
+
+entry:
+  %0 = tail call ptr @llvm.frameaddress.p0(i32 0)
+  %cmp = icmp ugt ptr %parent_frame_pointer, %0
+  br i1 %cmp, label %cond.end, label %cond.false
+
+cond.false:                                       ; preds = %entry
+  tail call void @__assert(ptr noundef nonnull @.str, ptr noundef nonnull @.str.1, i32 noundef 6) #4
+  unreachable
+
+cond.end:                                         ; preds = %entry
+  ret void
+}
+
+; Function Attrs: mustprogress nocallback nofree nosync nounwind willreturn memory(none)
+declare ptr @llvm.frameaddress.p0(i32 immarg) #1
+
+; Function Attrs: noreturn nounwind
+declare void @__assert(ptr noundef, ptr noundef, i32 noundef) local_unnamed_addr #2
+
+; Function Attrs: mustprogress norecurse nounwind
+define noundef i32 @main() local_unnamed_addr #3 {
+; POWERPC32-AIX-LABEL: .main:
+; POWERPC32-AIX:       # %bb.0:
+; POWERPC32-AIX-NEXT:  mflr 0
+; POWERPC32-AIX-NEXT:  stwu 1, -64(1)
+; POWERPC32-AIX-NEXT:  mr 3, 1
+; POWERPC32-AIX-NEXT:  stw 0, 72(1)
+; POWERPC32-AIX-NEXT:  bl ._Z3fooPv
+; POWERPC32-AIX-NEXT:  nop
+; POWERPC32-AIX-NEXT:  li 3, 0
+; POWERPC32-AIX-NEXT:  addi 1, 1, 64
+; POWERPC32-AIX-NEXT:  lwz 0, 8(1)
+; POWERPC32-AIX-NEXT:  mtlr 0
+; POWERPC32-AIX-NEXT:  blr
+
+; POWERPC64-AIX-LABEL: .main:
+; POWERPC64-AIX:       # %bb.0:
+; POWERPC64-AIX-NEXT:  mflr 0
+; POWERPC64-AIX-NEXT:  stdu 1, -112(1)
+; POWERPC64-AIX-NEXT:  mr 3, 1
+; POWERPC64-AIX-NEXT:  std 0, 128(1)
+; POWERPC64-AIX-NEXT:  bl ._Z3fooPv
+; POWERPC64-AIX-NEXT:  nop
+; POWERPC64-AIX-NEXT:  li 3, 0
+; POWERPC64-AIX-NEXT:  addi 1, 1, 112
+; POWERPC64-AIX-NEXT:  ld 0, 16(1)
+; POWERPC64-AIX-NEXT:  mtlr 0
+; POWERPC64-AIX-NEXT:  blr
+
+entry:
+  %0 = tail call ptr @llvm.frameaddress.p0(i32 0)
+  tail call void @_Z3fooPv(ptr noundef %0)
+  ret i32 0
+}

tonykuttai · 2025-03-05T09:24:16Z

@w2yehia @redstar @lei137

…r prolog by shrinkwrap pass

tonykuttai · 2025-03-06T05:43:11Z

Code formatter failed. Ran git-clang-format on the changes made.

llvm/lib/CodeGen/ShrinkWrap.cpp

llvm/test/CodeGen/PowerPC/shrink-wrap-frame-pointer.ll

…ement is check in shrinkwrap.cpp

github-actions · 2025-03-13T22:02:06Z

✅ With the latest revision this PR passed the C/C++ code formatter.

tonykuttai · 2025-03-20T04:34:34Z

NFC patch to check this test case with the existing codegen [NFC][shrinkwrap] Add test point to capture the prologue and epilogue insertion by shrinkwrap pass for powerpc. merged.

ping @lei137 @RolandF77 @redstar

RolandF77

LGTM

lei137

LGTM
Thx

github-actions · 2025-03-21T16:56:00Z

@tonykuttai Congratulations on having your first Pull Request (PR) merged into the LLVM Project!

Your changes will be combined with recent changes from other authors, then tested by our build bots. If there is a problem with a build, you may receive a report in an email or a comment on this PR.

Please check whether problems have been caused by your change specifically, as the builds can include changes from many authors. It is not uncommon for your change to be included in a build that fails due to someone else's changes, or infrastructure issues.

How to do this, and the rest of the post-merge process, is covered in detail here.

If your change does cause a problem, it may be reverted, or you can revert it yourself. This is a normal part of LLVM development. You can fix your changes and open a new PR to merge them again.

If you don't get any reports, no action is required from you. Your changes are working as expected, well done!

llvmbot added the backend:PowerPC label Mar 5, 2025

tonykuttai force-pushed the tvarghese/framebug branch from 91b493e to 73e69b9 Compare March 5, 2025 14:00

w2yehia requested review from redstar, RolandF77 and lei137 March 5, 2025 14:06

PPC's FP register should be honored when processing the save point fo…

f57b233

…r prolog by shrinkwrap pass

tonykuttai force-pushed the tvarghese/framebug branch from 579bfaf to f57b233 Compare March 6, 2025 04:28

RolandF77 reviewed Mar 7, 2025

View reviewed changes

llvm/lib/CodeGen/ShrinkWrap.cpp Outdated Show resolved Hide resolved

lei137 reviewed Mar 7, 2025

View reviewed changes

llvm/test/CodeGen/PowerPC/shrink-wrap-frame-pointer.ll Outdated Show resolved Hide resolved

llvm/test/CodeGen/PowerPC/shrink-wrap-frame-pointer.ll Outdated Show resolved Hide resolved

Modified the shrink-wrap-frame-pointer.ll and removed the return stat…

bd0fb18

…ement is check in shrinkwrap.cpp

tonykuttai requested review from lei137 and RolandF77 March 13, 2025 20:35

tonykuttai mentioned this pull request Mar 13, 2025

[NFC][shrinkwrap] Add test point to capture the prologue and epilogue insertion by shrinkwrap pass for powerpc. #131192

Merged

clang-format modification

e0dba4a

tonykuttai added 4 commits March 13, 2025 23:18

clang format change

6e4ea18

modified mflr instruction to use regex

7e44c4f

Merge branch 'main' into tvarghese/framebug

c9b47c5

Merge branch 'main' into tvarghese/framebug

62f40f8

Merge branch 'main' into tvarghese/framebug

6ccb35f

RolandF77 approved these changes Mar 20, 2025

View reviewed changes

lei137 approved these changes Mar 21, 2025

View reviewed changes

w2yehia merged commit ff9c5c3 into llvm:main Mar 21, 2025
11 checks passed

tonykuttai mentioned this pull request Jun 6, 2025

Request Commit Access For tonykuttai #143064

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[shrinkwrap] PowerPC's FP register should be honored when processing the save point for prologue. #129855

[shrinkwrap] PowerPC's FP register should be honored when processing the save point for prologue. #129855

Uh oh!

tonykuttai commented Mar 5, 2025

Uh oh!

github-actions bot commented Mar 5, 2025

Uh oh!

llvmbot commented Mar 5, 2025

Uh oh!

tonykuttai commented Mar 5, 2025 •

edited

Loading

Uh oh!

tonykuttai commented Mar 6, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

github-actions bot commented Mar 13, 2025 •

edited

Loading

Uh oh!

tonykuttai commented Mar 20, 2025 •

edited

Loading

Uh oh!

RolandF77 left a comment

Uh oh!

lei137 left a comment

Uh oh!

Uh oh!

github-actions bot commented Mar 21, 2025

Uh oh!

Uh oh!

[shrinkwrap] PowerPC's FP register should be honored when processing the save point for prologue. #129855

[shrinkwrap] PowerPC's FP register should be honored when processing the save point for prologue. #129855

Uh oh!

Conversation

tonykuttai commented Mar 5, 2025

Uh oh!

github-actions bot commented Mar 5, 2025

Uh oh!

llvmbot commented Mar 5, 2025

Uh oh!

tonykuttai commented Mar 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

tonykuttai commented Mar 6, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

github-actions bot commented Mar 13, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

tonykuttai commented Mar 20, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

RolandF77 left a comment

Choose a reason for hiding this comment

Uh oh!

lei137 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

github-actions bot commented Mar 21, 2025

Uh oh!

Uh oh!

tonykuttai commented Mar 5, 2025 •

edited

Loading

github-actions bot commented Mar 13, 2025 •

edited

Loading

tonykuttai commented Mar 20, 2025 •

edited

Loading