[llvm] [AArch64][GlobalISel] Fix incorrect ABI when tail call not supported (PR #70215)

[Nikita Popov via llvm-commits]

https://github.com/nikic updated https://github.com/llvm/llvm-project/pull/70215

>From 39f0a8782967ec33d8964d18665b98f119d4a331 Mon Sep 17 00:00:00 2001
From: Nikita Popov <npopov at redhat.com>
Date: Wed, 25 Oct 2023 15:39:54 +0200
Subject: [PATCH 1/2] [AArch64] Fix incorrect ABI when tail call not supported

The check for whether a tail call is supported calls
determineAssignments(), which may modify argument flags. As such,
even though the check fails and a non-tail call will be emitted,
it will not have a different (incorrect) ABI.

Fix this by operating on a separate copy of the arguments.
---
 .../AArch64/GISel/AArch64CallLowering.cpp      |  5 ++++-
 .../call-lowering-tail-call-fallback.ll        | 18 +++++++-----------
 2 files changed, 11 insertions(+), 12 deletions(-)

diff --git a/llvm/lib/Target/AArch64/GISel/AArch64CallLowering.cpp b/llvm/lib/Target/AArch64/GISel/AArch64CallLowering.cpp
index 2d6cc870f98e77f..02a0c58c8fd0f64 100644
--- a/llvm/lib/Target/AArch64/GISel/AArch64CallLowering.cpp
+++ b/llvm/lib/Target/AArch64/GISel/AArch64CallLowering.cpp
@@ -854,7 +854,10 @@ bool AArch64CallLowering::areCalleeOutgoingArgsTailCallable(
 
   AArch64OutgoingValueAssigner CalleeAssigner(AssignFnFixed, AssignFnVarArg,
                                               Subtarget, /*IsReturn*/ false);
-  if (!determineAssignments(CalleeAssigner, OutArgs, OutInfo)) {
+  // determineAssignments() may modify argument flags, so make a copy.
+  SmallVector<ArgInfo, 8> OutArgsCopy;
+  append_range(OutArgsCopy, OutArgs);
+  if (!determineAssignments(CalleeAssigner, OutArgsCopy, OutInfo)) {
     LLVM_DEBUG(dbgs() << "... Could not analyze call operands.\n");
     return false;
   }
diff --git a/llvm/test/CodeGen/AArch64/GlobalISel/call-lowering-tail-call-fallback.ll b/llvm/test/CodeGen/AArch64/GlobalISel/call-lowering-tail-call-fallback.ll
index fc6eefb4016b66d..ebd2beca6781050 100644
--- a/llvm/test/CodeGen/AArch64/GlobalISel/call-lowering-tail-call-fallback.ll
+++ b/llvm/test/CodeGen/AArch64/GlobalISel/call-lowering-tail-call-fallback.ll
@@ -3,30 +3,26 @@
 
 declare void @func(i64, i64, i64, i64, i64, i128, i128)
 
-; FIXME: This is a miscompile.
 ; Make sure the check for whether a tail call is allowed does not affect the
 ; calling convention if it fails.
 ; The first i128 argument should be passed in registers, not on the stack.
 define void @pr70207(i128 %arg1, i128 %arg2) nounwind {
 ; CHECK-LABEL: pr70207:
 ; CHECK:       // %bb.0:
-; CHECK-NEXT:    sub sp, sp, #64
+; CHECK-NEXT:    mov x8, x2
 ; CHECK-NEXT:    mov x6, x0
-; CHECK-NEXT:    mov x8, x1
-; CHECK-NEXT:    mov x9, x2
-; CHECK-NEXT:    mov x10, x3
+; CHECK-NEXT:    mov x7, x1
+; CHECK-NEXT:    mov x9, x3
 ; CHECK-NEXT:    mov x0, xzr
 ; CHECK-NEXT:    mov x1, xzr
 ; CHECK-NEXT:    mov x2, xzr
 ; CHECK-NEXT:    mov x3, xzr
 ; CHECK-NEXT:    mov x4, xzr
-; CHECK-NEXT:    str x30, [sp, #48] // 8-byte Folded Spill
-; CHECK-NEXT:    str x8, [sp]
-; CHECK-NEXT:    str x9, [sp, #16]
-; CHECK-NEXT:    str x10, [sp, #32]
+; CHECK-NEXT:    str x8, [sp, #-32]!
+; CHECK-NEXT:    stp x9, x30, [sp, #8] // 8-byte Folded Spill
 ; CHECK-NEXT:    bl func
-; CHECK-NEXT:    ldr x30, [sp, #48] // 8-byte Folded Reload
-; CHECK-NEXT:    add sp, sp, #64
+; CHECK-NEXT:    ldr x30, [sp, #16] // 8-byte Folded Reload
+; CHECK-NEXT:    add sp, sp, #32
 ; CHECK-NEXT:    ret
   tail call void @func(i64 0, i64 0, i64 0, i64 0, i64 0, i128 %arg1, i128 %arg2)
   ret void

>From 99d6b153b3a90b83a053cf60276290e3ce214d96 Mon Sep 17 00:00:00 2001
From: Nikita Popov <npopov at redhat.com>
Date: Mon, 30 Oct 2023 09:39:31 +0100
Subject: [PATCH 2/2] Make sure the modified OutArgs is used by later code

---
 llvm/lib/Target/AArch64/GISel/AArch64CallLowering.cpp | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/llvm/lib/Target/AArch64/GISel/AArch64CallLowering.cpp b/llvm/lib/Target/AArch64/GISel/AArch64CallLowering.cpp
index 02a0c58c8fd0f64..84057ea8d2214ac 100644
--- a/llvm/lib/Target/AArch64/GISel/AArch64CallLowering.cpp
+++ b/llvm/lib/Target/AArch64/GISel/AArch64CallLowering.cpp
@@ -832,9 +832,9 @@ bool AArch64CallLowering::doCallerAndCalleePassArgsTheSameWay(
 
 bool AArch64CallLowering::areCalleeOutgoingArgsTailCallable(
     CallLoweringInfo &Info, MachineFunction &MF,
-    SmallVectorImpl<ArgInfo> &OutArgs) const {
+    SmallVectorImpl<ArgInfo> &OrigOutArgs) const {
   // If there are no outgoing arguments, then we are done.
-  if (OutArgs.empty())
+  if (OrigOutArgs.empty())
     return true;
 
   const Function &CallerF = MF.getFunction();
@@ -855,9 +855,9 @@ bool AArch64CallLowering::areCalleeOutgoingArgsTailCallable(
   AArch64OutgoingValueAssigner CalleeAssigner(AssignFnFixed, AssignFnVarArg,
                                               Subtarget, /*IsReturn*/ false);
   // determineAssignments() may modify argument flags, so make a copy.
-  SmallVector<ArgInfo, 8> OutArgsCopy;
-  append_range(OutArgsCopy, OutArgs);
-  if (!determineAssignments(CalleeAssigner, OutArgsCopy, OutInfo)) {
+  SmallVector<ArgInfo, 8> OutArgs;
+  append_range(OutArgs, OrigOutArgs);
+  if (!determineAssignments(CalleeAssigner, OutArgs, OutInfo)) {
     LLVM_DEBUG(dbgs() << "... Could not analyze call operands.\n");
     return false;
   }