[llvm] r342402 - [X86ISel] Implement byval lowering for Win64 calling convention
Keno Fischer via llvm-commits
llvm-commits at lists.llvm.org
Mon Sep 17 10:37:14 PDT 2018
Author: kfischer
Date: Mon Sep 17 10:37:14 2018
New Revision: 342402
URL: http://llvm.org/viewvc/llvm-project?rev=342402&view=rev
Log:
[X86ISel] Implement byval lowering for Win64 calling convention
Summary:
The IR reference for the `byval` attribute states:
```
This indicates that the pointer parameter should really be passed by value
to the function. The attribute implies that a hidden copy of the pointee is
made between the caller and the callee, so the callee is unable to modify
the value in the caller. This attribute is only valid on LLVM pointer arguments.
```
However, on Win64, this attribute is unimplemented and the raw pointer is
passed to the callee instead. This is problematic, because frontend authors
relying on the implicit hidden copy (as happens for every other calling
convention) will see the passed value silently (if mutable memory) or
loudly (by means of a crash) modified because the callee treats the
location as scratch memory space it is allowed to mutate.
At this point, it's worth taking a step back to understand the context.
In most calling conventions, aggregates that are too large to be passed
in registers, instead get *copied* to the stack at a fixed (computable
from the signature) offset of the stack pointer. At the LLVM, we hide
this hidden copy behind the byval attribute. The caller passes a pointer
to the desired data and the callee receives a pointer, but these pointers
are not the same. In particular, the pointer that the callee receives
points to temporary stack memory allocated as part of the call lowering.
In most calling conventions, this pointer is never realized in registers
or memory. The temporary memory is simply defined by an implicit
offset from the stack pointer at function entry.
Win64, uniquely, works differently. The structure is still passed in
memory, but instead of being stored at an implicit memory offset, the
caller computes a pointer to the temporary memory and passes it to
the callee as a regular pointer (taking up a register, or if all
registers are taken up, an additional stack slot). Presumably, this
was done to allow eliding the copy when passing aggregates through
several functions on the stack.
This explains why ignoring the `byval` attribute mostly works on Win64.
The argument simply gets passed as a pointer and as long as we're ok
with the callee trampling all over that memory, there are no ill effects.
However, it does contradict the documentation of the `byval` attribute
which specifies that there is to be an implicit copy.
Frontends can of course work around this by never emitting the `byval`
attribute for Win64 and creating `alloca`s for the requisite temporary
stack slots (and that does appear to be what frontends are doing).
However, the presence of the `byval` attribute is not a trap for
frontend authors, since it seems to work, but silently modifies the
passed memory contrary to documentation.
I see two solutions:
- Disallow the `byval` attribute in the verifier if using the Win64
calling convention.
- Make it work by simply emitting a temporary stack copy as we would
with any other calling convention (frontends can of course always
not use the attribute if they want to elide the copy).
This patch implements the second option (make it work), though I would
be fine with the first also.
Ref: https://github.com/JuliaLang/julia/issues/28338
Reviewers: rnk
Subscribers: llvm-commits
Differential Revision: https://reviews.llvm.org/D51842
Added:
llvm/trunk/test/CodeGen/X86/win64-byval.ll
Modified:
llvm/trunk/lib/Target/X86/X86CallingConv.td
llvm/trunk/lib/Target/X86/X86ISelLowering.cpp
Modified: llvm/trunk/lib/Target/X86/X86CallingConv.td
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/X86/X86CallingConv.td?rev=342402&r1=342401&r2=342402&view=diff
==============================================================================
--- llvm/trunk/lib/Target/X86/X86CallingConv.td (original)
+++ llvm/trunk/lib/Target/X86/X86CallingConv.td Mon Sep 17 10:37:14 2018
@@ -590,9 +590,11 @@ def CC_X86_64_HHVM_C : CallingConv<[
// Calling convention used on Win64
def CC_X86_Win64_C : CallingConv<[
- // FIXME: Handle byval stuff.
// FIXME: Handle varargs.
+ // Byval aggregates are passed by pointer
+ CCIfByVal<CCPassIndirect<i64>>,
+
// Promote i1/v1i1 arguments to i8.
CCIfType<[i1, v1i1], CCPromoteToType<i8>>,
Modified: llvm/trunk/lib/Target/X86/X86ISelLowering.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/X86/X86ISelLowering.cpp?rev=342402&r1=342401&r2=342402&view=diff
==============================================================================
--- llvm/trunk/lib/Target/X86/X86ISelLowering.cpp (original)
+++ llvm/trunk/lib/Target/X86/X86ISelLowering.cpp Mon Sep 17 10:37:14 2018
@@ -3148,7 +3148,7 @@ SDValue X86TargetLowering::LowerFormalAr
}
// If value is passed via pointer - do a load.
- if (VA.getLocInfo() == CCValAssign::Indirect)
+ if (VA.getLocInfo() == CCValAssign::Indirect && !Ins[I].Flags.isByVal())
ArgValue =
DAG.getLoad(VA.getValVT(), dl, Chain, ArgValue, MachinePointerInfo());
@@ -3631,13 +3631,29 @@ X86TargetLowering::LowerCall(TargetLower
Arg = DAG.getBitcast(RegVT, Arg);
break;
case CCValAssign::Indirect: {
- // Store the argument.
- SDValue SpillSlot = DAG.CreateStackTemporary(VA.getValVT());
- int FI = cast<FrameIndexSDNode>(SpillSlot)->getIndex();
- Chain = DAG.getStore(
- Chain, dl, Arg, SpillSlot,
- MachinePointerInfo::getFixedStack(DAG.getMachineFunction(), FI));
- Arg = SpillSlot;
+ if (isByVal) {
+ // Memcpy the argument to a temporary stack slot to prevent
+ // the caller from seeing any modifications the callee may make
+ // as guaranteed by the `byval` attribute.
+ int FrameIdx = MF.getFrameInfo().CreateStackObject(
+ Flags.getByValSize(), std::max(16, (int)Flags.getByValAlign()),
+ false);
+ SDValue StackSlot =
+ DAG.getFrameIndex(FrameIdx, getPointerTy(DAG.getDataLayout()));
+ Chain =
+ CreateCopyOfByValArgument(Arg, StackSlot, Chain, Flags, DAG, dl);
+ // From now on treat this as a regular pointer
+ Arg = StackSlot;
+ isByVal = false;
+ } else {
+ // Store the argument.
+ SDValue SpillSlot = DAG.CreateStackTemporary(VA.getValVT());
+ int FI = cast<FrameIndexSDNode>(SpillSlot)->getIndex();
+ Chain = DAG.getStore(
+ Chain, dl, Arg, SpillSlot,
+ MachinePointerInfo::getFixedStack(DAG.getMachineFunction(), FI));
+ Arg = SpillSlot;
+ }
break;
}
}
Added: llvm/trunk/test/CodeGen/X86/win64-byval.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/X86/win64-byval.ll?rev=342402&view=auto
==============================================================================
--- llvm/trunk/test/CodeGen/X86/win64-byval.ll (added)
+++ llvm/trunk/test/CodeGen/X86/win64-byval.ll Mon Sep 17 10:37:14 2018
@@ -0,0 +1,34 @@
+; RUN: llc -mtriple x86_64-w64-mingw32 %s -o - | FileCheck %s
+
+declare void @foo({ float, double }* byval)
+ at G = external constant { float, double }
+
+define void @bar()
+{
+; Make sure we're creating a temporary stack slot, rather than just passing
+; the pointer through unmodified.
+; CHECK-LABEL: @bar
+; CHECK: movq .refptr.G(%rip), %rax
+; CHECK: movq (%rax), %rcx
+; CHECK: movq 8(%rax), %rax
+; CHECK: movq %rax, 40(%rsp)
+; CHECK: movq %rcx, 32(%rsp)
+; CHECK: leaq 32(%rsp), %rcx
+ call void @foo({ float, double }* byval @G)
+ ret void
+}
+
+define void @baz({ float, double }* byval %arg)
+{
+; On Win64 the byval is effectively ignored on declarations, since we do
+; pass a real pointer in registers. However, by our semantics if we pass
+; the pointer on to another byval function, we do need to make a copy.
+; CHECK-LABEL: @baz
+; CHECK: movq (%rcx), %rax
+; CHECK: movq 8(%rcx), %rcx
+; CHECK: movq %rcx, 40(%rsp)
+; CHECK: movq %rax, 32(%rsp)
+; CHECK: leaq 32(%rsp), %rcx
+ call void @foo({ float, double }* byval %arg)
+ ret void
+}
More information about the llvm-commits
mailing list