-
Notifications
You must be signed in to change notification settings - Fork 13.4k
[PowerPC] provide CFI for ELF32 to unwind cr2, cr3, cr4 #83098
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-powerpc Author: George Koehler (kernigh) ChangesDelete the code that skips the CFI for the condition register on ELF32. The code checked !MustSaveCR, which happened only when Subtarget.is32BitELFABI(), where spillCalleeSavedRegisters is spilling cr in a different way. The spill was missing CFI. After deleting this code, a spill of cr2 to cr4 gets CFI in the same way as a spill of r14 to r31. Fixes #83094 Full diff: https://github.com/llvm/llvm-project/pull/83098.diff 2 Files Affected:
diff --git a/llvm/lib/Target/PowerPC/PPCFrameLowering.cpp b/llvm/lib/Target/PowerPC/PPCFrameLowering.cpp
index 6792842f8550c1..424501c35c043c 100644
--- a/llvm/lib/Target/PowerPC/PPCFrameLowering.cpp
+++ b/llvm/lib/Target/PowerPC/PPCFrameLowering.cpp
@@ -1191,12 +1191,6 @@ void PPCFrameLowering::emitPrologue(MachineFunction &MF,
if ((Reg == PPC::X2 || Reg == PPC::R2) && MustSaveTOC)
continue;
- // For SVR4, don't emit a move for the CR spill slot if we haven't
- // spilled CRs.
- if (isSVR4ABI && (PPC::CR2 <= Reg && Reg <= PPC::CR4)
- && !MustSaveCR)
- continue;
-
// For 64-bit SVR4 when we have spilled CRs, the spill location
// is SP+8, not a frame-relative slot.
if (isSVR4ABI && isPPC64 && (PPC::CR2 <= Reg && Reg <= PPC::CR4)) {
diff --git a/llvm/test/CodeGen/PowerPC/crsave.ll b/llvm/test/CodeGen/PowerPC/crsave.ll
index 81e7a0adcc8ca1..efa8b3f0120998 100644
--- a/llvm/test/CodeGen/PowerPC/crsave.ll
+++ b/llvm/test/CodeGen/PowerPC/crsave.ll
@@ -15,11 +15,12 @@ entry:
}
; PPC32-LABEL: test_cr2:
-; PPC32: stwu 1, -32(1)
-; PPC32: stw 31, 28(1)
+; PPC32: stwu 1, -[[#%u,AMT:]](1)
+; PPC32: stw 31, [[#AMT - 4]](1)
+; PPC32: .cfi_offset cr2, -[[#%u,DOWN:]]
; PPC32: mfcr 12
-; PPC32-NEXT: stw 12, 24(31)
-; PPC32: lwz 12, 24(31)
+; PPC32-NEXT: stw 12, [[#AMT - DOWN]](31)
+; PPC32: lwz 12, [[#AMT - DOWN]](31)
; PPC32-NEXT: mtocrf 32, 12
; PPC64: .cfi_startproc
@@ -34,7 +35,7 @@ entry:
; PPC64: mtocrf 32, 12
; PPC64: .cfi_endproc
-define i32 @test_cr234() nounwind {
+define i32 @test_cr234() nounwind uwtable {
entry:
%ret = alloca i32, align 4
%0 = call i32 asm sideeffect "\0A\09mtcr $4\0A\09cmpw 2,$2,$1\0A\09cmpw 3,$2,$2\0A\09cmpw 4,$2,$3\0A\09mfcr $0", "=r,r,r,r,r,~{cr2},~{cr3},~{cr4}"(i32 1, i32 2, i32 3, i32 0) nounwind
@@ -45,11 +46,14 @@ entry:
}
; PPC32-LABEL: test_cr234:
-; PPC32: stwu 1, -32(1)
-; PPC32: stw 31, 28(1)
+; PPC32: stwu 1, -[[#%u,AMT:]](1)
+; PPC32: stw 31, [[#AMT - 4]](1)
+; PPC32: .cfi_offset cr2, -[[#%u,DOWN:]]
+; PPC32: .cfi_offset cr3, -[[#DOWN]]
+; PPC32: .cfi_offset cr4, -[[#DOWN]]
; PPC32: mfcr 12
-; PPC32-NEXT: stw 12, 24(31)
-; PPC32: lwz 12, 24(31)
+; PPC32-NEXT: stw 12, [[#AMT - DOWN]](31)
+; PPC32: lwz 12, [[#AMT - DOWN]](31)
; PPC32-NEXT: mtocrf 32, 12
; PPC32-NEXT: mtocrf 16, 12
; PPC32-NEXT: mtocrf 8, 12
|
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 looks correct to me. MustSaveCR
does not reflect the situation that whether cr2 ~ cr4 is spilled or not for SVR4 32-bit. So we can not depend on MustSaveCR
to decide whether the CFI instruction should be generated. And for SVR4 32-bit, the non volatile CRs are stored related to frame-related slot, just below the GPR save area, so it is OK we use the common CFI generation logic at line 1222 instead of a customization one like the one for SVR4 64-bit. For SVR4 64-bit, the non volatile CRs are saved at a fixed address.
I will add some PPC reviewers. Please wait for some days before you commit this.
Thanks very much for fixing this.
llvm/test/CodeGen/PowerPC/crsave.ll
Outdated
; PPC32: stwu 1, -32(1) | ||
; PPC32: stw 31, 28(1) | ||
; PPC32: stwu 1, -[[#%u,AMT:]](1) | ||
; PPC32: stw 31, [[#AMT - 4]](1) |
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.
would you please rebase the case after change 3196005 ? Thanks.
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.
Done. I stopped adding CFI to test_cr234, because cloberAllNvCrField has similar CFI.
Delete the code that skips the CFI for the condition register on ELF32. The code checked !MustSaveCR, which happened only when Subtarget.is32BitELFABI(), where spillCalleeSavedRegisters is spilling cr in a different way. The spill was missing CFI. After deleting this code, a spill of cr2 to cr4 gets CFI in the same way as a spill of r14 to r31. Fixes llvm#83094
4cf34d7
to
260d860
Compare
I don't have commit access to LLVM, so I will need someone else to commit it. |
clang -S was missing a line like ".cfi_offset cr2, -16" in functions that spill cr2 (or cr3, cr4) to the stack. This was breaking a few C++ exceptions. This fix adds the missing CFI. llvm/llvm-project#83098 ok robert@ (maintainer)
Delete the code that skips the CFI for the condition register on ELF32. The code checked !MustSaveCR, which happened only when Subtarget.is32BitELFABI(), where spillCalleeSavedRegisters is spilling cr in a different way. The spill was missing CFI. After deleting this code, a spill of cr2 to cr4 gets CFI in the same way as a spill of r14 to r31. Fixes llvm#83094 (cherry picked from commit 6b70c5d)
Delete the code that skips the CFI for the condition register on ELF32. The code checked !MustSaveCR, which happened only when Subtarget.is32BitELFABI(), where spillCalleeSavedRegisters is spilling cr in a different way. The spill was missing CFI. After deleting this code, a spill of cr2 to cr4 gets CFI in the same way as a spill of r14 to r31. Fixes llvm#83094 (cherry picked from commit 6b70c5d)
Delete the code that skips the CFI for the condition register on ELF32. The code checked !MustSaveCR, which happened only when Subtarget.is32BitELFABI(), where spillCalleeSavedRegisters is spilling cr in a different way. The spill was missing CFI. After deleting this code, a spill of cr2 to cr4 gets CFI in the same way as a spill of r14 to r31.
Fixes #83094