[llvm] x86: fix musttail sibcall miscompilation (PR #168956)
Folkert de Vries via llvm-commits
llvm-commits at lists.llvm.org
Tue Nov 25 04:26:17 PST 2025
================
@@ -2404,16 +2530,27 @@ X86TargetLowering::LowerCall(TargetLowering::CallLoweringInfo &CLI,
FIN = DAG.getFrameIndex(FI, getPointerTy(DAG.getDataLayout()));
if (Flags.isByVal()) {
- // Copy relative to framepointer.
- SDValue Source = DAG.getIntPtrConstant(VA.getLocMemOffset(), dl);
- if (!StackPtr.getNode())
- StackPtr = DAG.getCopyFromReg(Chain, dl, RegInfo->getStackRegister(),
- getPointerTy(DAG.getDataLayout()));
- Source = DAG.getNode(ISD::ADD, dl, getPointerTy(DAG.getDataLayout()),
- StackPtr, Source);
-
- MemOpChains2.push_back(
- CreateCopyOfByValArgument(Source, FIN, Chain, Flags, DAG, dl));
+ SDValue ByValSrc;
+ bool NeedsStackCopy;
+ if (auto It = ByValTemporaries.find(OutsIndex);
+ It != ByValTemporaries.end()) {
+ ByValSrc = It->second;
+ NeedsStackCopy = true;
+ } else {
+ ByValSrc = Arg;
+ NeedsStackCopy = !isTailCall;
+ }
+
+ // FIXME: contrary to the arm backend, with the current logic we always
+ // seem to need a stack copy.
+ (void)NeedsStackCopy;
----------------
folkertdev wrote:
This unfortunately is still needed, guarding the copy below with `if (NeedsStackCopy)` leads to this diff
```diff
@@ -76,10 +76,8 @@ define dso_local i32 @swapByValArguments(ptr byval(%struct.1xi32) %0, ptr byval(
; CHECK: # %bb.0:
; CHECK-NEXT: mov eax, dword ptr [rsp + 8]
; CHECK-NEXT: mov dword ptr [rsp - 16], eax
-; CHECK-NEXT: mov ecx, dword ptr [rsp + 16]
-; CHECK-NEXT: mov dword ptr [rsp - 8], ecx
-; CHECK-NEXT: mov dword ptr [rsp + 8], ecx
-; CHECK-NEXT: mov dword ptr [rsp + 16], eax
+; CHECK-NEXT: mov eax, dword ptr [rsp + 16]
+; CHECK-NEXT: mov dword ptr [rsp - 8], eax
; CHECK-NEXT: jmp swap # TAILCALL
```
i.e. the values are stored on the stack, but then no longer copied back into the correct argument position.
https://github.com/llvm/llvm-project/pull/168956
More information about the llvm-commits
mailing list