Skip to content

[ARM] Fix expansion of ABS in a call sequence #147270

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

Merged
merged 1 commit into from
Jul 7, 2025

Conversation

jayfoad
Copy link
Contributor

@jayfoad jayfoad commented Jul 7, 2025

Fixes #147162

@llvmbot
Copy link
Member

llvmbot commented Jul 7, 2025

@llvm/pr-subscribers-backend-arm

Author: Jay Foad (jayfoad)

Changes

Fixes #147162


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

2 Files Affected:

  • (modified) llvm/lib/Target/ARM/ARMISelLowering.cpp (+5)
  • (modified) llvm/test/CodeGen/ARM/iabs.ll (+25)
diff --git a/llvm/lib/Target/ARM/ARMISelLowering.cpp b/llvm/lib/Target/ARM/ARMISelLowering.cpp
index eb3c741c6ded2..5378ca2a485d4 100644
--- a/llvm/lib/Target/ARM/ARMISelLowering.cpp
+++ b/llvm/lib/Target/ARM/ARMISelLowering.cpp
@@ -12359,6 +12359,11 @@ ARMTargetLowering::EmitInstrWithCustomInserter(MachineInstr &MI,
     Fn->insert(BBI, RSBBB);
     Fn->insert(BBI, SinkBB);
 
+    // Set the call frame size on entry to the new basic blocks.
+    unsigned CallFrameSize = TII->getCallFrameSizeAt(MI);
+    RSBBB->setCallFrameSize(CallFrameSize);
+    SinkBB->setCallFrameSize(CallFrameSize);
+
     Register ABSSrcReg = MI.getOperand(1).getReg();
     Register ABSDstReg = MI.getOperand(0).getReg();
     bool ABSSrcKIll = MI.getOperand(1).isKill();
diff --git a/llvm/test/CodeGen/ARM/iabs.ll b/llvm/test/CodeGen/ARM/iabs.ll
index fffa9555b2966..758fe7507c0b2 100644
--- a/llvm/test/CodeGen/ARM/iabs.ll
+++ b/llvm/test/CodeGen/ARM/iabs.ll
@@ -50,3 +50,28 @@ define i64 @test3(i64 %a) {
   %abs = select i1 %b, i64 %a, i64 %tmp1neg
   ret i64 %abs
 }
+
+declare void @callee(...)
+
+define void @testcallframe(i32 %a) {
+; CHECK-LABEL: testcallframe:
+; CHECK:       @ %bb.0: @ %bb
+; CHECK-NEXT:    .save {r11, lr}
+; CHECK-NEXT:    push {r11, lr}
+; CHECK-NEXT:    .pad #8
+; CHECK-NEXT:    sub sp, sp, #8
+; CHECK-NEXT:    cmp r0, #0
+; CHECK-NEXT:    mov r1, #0
+; CHECK-NEXT:    rsbmi r0, r0, #0
+; CHECK-NEXT:    mov r2, #0
+; CHECK-NEXT:    mov r3, #0
+; CHECK-NEXT:    str r1, [sp]
+; CHECK-NEXT:    bl callee
+; CHECK-NEXT:    add sp, sp, #8
+; CHECK-NEXT:    pop {r11, lr}
+; CHECK-NEXT:    bx lr
+bb:
+  %i = tail call i32 @llvm.abs.i32(i32 %a, i1 false)
+  tail call void @callee(i32 %i, i32 0, i32 0, i32 0, i32 0)
+  ret void
+}

@jayfoad jayfoad requested review from mstorsjo and JonPsson1 July 7, 2025 10:21
@mstorsjo mstorsjo requested a review from davemgreen July 7, 2025 10:39
@mstorsjo
Copy link
Member

mstorsjo commented Jul 7, 2025

#147195 is another attempt at solving the same issue. Thanks for both fixes! I haven't had time to look at them and I'm probably not qualified to comment on them anyway though.

Copy link
Collaborator

@davemgreen davemgreen left a comment

Choose a reason for hiding this comment

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

It's a good sign that we wrote the same thing. LGTM

@jayfoad jayfoad merged commit 17d6aa0 into llvm:main Jul 7, 2025
11 checks passed
@jayfoad jayfoad deleted the arm-callframesize branch July 7, 2025 14:53
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.

[ARM] Expensive checks failures
4 participants