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 <jykni...@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 <dblai...@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@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@lists.llvm.org >>> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits >>> >> >> >
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits