[PATCH] D146242: [ARM] Fixing ABI mismatch for packed structs passed as function arguments

[Jirui Wu via Phabricator via cfe-commits]

JiruiWu added inline comments.



Comment at: clang/test/CodeGen/aarch64-ABI-align-packed-assembly.c:100
   struct packed_struct on_callee_stack;
   on_callee_stack = va_arg(vl, struct packed_struct);
 }

chill wrote:
> Can we add some `CHECK:` lines here and to other variadic functions as well 
> (I recognize it might not be straightforward)?
I compared the assembly output before and after applying my patch, there is no 
change in this part and thus I think there is no need for additional `CHECK:` 
lines.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D146242/new/

https://reviews.llvm.org/D146242

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D146242: [ARM] Fixing ABI mismatch for packed structs passed as function arguments

[Jirui Wu via Phabricator via cfe-commits]

JiruiWu added a comment.

Ping: @rjmccall


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D146242/new/

https://reviews.llvm.org/D146242

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D152932: [ARM] Adding precommit tests for D146242

[Jirui Wu via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG16902df6f25a: [ARM] Adding precommit tests for D146242 
(authored by JiruiWu).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D152932/new/

https://reviews.llvm.org/D152932

Files:
  clang/test/CodeGen/aarch64-ABI-align-packed-assembly.c
  clang/test/CodeGen/aarch64-ABI-align-packed.c

Index: clang/test/CodeGen/aarch64-ABI-align-packed.c
===
--- /dev/null
+++ clang/test/CodeGen/aarch64-ABI-align-packed.c
@@ -0,0 +1,436 @@
+// REQUIRES: aarch64-registered-target
+// RUN: %clang_cc1 -fsyntax-only -triple aarch64-none-eabi -target-feature +neon -emit-llvm -O2 -o - %s | FileCheck %s
+#include 
+#include 
+
+// natural alignment 16, adjusted alignment 16
+// expected alignment of copy on callee stack: 16
+struct non_packed_struct {
+  uint16x8_t M0; // member alignment 16
+};
+
+// natural alignment 1, adjusted alignment 1
+// expected alignment of copy on callee stack: 8
+struct __attribute((packed)) packed_struct {
+  uint16x8_t M0; // member alignment 1, because the field is packed when the struct is packed
+};
+
+// natural alignment 1, adjusted alignment 1
+// expected alignment of copy on callee stack: 8
+struct packed_member {
+  uint16x8_t M0 __attribute((packed)); // member alignment 1
+};
+
+// natural alignment 16, adjusted alignment 16 since __attribute((aligned (n))) sets the minimum alignment
+// expected alignment of copy on callee stack: 16
+struct __attribute((aligned (8))) aligned_struct_8 {
+  uint16x8_t M0; // member alignment 16
+};
+
+// natural alignment 16, adjusted alignment 16
+// expected alignment of copy on callee stack: 16
+struct aligned_member_8 {
+  uint16x8_t M0 __attribute((aligned (8))); // member alignment 16 since __attribute((aligned (n))) sets the minimum alignment
+};
+
+// natural alignment 8, adjusted alignment 8
+// expected alignment of copy on callee stack: 8
+#pragma pack(8)
+struct pragma_packed_struct_8 {
+  uint16x8_t M0; // member alignment 8 because the struct is subject to packed(8)
+};
+
+// natural alignment 4, adjusted alignment 4
+// expected alignment of copy on callee stack: 8
+#pragma pack(4)
+struct pragma_packed_struct_4 {
+  uint16x8_t M0; // member alignment 4 because the struct is subject to packed(4)
+};
+
+double gd;
+void init(int, ...);
+
+struct non_packed_struct gs_non_packed_struct;
+
+// CHECK-LABEL: define dso_local void @named_arg_non_packed_struct
+// CHECK-SAME: (double [[D0:%.*]], double [[D1:%.*]], double [[D2:%.*]], double [[D3:%.*]], double [[D4:%.*]], double [[D5:%.*]], double [[D6:%.*]], double [[D7:%.*]], double noundef [[D8:%.*]], [1 x <8 x i16>] [[S_NON_PACKED_STRUCT_COERCE:%.*]]) local_unnamed_addr #[[ATTR0:[0-9]+]] {
+// CHECK-NEXT:  entry:
+// CHECK-NEXT:[[S_NON_PACKED_STRUCT_COERCE_FCA_0_EXTRACT:%.*]] = extractvalue [1 x <8 x i16>] [[S_NON_PACKED_STRUCT_COERCE]], 0
+// CHECK-NEXT:store double [[D8]], ptr @gd, align 8, !tbaa [[TBAA2:![0-9]+]]
+// CHECK-NEXT:store <8 x i16> [[S_NON_PACKED_STRUCT_COERCE_FCA_0_EXTRACT]], ptr @gs_non_packed_struct, align 16, !tbaa.struct [[TBAA_STRUCT6:![0-9]+]]
+// CHECK-NEXT:ret void
+__attribute__((noinline)) void named_arg_non_packed_struct(double d0, double d1, double d2, double d3,
+ double d4, double d5, double d6, double d7,
+ double d8, struct non_packed_struct s_non_packed_struct) {
+gd = d8;
+gs_non_packed_struct = s_non_packed_struct;
+}
+
+// CHECK-LABEL: define dso_local void @variadic_non_packed_struct
+// CHECK-SAME: (double [[D0:%.*]], double [[D1:%.*]], double [[D2:%.*]], double [[D3:%.*]], double [[D4:%.*]], double [[D5:%.*]], double [[D6:%.*]], double [[D7:%.*]], double [[D8:%.*]], ...) local_unnamed_addr #[[ATTR1:[0-9]+]] {
+// CHECK-NEXT:  entry:
+// CHECK-NEXT:[[VL:%.*]] = alloca [[STRUCT___VA_LIST:%.*]], align 8
+// CHECK-NEXT:call void @llvm.lifetime.start.p0(i64 32, ptr nonnull [[VL]]) #[[ATTR6:[0-9]+]]
+// CHECK-NEXT:call void @llvm.va_start(ptr nonnull [[VL]])
+// CHECK-NEXT:call void @llvm.lifetime.end.p0(i64 32, ptr nonnull [[VL]]) #[[ATTR6]]
+// CHECK-NEXT:ret void
+void variadic_non_packed_struct(double d0, double d1, double d2, double d3,
+ double d4, double d5, double d6, double d7,
+ double d8, ...) {
+  va_list vl;
+  va_start(vl, d8);
+  struct non_packed_struct on_callee_stack;
+  on_callee_stack = va_arg(vl, struct non_packed_struct);
+}
+
+// CHECK-LABEL: define dso_local void @test_non_packed_struct
+// CHECK-SAME: () local_unnamed_addr #[[ATTR4:[0-9]+]] {
+// CHECK-NEXT:  entry:
+// CHECK-NEXT:[[S_NON_PACKED_STRUCT:%.*]] = alloca [[STRUCT_NON_PACKED_STRUCT:%.*]], align 16
+// CHECK-NEXT:call void @llvm.lifetime.start.p0(i64 16, ptr nonnull [[S_NON_PACKED_STRUCT]]) #[[ATTR6]]
+/

[PATCH] D146242: [ARM] Fixing ABI mismatch for packed structs and fields

[Jirui Wu via Phabricator via cfe-commits]

JiruiWu marked 2 inline comments as done.
JiruiWu added inline comments.



Comment at: clang/lib/CodeGen/TargetInfo.cpp:5811
+// For alignment adjusted HFAs, cap the argument alignment to 16, otherwise
+// set it to 8 according to the AAPCS64 document.
 unsigned Align =

tmatheson wrote:
> Does the similar code added in D100853 need updated too?
No, because this patch is on AArch64 and https://reviews.llvm.org/D100853 is on 
AArch32. The alignment is capped to 16 in AArch64 and capped to 8 in AArch32.



Comment at: clang/lib/CodeGen/TargetInfo.cpp:5814
 getContext().getTypeUnadjustedAlignInChars(Ty).getQuantity();
-unsigned BaseAlign = getContext().getTypeAlignInChars(Base).getQuantity();
-Align = (Align > BaseAlign && Align >= 16) ? 16 : 0;
+Align = (Align >= 16) ? 16 : 8;
 return ABIArgInfo::getDirect(

tmatheson wrote:
> Does this code definitely only apply when the ABI is AAPCS64, or should there 
> be a check for that somewhere here? I can't tell whether the `if` on line 
> 5806 is sufficient.
The code on line 5814 only applies when the ABI is AAPCS64 because it is in the 
`if` block that starts on line 5805 and ends on line 5818. As a result, the 
`if` on line 5806 is sufficient.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D146242/new/

https://reviews.llvm.org/D146242

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D152932: [ARM] Adding precommit tests for D146242

[Jirui Wu via Phabricator via cfe-commits]

JiruiWu updated this revision to Diff 536157.
JiruiWu added a comment.

Update tests, adding an assembly test to better demonstrate the effects of 
D146242 .


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D152932/new/

https://reviews.llvm.org/D152932

Files:
  clang/test/CodeGen/aarch64-ABI-align-packed-assembly.c
  clang/test/CodeGen/aarch64-ABI-align-packed.c

Index: clang/test/CodeGen/aarch64-ABI-align-packed.c
===
--- /dev/null
+++ clang/test/CodeGen/aarch64-ABI-align-packed.c
@@ -0,0 +1,436 @@
+// REQUIRES: aarch64-registered-target
+// RUN: %clang_cc1 -fsyntax-only -triple aarch64-none-eabi -target-feature +neon -emit-llvm -O2 -o - %s | FileCheck %s
+#include 
+#include 
+
+// natural alignment 16, adjusted alignment 16
+// expected alignment of copy on callee stack: 16
+struct non_packed_struct {
+  uint16x8_t M0; // member alignment 16
+};
+
+// natural alignment 1, adjusted alignment 1
+// expected alignment of copy on callee stack: 8
+struct __attribute((packed)) packed_struct {
+  uint16x8_t M0; // member alignment 1, because the field is packed when the struct is packed
+};
+
+// natural alignment 1, adjusted alignment 1
+// expected alignment of copy on callee stack: 8
+struct packed_member {
+  uint16x8_t M0 __attribute((packed)); // member alignment 1
+};
+
+// natural alignment 16, adjusted alignment 16 since __attribute((aligned (n))) sets the minimum alignment
+// expected alignment of copy on callee stack: 16
+struct __attribute((aligned (8))) aligned_struct_8 {
+  uint16x8_t M0; // member alignment 16
+};
+
+// natural alignment 16, adjusted alignment 16
+// expected alignment of copy on callee stack: 16
+struct aligned_member_8 {
+  uint16x8_t M0 __attribute((aligned (8))); // member alignment 16 since __attribute((aligned (n))) sets the minimum alignment
+};
+
+// natural alignment 8, adjusted alignment 8
+// expected alignment of copy on callee stack: 8
+#pragma pack(8)
+struct pragma_packed_struct_8 {
+  uint16x8_t M0; // member alignment 8 because the struct is subject to packed(8)
+};
+
+// natural alignment 4, adjusted alignment 4
+// expected alignment of copy on callee stack: 8
+#pragma pack(4)
+struct pragma_packed_struct_4 {
+  uint16x8_t M0; // member alignment 4 because the struct is subject to packed(4)
+};
+
+double gd;
+void init(int, ...);
+
+struct non_packed_struct gs_non_packed_struct;
+
+// CHECK-LABEL: define dso_local void @named_arg_non_packed_struct
+// CHECK-SAME: (double [[D0:%.*]], double [[D1:%.*]], double [[D2:%.*]], double [[D3:%.*]], double [[D4:%.*]], double [[D5:%.*]], double [[D6:%.*]], double [[D7:%.*]], double noundef [[D8:%.*]], [1 x <8 x i16>] [[S_NON_PACKED_STRUCT_COERCE:%.*]]) local_unnamed_addr #[[ATTR0:[0-9]+]] {
+// CHECK-NEXT:  entry:
+// CHECK-NEXT:[[S_NON_PACKED_STRUCT_COERCE_FCA_0_EXTRACT:%.*]] = extractvalue [1 x <8 x i16>] [[S_NON_PACKED_STRUCT_COERCE]], 0
+// CHECK-NEXT:store double [[D8]], ptr @gd, align 8, !tbaa [[TBAA2:![0-9]+]]
+// CHECK-NEXT:store <8 x i16> [[S_NON_PACKED_STRUCT_COERCE_FCA_0_EXTRACT]], ptr @gs_non_packed_struct, align 16, !tbaa.struct [[TBAA_STRUCT6:![0-9]+]]
+// CHECK-NEXT:ret void
+__attribute__((noinline)) void named_arg_non_packed_struct(double d0, double d1, double d2, double d3,
+ double d4, double d5, double d6, double d7,
+ double d8, struct non_packed_struct s_non_packed_struct) {
+gd = d8;
+gs_non_packed_struct = s_non_packed_struct;
+}
+
+// CHECK-LABEL: define dso_local void @variadic_non_packed_struct
+// CHECK-SAME: (double [[D0:%.*]], double [[D1:%.*]], double [[D2:%.*]], double [[D3:%.*]], double [[D4:%.*]], double [[D5:%.*]], double [[D6:%.*]], double [[D7:%.*]], double [[D8:%.*]], ...) local_unnamed_addr #[[ATTR1:[0-9]+]] {
+// CHECK-NEXT:  entry:
+// CHECK-NEXT:[[VL:%.*]] = alloca [[STRUCT___VA_LIST:%.*]], align 8
+// CHECK-NEXT:call void @llvm.lifetime.start.p0(i64 32, ptr nonnull [[VL]]) #[[ATTR6:[0-9]+]]
+// CHECK-NEXT:call void @llvm.va_start(ptr nonnull [[VL]])
+// CHECK-NEXT:call void @llvm.lifetime.end.p0(i64 32, ptr nonnull [[VL]]) #[[ATTR6]]
+// CHECK-NEXT:ret void
+void variadic_non_packed_struct(double d0, double d1, double d2, double d3,
+ double d4, double d5, double d6, double d7,
+ double d8, ...) {
+  va_list vl;
+  va_start(vl, d8);
+  struct non_packed_struct on_callee_stack;
+  on_callee_stack = va_arg(vl, struct non_packed_struct);
+}
+
+// CHECK-LABEL: define dso_local void @test_non_packed_struct
+// CHECK-SAME: () local_unnamed_addr #[[ATTR4:[0-9]+]] {
+// CHECK-NEXT:  entry:
+// CHECK-NEXT:[[S_NON_PACKED_STRUCT:%.*]] = alloca [[STRUCT_NON_PACKED_STRUCT:%.*]], align 16
+// CHECK-NEXT:call void @llvm.lifetime.start.p0(i64 16, ptr nonnull [[S_NON_PACKED_ST

[PATCH] D146242: [ARM] Fixing ABI mismatch for packed structs passed as function arguments

[Jirui Wu via Phabricator via cfe-commits]

JiruiWu marked an inline comment as done.
JiruiWu added inline comments.



Comment at: clang/lib/CodeGen/TargetInfo.cpp:5813
 getContext().getTypeUnadjustedAlignInChars(Ty).getQuantity();
-unsigned BaseAlign = getContext().getTypeAlignInChars(Base).getQuantity();
-Align = (Align > BaseAlign && Align >= 16) ? 16 : 0;
+Align = (Align >= 16) ? 16 : 8;
 return ABIArgInfo::getDirect(

chill wrote:
> The backend ought to set the minimum alignment of a stack slot to 8 anyway 
> (for AAPCS), hence setting the minimum here to 8 is redundant.
> 
This 8 is necessary. I tried to use 0 instead of 8 but clang set the alignment 
to 16 by default, which is wrong. Specifying the alignment to 8 here fixes the 
problem.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D146242/new/

https://reviews.llvm.org/D146242

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D146242: [ARM] Fixing ABI mismatch for packed structs and fields

[Jirui Wu via Phabricator via cfe-commits]

JiruiWu marked an inline comment as done.
JiruiWu added inline comments.



Comment at: clang/lib/AST/RecordLayoutBuilder.cpp:2118
+  if (Packed)
+UnadjustedAlignment = std::max(UnadjustedAlignment, UnpackedFieldAlign);
   UpdateAlignment(FieldAlign, UnpackedFieldAlign, PreferredAlign);

dblaikie wrote:
> rjmccall wrote:
> > I've always felt the data flow in this function was excessively convoluted. 
> >  Let's puzzle it out to figure out what's going on.  Ignoring the AIX stuff 
> > which I assume can't coincide with AArch64, we've got:
> > 
> > ```
> > UnpackedFieldAlign = min(max(TyAlign, MaxAlignmentInChars), 
> > MaxFieldAlignment)
> > PackedFieldAlign = min(max(1, MaxAlignmentInChars), MaxFieldAlignment)
> > FieldAlign = FieldPacked ? PackedFieldAlign : UnpackedFieldAlign
> > ```
> > 
> > where `MaxAlignmentInChars` is the highest value of all the alignment 
> > attributes on the field and `MaxFieldAlignment` is the value of `#pragma 
> > pack` that was active at the time of the struct definition.
> > 
> > Note that this gives us `PackedFieldAlign <= FieldAlign <= 
> > UnpackedFieldAlign`.
> > 
> > So:
> > 1. I think it's wrong to be checking `Packed` instead of `FieldPacked` 
> > here.  But:
> > 2. If `FieldPacked`, then because `UnpackedFieldAlign >= FieldAlign`, the 
> > net effect of these three lines is `UnadjustedAlignment = 
> > std::max(UnadjustedAlignment, UnpackedFieldAlign)`.
> > 3. If `!FieldPacked`, then `UnpackedFieldAlign == FieldAlign`, so the net 
> > effect of these three lines is *also* `UnadjustedAlignment = 
> > std::max(UnadjustedAlignment, UnpackedFieldAlign)`.
> > 4. So actually you don't need to check `FieldPacked` at all; you should 
> > remove the old line and just do your new one unconditionally.
> > 
> > Also, AAPCS64 seems to define UnadjustedAlignment as the "natural 
> > alignment", and there's a doc comment saying it's the max of the type 
> > alignments.  That makes me wonder if we should really be considering either 
> > the `aligned` attribute or `#pragma pack` in this computation at all; maybe 
> > we should just be looking at the type alignment.
> I think I had a go at this over here & failed, might have some relevant 
> notes: https://reviews.llvm.org/D118511#inline-1140212
> 
> But, yeah, would love to see it simplified, if possible - just the data point 
> that I tried and failed recently :/ (& contributed to some of the current 
> complexity)
I think the logic here is correct.



Comment at: clang/lib/CodeGen/TargetInfo.cpp:5806
   if (!IsWinVariadic && isHomogeneousAggregate(Ty, Base, Members)) {
 if (Kind != AArch64ABIInfo::AAPCS)
   return ABIArgInfo::getDirect(

tmatheson wrote:
> Should this change cover AAPCS_VFP too?
This patch does not cover AAPCS_VFP because AAPCS_VFP is not listed in the 
ABIKind of the class AArch64ABIInfo.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D146242/new/

https://reviews.llvm.org/D146242

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D152932: [ARM] Adding precommit tests for D146242.

[Jirui Wu via Phabricator via cfe-commits]

JiruiWu created this revision.
JiruiWu added reviewers: olista01, simon_tatham, rjmccall, tmatheson, pratlucas.
Herald added a subscriber: kristof.beyls.
Herald added a project: All.
JiruiWu requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Adding precommit tests to better demonstrate the effects of D146242 
.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D152932

Files:
  clang/test/CodeGen/aarch64-ABI-align-packed.c

Index: clang/test/CodeGen/aarch64-ABI-align-packed.c
===
--- /dev/null
+++ clang/test/CodeGen/aarch64-ABI-align-packed.c
@@ -0,0 +1,240 @@
+// RUN: %clang_cc1 -fsyntax-only -triple aarch64-none-eabi \
+// RUN:   -target-feature +neon \
+// RUN:   -emit-llvm -o - %s | FileCheck %s
+#include 
+#include 
+
+// natural alignment 16, adjusted alignment 16
+// expected alignment of copy on callee stack: 16
+struct non_packed_struct {
+  uint16x8_t M0; // member alignment 16
+};
+
+// natural alignment 1, adjusted alignment 1
+// expected alignment of copy on callee stack: 8
+struct __attribute((packed)) packed_struct {
+  uint16x8_t M0; // member alignment 1, because the field is packed when the struct is packed
+};
+
+// natural alignment 1, adjusted alignment 1
+// expected alignment of copy on callee stack: 8
+struct packed_member {
+  uint16x8_t M0 __attribute((packed)); // member alignment 1
+};
+
+// natural alignment 16, adjusted alignment 16 since __attribute((aligned (n))) sets the minimum alignment
+// expected alignment of copy on callee stack: 16
+struct __attribute((aligned (8))) aligned_struct_8 {
+  uint16x8_t M0; // member alignment 16
+};
+
+// natural alignment 16, adjusted alignment 16
+// expected alignment of copy on callee stack: 16
+struct aligned_member_8 {
+  uint16x8_t M0 __attribute((aligned (8))); // member alignment 16 since __attribute((aligned (n))) sets the minimum alignment
+};
+
+// natural alignment 8, adjusted alignment 8
+// expected alignment of copy on callee stack: 8
+#pragma pack(8)
+struct pragma_packed_struct_8 {
+  uint16x8_t M0; // member alignment 8 because the struct is subject to packed(8)
+};
+
+// natural alignment 4, adjusted alignment 4
+// expected alignment of copy on callee stack: 8
+#pragma pack(4)
+struct pragma_packed_struct_4 {
+  uint16x8_t M0; // member alignment 4 because the struct is subject to packed(4)
+};
+
+// Struct passed as a named argument
+// CHECK-LABEL: define dso_local void @named_arg_non_packed_struct
+// CHECK-SAME: ([1 x <8 x i16>] [[ARG_COERCE:%.*]]) #[[ATTR0:[0-9]+]] {
+void named_arg_non_packed_struct(struct non_packed_struct arg) {}
+
+// CHECK-LABEL: define dso_local void @named_arg_packed_struct
+// CHECK-SAME: ([1 x <8 x i16>] [[ARG_COERCE:%.*]]) #[[ATTR0]] {
+void named_arg_packed_struct(struct packed_struct arg) {}
+
+// CHECK-LABEL: define dso_local void @named_arg_packed_member
+// CHECK-SAME: ([1 x <8 x i16>] [[ARG_COERCE:%.*]]) #[[ATTR0]] {
+void named_arg_packed_member(struct packed_member arg) {}
+
+// CHECK-LABEL: define dso_local void @named_arg_aligned_struct_8
+// CHECK-SAME: ([1 x <8 x i16>] [[ARG_COERCE:%.*]]) #[[ATTR0]] {
+void named_arg_aligned_struct_8(struct aligned_struct_8 arg) {}
+
+// CHECK-LABEL: define dso_local void @named_arg_aligned_member_8
+// CHECK-SAME: ([1 x <8 x i16>] [[ARG_COERCE:%.*]]) #[[ATTR0]] {
+void named_arg_aligned_member_8(struct aligned_member_8 arg) {}
+
+// CHECK-LABEL: define dso_local void @named_arg_pragma_packed_struct_8
+// CHECK-SAME: ([1 x <8 x i16>] [[ARG_COERCE:%.*]]) #[[ATTR0]] {
+void named_arg_pragma_packed_struct_8(struct pragma_packed_struct_8 arg) {}
+
+// CHECK-LABEL: define dso_local void @named_arg_pragma_packed_struct_4
+// CHECK-SAME: ([1 x <8 x i16>] [[ARG_COERCE:%.*]]) #[[ATTR0]] {
+void named_arg_pragma_packed_struct_4(struct pragma_packed_struct_4 arg) {}
+
+// Struct passed as a variadic argument
+// CHECK-LABEL: define dso_local void @variadic_non_packed_struct
+// CHECK:   vaarg.end:
+// CHECK:   call void @llvm.memcpy.p0.p0.i64(ptr align 16 [[ON_CALLEE_STACK:.*]], ptr align 16 [[VAARGS_ADDR:.*]], i64 16, i1 false)
+// CHECK-NEXT:ret void
+void variadic_non_packed_struct(int named_arg, ...) {
+  va_list vl;
+  va_start(vl, named_arg);
+  struct non_packed_struct on_callee_stack;
+  on_callee_stack = va_arg(vl, struct non_packed_struct);
+}
+
+// CHECK-LABEL: define dso_local void @variadic_packed_struct
+// CHECK:   vaarg.end:
+// CHECK:   call void @llvm.memcpy.p0.p0.i64(ptr align 1 [[ON_CALLEE_STACK:.*]], ptr align 8 [[VAARGS_ADDR:.*]], i64 16, i1 false)
+// CHECK-NEXT:ret void
+void variadic_packed_struct(int named_arg, ...) {
+  va_list vl;
+  va_start(vl, named_arg);
+  struct packed_struct on_callee_stack;
+  on_callee_stack = va_arg(vl, struct packed_struct);
+}
+
+// CHECK-LABEL: define dso_local void @variadic_packed_member
+// CHECK:   

[PATCH] D152932: [ARM] Adding precommit tests for D146242

[Jirui Wu via Phabricator via cfe-commits]

JiruiWu updated this revision to Diff 531373.
JiruiWu retitled this revision from "[ARM] Adding precommit tests for D146242." 
to "[ARM] Adding precommit tests for D146242".
JiruiWu added a comment.

Updating the commit message.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D152932/new/

https://reviews.llvm.org/D152932

Files:
  clang/test/CodeGen/aarch64-ABI-align-packed.c

Index: clang/test/CodeGen/aarch64-ABI-align-packed.c
===
--- /dev/null
+++ clang/test/CodeGen/aarch64-ABI-align-packed.c
@@ -0,0 +1,240 @@
+// RUN: %clang_cc1 -fsyntax-only -triple aarch64-none-eabi \
+// RUN:   -target-feature +neon \
+// RUN:   -emit-llvm -o - %s | FileCheck %s
+#include 
+#include 
+
+// natural alignment 16, adjusted alignment 16
+// expected alignment of copy on callee stack: 16
+struct non_packed_struct {
+  uint16x8_t M0; // member alignment 16
+};
+
+// natural alignment 1, adjusted alignment 1
+// expected alignment of copy on callee stack: 8
+struct __attribute((packed)) packed_struct {
+  uint16x8_t M0; // member alignment 1, because the field is packed when the struct is packed
+};
+
+// natural alignment 1, adjusted alignment 1
+// expected alignment of copy on callee stack: 8
+struct packed_member {
+  uint16x8_t M0 __attribute((packed)); // member alignment 1
+};
+
+// natural alignment 16, adjusted alignment 16 since __attribute((aligned (n))) sets the minimum alignment
+// expected alignment of copy on callee stack: 16
+struct __attribute((aligned (8))) aligned_struct_8 {
+  uint16x8_t M0; // member alignment 16
+};
+
+// natural alignment 16, adjusted alignment 16
+// expected alignment of copy on callee stack: 16
+struct aligned_member_8 {
+  uint16x8_t M0 __attribute((aligned (8))); // member alignment 16 since __attribute((aligned (n))) sets the minimum alignment
+};
+
+// natural alignment 8, adjusted alignment 8
+// expected alignment of copy on callee stack: 8
+#pragma pack(8)
+struct pragma_packed_struct_8 {
+  uint16x8_t M0; // member alignment 8 because the struct is subject to packed(8)
+};
+
+// natural alignment 4, adjusted alignment 4
+// expected alignment of copy on callee stack: 8
+#pragma pack(4)
+struct pragma_packed_struct_4 {
+  uint16x8_t M0; // member alignment 4 because the struct is subject to packed(4)
+};
+
+// Struct passed as a named argument
+// CHECK-LABEL: define dso_local void @named_arg_non_packed_struct
+// CHECK-SAME: ([1 x <8 x i16>] [[ARG_COERCE:%.*]]) #[[ATTR0:[0-9]+]] {
+void named_arg_non_packed_struct(struct non_packed_struct arg) {}
+
+// CHECK-LABEL: define dso_local void @named_arg_packed_struct
+// CHECK-SAME: ([1 x <8 x i16>] [[ARG_COERCE:%.*]]) #[[ATTR0]] {
+void named_arg_packed_struct(struct packed_struct arg) {}
+
+// CHECK-LABEL: define dso_local void @named_arg_packed_member
+// CHECK-SAME: ([1 x <8 x i16>] [[ARG_COERCE:%.*]]) #[[ATTR0]] {
+void named_arg_packed_member(struct packed_member arg) {}
+
+// CHECK-LABEL: define dso_local void @named_arg_aligned_struct_8
+// CHECK-SAME: ([1 x <8 x i16>] [[ARG_COERCE:%.*]]) #[[ATTR0]] {
+void named_arg_aligned_struct_8(struct aligned_struct_8 arg) {}
+
+// CHECK-LABEL: define dso_local void @named_arg_aligned_member_8
+// CHECK-SAME: ([1 x <8 x i16>] [[ARG_COERCE:%.*]]) #[[ATTR0]] {
+void named_arg_aligned_member_8(struct aligned_member_8 arg) {}
+
+// CHECK-LABEL: define dso_local void @named_arg_pragma_packed_struct_8
+// CHECK-SAME: ([1 x <8 x i16>] [[ARG_COERCE:%.*]]) #[[ATTR0]] {
+void named_arg_pragma_packed_struct_8(struct pragma_packed_struct_8 arg) {}
+
+// CHECK-LABEL: define dso_local void @named_arg_pragma_packed_struct_4
+// CHECK-SAME: ([1 x <8 x i16>] [[ARG_COERCE:%.*]]) #[[ATTR0]] {
+void named_arg_pragma_packed_struct_4(struct pragma_packed_struct_4 arg) {}
+
+// Struct passed as a variadic argument
+// CHECK-LABEL: define dso_local void @variadic_non_packed_struct
+// CHECK:   vaarg.end:
+// CHECK:   call void @llvm.memcpy.p0.p0.i64(ptr align 16 [[ON_CALLEE_STACK:.*]], ptr align 16 [[VAARGS_ADDR:.*]], i64 16, i1 false)
+// CHECK-NEXT:ret void
+void variadic_non_packed_struct(int named_arg, ...) {
+  va_list vl;
+  va_start(vl, named_arg);
+  struct non_packed_struct on_callee_stack;
+  on_callee_stack = va_arg(vl, struct non_packed_struct);
+}
+
+// CHECK-LABEL: define dso_local void @variadic_packed_struct
+// CHECK:   vaarg.end:
+// CHECK:   call void @llvm.memcpy.p0.p0.i64(ptr align 1 [[ON_CALLEE_STACK:.*]], ptr align 8 [[VAARGS_ADDR:.*]], i64 16, i1 false)
+// CHECK-NEXT:ret void
+void variadic_packed_struct(int named_arg, ...) {
+  va_list vl;
+  va_start(vl, named_arg);
+  struct packed_struct on_callee_stack;
+  on_callee_stack = va_arg(vl, struct packed_struct);
+}
+
+// CHECK-LABEL: define dso_local void @variadic_packed_member
+// CHECK:   vaarg.end:
+// CHECK:   call void @llvm.memcpy.p0.p0.i64(ptr align 1 [[ON_CALLEE_STACK:.*]], ptr al

[PATCH] D146242: [ARM] Fixing ABI mismatch for packed structs passed as function arguments

[Jirui Wu via Phabricator via cfe-commits]

JiruiWu marked an inline comment as done.
JiruiWu added inline comments.



Comment at: clang/test/CodeGen/aarch64-ABI-align-packed.c:34
 struct aligned_member_8 {
   uint16x8_t M0 __attribute((aligned (8))); // member alignment 16 since 
__attribute((aligned (n))) sets the minimum alignment
 };

chill wrote:
> Don't you mean "`__attribute__((aligned(n)))` cannot decrease the minimum 
> required alignment" ?
> 
> 
I added this comment to explain that the natural alignment of the struct 
`aligned_member_8` is 16-byte instead of 8-byte. In this test case the 
alignment of  `M0` is 16 bytes, which is above the minimum required alignment 
specified by `__attribute__((aligned(8)))`.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D146242/new/

https://reviews.llvm.org/D146242

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits