r245719 - Properly provide alignment of 'byval' arguments down to llvm.
David Blaikie via cfe-commits
cfe-commits at lists.llvm.org
Fri Jun 10 14:26:44 PDT 2016
That was certainly one of the counterarguments (& global variables also use
a type rather than a size). I've not really settled on which way to
go/haven't given it lots of thought. I may loop back around to the original
thread when it comes to that.
On Fri, Jun 10, 2016 at 2:18 PM, James Y Knight <jyknight at google.com> wrote:
> Yes, it was intended to -- at least for bitcode produced by clang.
>
> I do think it would be a good idea to continue to pass the value type to
> byval, though...Either that or get rid of the type in the "alloca"
> instruction. They're basically doing the same thing, and having them
> specified completely differently seems unfortunate.
>
> On Fri, Jun 10, 2016 at 4:58 PM, David Blaikie <dblaikie at gmail.com> wrote:
>
>> Excuse the necromancy, but do you know if this change (or other work you
>> did in this area) completely eclipsed LLVM's use of inferred alignment via
>> the llvm struct's alignment for byval arguments?
>>
>> I ask because this was something I was going to need to fix for the
>> typeless pointer work & I have some outdated patches, etc, that I can throw
>> away if that's the case (at least it looks like it touched most of the
>> places my patch did).
>>
>> (I'll still need to remove the pointer type in favor of a number of bytes
>> for byval (byval to copy (or moving the type from the argument's pointer
>> type, into a parameter to byval, eg: %ptr byval(struct_foo) %arg) but
>> knowing the alignment's totally handled would be a good first step in that
>> work)
>>
>> On Fri, Aug 21, 2015 at 11:19 AM, James Y Knight via cfe-commits <
>> cfe-commits at lists.llvm.org> wrote:
>>
>>> Author: jyknight
>>> Date: Fri Aug 21 13:19:06 2015
>>> New Revision: 245719
>>>
>>> URL: http://llvm.org/viewvc/llvm-project?rev=245719&view=rev
>>> Log:
>>> Properly provide alignment of 'byval' arguments down to llvm.
>>>
>>> This is important in the case that the LLVM-inferred llvm-struct
>>> alignment is not the same as the clang-known C-struct alignment.
>>>
>>> Differential Revision: http://reviews.llvm.org/D12243
>>>
>>> Added:
>>> cfe/trunk/test/CodeGen/sparc-arguments.c
>>> Modified:
>>> cfe/trunk/lib/CodeGen/CGCall.cpp
>>> cfe/trunk/test/CodeGen/le32-arguments.c
>>> cfe/trunk/test/CodeGen/nvptx-abi.c
>>>
>>> Modified: cfe/trunk/lib/CodeGen/CGCall.cpp
>>> URL:
>>> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGCall.cpp?rev=245719&r1=245718&r2=245719&view=diff
>>>
>>> ==============================================================================
>>> --- cfe/trunk/lib/CodeGen/CGCall.cpp (original)
>>> +++ cfe/trunk/lib/CodeGen/CGCall.cpp Fri Aug 21 13:19:06 2015
>>> @@ -1663,20 +1663,35 @@ void CodeGenModule::ConstructAttributeLi
>>> Attrs.addAttribute(llvm::Attribute::InReg);
>>> break;
>>>
>>> - case ABIArgInfo::Indirect:
>>> + case ABIArgInfo::Indirect: {
>>> if (AI.getInReg())
>>> Attrs.addAttribute(llvm::Attribute::InReg);
>>>
>>> if (AI.getIndirectByVal())
>>> Attrs.addAttribute(llvm::Attribute::ByVal);
>>>
>>> - Attrs.addAlignmentAttr(AI.getIndirectAlign());
>>> + unsigned Align = AI.getIndirectAlign();
>>> +
>>> + // In a byval argument, it is important that the required
>>> + // alignment of the type is honored, as LLVM might be creating a
>>> + // *new* stack object, and needs to know what alignment to give
>>> + // it. (Sometimes it can deduce a sensible alignment on its own,
>>> + // but not if clang decides it must emit a packed struct, or the
>>> + // user specifies increased alignment requirements.)
>>> + //
>>> + // This is different from indirect *not* byval, where the object
>>> + // exists already, and the align attribute is purely
>>> + // informative.
>>> + if (Align == 0 && AI.getIndirectByVal())
>>> + Align =
>>> getContext().getTypeAlignInChars(ParamType).getQuantity();
>>> +
>>> + Attrs.addAlignmentAttr(Align);
>>>
>>> // byval disables readnone and readonly.
>>> FuncAttrs.removeAttribute(llvm::Attribute::ReadOnly)
>>> .removeAttribute(llvm::Attribute::ReadNone);
>>> break;
>>> -
>>> + }
>>> case ABIArgInfo::Ignore:
>>> case ABIArgInfo::Expand:
>>> continue;
>>>
>>> Modified: cfe/trunk/test/CodeGen/le32-arguments.c
>>> URL:
>>> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/le32-arguments.c?rev=245719&r1=245718&r2=245719&view=diff
>>>
>>> ==============================================================================
>>> --- cfe/trunk/test/CodeGen/le32-arguments.c (original)
>>> +++ cfe/trunk/test/CodeGen/le32-arguments.c Fri Aug 21 13:19:06 2015
>>> @@ -10,7 +10,7 @@ typedef struct {
>>> int bb;
>>> } s1;
>>> // Structs should be passed byval and not split up
>>> -// CHECK-LABEL: define void @f1(%struct.s1* byval %i)
>>> +// CHECK-LABEL: define void @f1(%struct.s1* byval align 4 %i)
>>> void f1(s1 i) {}
>>>
>>> typedef struct {
>>> @@ -48,7 +48,7 @@ union simple_union {
>>> char b;
>>> };
>>> // Unions should be passed as byval structs
>>> -// CHECK-LABEL: define void @f7(%union.simple_union* byval %s)
>>> +// CHECK-LABEL: define void @f7(%union.simple_union* byval align 4 %s)
>>> void f7(union simple_union s) {}
>>>
>>> typedef struct {
>>> @@ -57,5 +57,5 @@ typedef struct {
>>> int b8 : 8;
>>> } bitfield1;
>>> // Bitfields should be passed as byval structs
>>> -// CHECK-LABEL: define void @f8(%struct.bitfield1* byval %bf1)
>>> +// CHECK-LABEL: define void @f8(%struct.bitfield1* byval align 4 %bf1)
>>> void f8(bitfield1 bf1) {}
>>>
>>> Modified: cfe/trunk/test/CodeGen/nvptx-abi.c
>>> URL:
>>> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/nvptx-abi.c?rev=245719&r1=245718&r2=245719&view=diff
>>>
>>> ==============================================================================
>>> --- cfe/trunk/test/CodeGen/nvptx-abi.c (original)
>>> +++ cfe/trunk/test/CodeGen/nvptx-abi.c Fri Aug 21 13:19:06 2015
>>> @@ -21,14 +21,14 @@ float bar(void) {
>>>
>>> void foo(float4_t x) {
>>> // CHECK-LABEL: @foo
>>> -// CHECK: %struct.float4_s* byval %x
>>> +// CHECK: %struct.float4_s* byval align 4 %x
>>> }
>>>
>>> void fooN(float4_t x, float4_t y, float4_t z) {
>>> // CHECK-LABEL: @fooN
>>> -// CHECK: %struct.float4_s* byval %x
>>> -// CHECK: %struct.float4_s* byval %y
>>> -// CHECK: %struct.float4_s* byval %z
>>> +// CHECK: %struct.float4_s* byval align 4 %x
>>> +// CHECK: %struct.float4_s* byval align 4 %y
>>> +// CHECK: %struct.float4_s* byval align 4 %z
>>> }
>>>
>>> typedef struct nested_s {
>>> @@ -39,5 +39,5 @@ typedef struct nested_s {
>>>
>>> void baz(nested_t x) {
>>> // CHECK-LABEL: @baz
>>> -// CHECK: %struct.nested_s* byval %x)
>>> +// CHECK: %struct.nested_s* byval align 8 %x)
>>> }
>>>
>>> Added: cfe/trunk/test/CodeGen/sparc-arguments.c
>>> URL:
>>> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/sparc-arguments.c?rev=245719&view=auto
>>>
>>> ==============================================================================
>>> --- cfe/trunk/test/CodeGen/sparc-arguments.c (added)
>>> +++ cfe/trunk/test/CodeGen/sparc-arguments.c Fri Aug 21 13:19:06 2015
>>> @@ -0,0 +1,27 @@
>>> +// RUN: %clang_cc1 -triple sparc-unknown-unknown -emit-llvm -o - %s |
>>> FileCheck %s
>>> +
>>> +// Ensure that we pass proper alignment to llvm in the call
>>> +// instruction. The proper alignment for the type is sometimes known
>>> +// only by clang, and is not manifest in the LLVM-type. So, it must be
>>> +// explicitly passed through. (Besides the case of the user specifying
>>> +// alignment, as here, this situation also occurrs for non-POD C++
>>> +// structs with tail-padding: clang emits these as packed llvm-structs
>>> +// for ABI reasons.)
>>> +
>>> +struct s1 {
>>> + int x;
>>> +} __attribute__((aligned(8)));
>>> +
>>> +struct s1 x1;
>>> +
>>> +
>>> +// Ensure the align 8 is passed through:
>>> +// CHECK-LABEL: define void @f1()
>>> +// CHECK: call void @f1_helper(%struct.s1* byval align 8 @x1)
>>> +// Also ensure the declaration of f1_helper includes it
>>> +// CHECK: declare void @f1_helper(%struct.s1* byval align 8)
>>> +
>>> +void f1_helper(struct s1);
>>> +void f1() {
>>> + f1_helper(x1);
>>> +}
>>>
>>>
>>> _______________________________________________
>>> cfe-commits mailing list
>>> cfe-commits at lists.llvm.org
>>> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>>>
>>
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20160610/ed7771d9/attachment.html>
More information about the cfe-commits
mailing list