[ARM] Conform to AAPCS when passing overaligned composites as arguments

[chill]

Summary

The "Procedure Call Procedure Call Standard for the ARM® Architecture" (https://static.docs.arm.com/ihi0042/f/IHI0042F_aapcs.pdf), specifies that composite types are passed according to their natural alignment:

5.5 Parameter Passing
...
B.5 If the argument is an alignment adjusted type its value is passed as a copy of the actual value. The
copy will have an alignment defined as follows.

• For a Fundamental Data Type, the alignment is the natural alignment of that type, after any

promotions

• For a Composite Type, the alignment of the copy will have 4-byte alignment if its natural alignment is <= 4 and 8-byte alignment if its natural alignment is >= 8

The "natural alignment" is defined as the maximum of the alignment of the top-level components:

4.3 Composite Types
...

• The natural alignment of a composite type is the maximum of each of the member alignments of the 'top-level' members of the composite type i.e. before any alignment adjustment of the entire composite is applied

clang, however, uses the actual alignment of the composite, instead of the natural alignment.

This patch fixes passing of composite types to use the natural alignment. With this patch clang conforms to AAPCS and is compatible with GCC.

[chill]

Ping?

[efriedma]

I'd like to see some tests for __attribute((packed)).

lib/CodeGen/TargetInfo.cpp

5822

Whitespace.

[chill]

In D46013#1084440, @efriedma wrote:

I'd like to see some tests for __attribute((packed)).

Thanks, indeed it does not work correctly on packed structures. Back to the drawing board ...

[chill]

Update:

• similar changes needed for AArch64
• added/updated tests

[efriedma]

I'm not sure Apple will want to mess with their ABI like this... adding some reviewers.

Otherwise LGTM.

lib/CodeGen/TargetInfo.cpp

5815–5817

I'd rather do alignment computations in bytes, rather than bits (can we use getTypeAlignInChars here?)

[chill]

Update:

• fix only APCS, don't touch other ABIs
• misc other

[chill]

In D46013#1118014, @efriedma wrote:

I'm not sure Apple will want to mess with their ABI like this... adding some reviewers.

Fair enough, I'd rather not touch it :)

[rjmccall]

I'm not sure it's appropriate to impose this as an overhead on all targets.

[efriedma]

I'm not sure it's appropriate to impose this as an overhead on all targets.

You mean the overhead of increasing the size of TypeInfo? That's fair.

[rjmccall]

In D46013#1119513, @efriedma wrote:

I'm not sure it's appropriate to impose this as an overhead on all targets.

You mean the overhead of increasing the size of TypeInfo? That's fair.

Yeah. It seems like something that could be pretty easily computed retroactively in the places that need it. It might be okay to have a ASTContext-level cache for it; that cache would just stay empty in practice for most compiler invocations.

[chill]

Update: refactor a bit to not impose size overhead on targets, which don't use natural alignment.

[rjmccall]

Okay, as a code change this seems more reasonable to me. I haven't really thought through the ABI-compatibility issues, though. CC'ing Tim.

[chill]

Ping?

[chill]

Ping?

[t.p.northover]

I'm fine with the ABI changes, but I'm not very convinced by the "NaturalAlignment" name.

Far from being a privileged alignment kind, it seems to take account of a pretty arbitrary set of modifiers. In fact, I can't help wondering if what's really happened is that ARM has decided to document existing GCC practice as the path of least resistance and scrambled for a name. This would be fine in ARM-specific code but could be quite misleading in the generic ASTContext. Personally I'd go for ARMNaturalAlignment and stop pretending it's something anyone else needs to care about (at least until some other ABI comes along that does).

lib/CodeGen/TargetInfo.cpp

5069

I think the max/min logic is more confusing here than the alternative:

Alignment = Alignment < 128 ? 64 : 128;

[chill]

In D46013#1140336, @t.p.northover wrote:

I'm fine with the ABI changes, but I'm not very convinced by the "NaturalAlignment" name.

I'd rather not put target names in API functions. The meaning of that field is pretty target
independent ("alignment of type, before alignment adjustments")
If we use it only in ARM/AArch64 we can put a comment in that sense and
maybe rename it to UnadjustedAlignement or something else more descriptive than NaturalAlignement ?

lib/CodeGen/TargetInfo.cpp

5069

I'll change it.

[t.p.northover]

I'd rather not put target names in API functions. The meaning of that field is pretty target independent ("alignment of type, before alignment adjustments")

Except that rule only applies to aggregates. Scalar types get their alignment adjusted anyway I believe. The definition is ridiculously arbitrary.

If we use it only in ARM/AArch64 we can put a comment in that sense and maybe rename it to UnadjustedAlignement or something else more descriptive than NaturalAlignement?

I could just about live with that (or UnnaturalAlignment, though perhaps best not).

[chill]

Update: use "unadjusted alignment" instead of "natural alignment", rename things accordingly.

[chill]

Ping?

[chill]

Ping?

[chill]

Ping?

[chill]

Ping?

[efriedma]

lib/AST/ASTContext.cpp

2088

This "default" isn't right; there are a lot of non-canonical types which need to be handled here (which getTypeAlign handles, but this doesn't).

Could you just write something like if (const auto *RTy = T->getAs<RecordType>()) [...] else return getTypeAlign(T);?

[chill]

Fixed to properly examine the canonical type when getting it's unadjusted alignment.

[chill]

Thanks for pointing this!

[efriedma]

LGTM

[stefson]

hey there, I've run into problems with stripping static-libs on arm when using llvm/clang-7. Could you imagine this patch being at fault?

strip: armv7a-unknown-linux-gnueabihf-strip --strip-unneeded -R .comment -R .GCC.command.line -R .note.gnu.gold-version
   lib/libbz2.so.1.0.6
   usr/bin/bzip2recover
   bin/bzip2
   usr/lib/libbz2.a
armv7a-unknown-linux-gnueabihf-strip: /var/tmp/portage/app-arch/bzip2-1.0.6-r10/image/usr/lib/stImUpsE/bzlib.o: Failed to find link section for section 11
armv7a-unknown-linux-gnueabihf-strip: /var/tmp/portage/app-arch/bzip2-1.0.6-r10/image/usr/lib/stImUpsE/bzlib.o: Failed to find link section for section 11

[efriedma]

Starting multiple threads on the LLVM mailing lists with the same message is spam. Please don't do that.

If you think you've run into a bug, file a bug report at bugs.llvm.org. For general questions, send an email to llvm-dev.

include/clang/AST/ASTContext.h

mutable llvm::DenseMap<const RecordDecl*, const ASTRecordLayout*>
     ASTRecordLayouts;
   mutable llvm::DenseMap<const ObjCContainerDecl*, const ASTRecordLayout*>
     ObjCLayouts;
 
   /// A cache from types to size and alignment information.
   using TypeInfoMap = llvm::DenseMap<const Type *, struct TypeInfo>;
   mutable TypeInfoMap MemoizedTypeInfo;
 
+  /// A cache from types to unadjusted alignment information. Only ARM and
+  /// AArch64 targets need this information, keeping it separate prevents
+  /// imposing overhead on TypeInfo size.
+  using UnadjustedAlignMap = llvm::DenseMap<const Type *, unsigned>;
+  mutable UnadjustedAlignMap MemoizedUnadjustedAlign;
+
   /// A cache mapping from CXXRecordDecls to key functions.
   llvm::DenseMap<const CXXRecordDecl*, LazyDeclPtr> KeyFunctions;
 
   /// Mapping from ObjCContainers to their ObjCImplementations.
   llvm::DenseMap<ObjCContainerDecl*, ObjCImplDecl*> ObjCImpls;
 
   /// Mapping from ObjCMethod to its duplicate declaration in the same
   /// interface.
public:
   CharUnits getTypeSizeInChars(QualType T) const;
   CharUnits getTypeSizeInChars(const Type *T) const;
 
   /// Return the ABI-specified alignment of a (complete) type \p T, in
   /// bits.
   unsigned getTypeAlign(QualType T) const { return getTypeInfo(T).Align; }
   unsigned getTypeAlign(const Type *T) const { return getTypeInfo(T).Align; }
 
+  /// Return the ABI-specified natural alignment of a (complete) type \p T,
+  /// before alignment adjustments, in bits.
+  ///
+  /// This alignment is curently used only by ARM and AArch64 when passing
+  /// arguments of a composite type.
+  unsigned getTypeUnadjustedAlign(QualType T) const {
+    return getTypeUnadjustedAlign(T.getTypePtr());
+  }
+  unsigned getTypeUnadjustedAlign(const Type *T) const;
+
   /// Return the ABI-specified alignment of a type, in bits, or 0 if
   /// the type is incomplete and we cannot determine the alignment (for
   /// example, from alignment attributes).
   unsigned getTypeAlignIfKnown(QualType T) const;
 
   /// Return the ABI-specified alignment of a (complete) type \p T, in
   /// characters.
   CharUnits getTypeAlignInChars(QualType T) const;
   CharUnits getTypeAlignInChars(const Type *T) const;
 
+  /// getTypeUnadjustedAlignInChars - Return the ABI-specified alignment of a type,
+  /// in characters, before alignment adjustments. This method does not work on
+  /// incomplete types.
+  CharUnits getTypeUnadjustedAlignInChars(QualType T) const;
+  CharUnits getTypeUnadjustedAlignInChars(const Type *T) const;
+
   // getTypeInfoDataSizeInChars - Return the size of a type, in chars. If the
   // type is a record, its data size is returned.
   std::pair<CharUnits, CharUnits> getTypeInfoDataSizeInChars(QualType T) const;
 
   std::pair<CharUnits, CharUnits> getTypeInfoInChars(const Type *T) const;
   std::pair<CharUnits, CharUnits> getTypeInfoInChars(QualType T) const;
 
   /// Determine if the alignment the type has was required using an

include/clang/AST/RecordLayout.h

private:
   CharUnits Size;
 
   /// DataSize - Size of record in characters without tail padding.
   CharUnits DataSize;
 
   // Alignment - Alignment of record in characters.
   CharUnits Alignment;
 
+  // UnadjustedAlignment - Maximum of the alignments of the record members in
+  // characters.
+  CharUnits UnadjustedAlignment;
+
   /// RequiredAlignment - The required alignment of the object.  In the MS-ABI
   /// the __declspec(align()) trumps #pramga pack and must always be obeyed.
   CharUnits RequiredAlignment;
 
   /// FieldOffsets - Array of field offsets in bits.
   ASTVector<uint64_t> FieldOffsets;
 
   /// CXXRecordLayoutInfo - Contains C++ specific layout information.
struct CXXRecordLayoutInfo {
     VBaseOffsetsMapTy VBaseOffsets;
   };
 
   /// CXXInfo - If the record layout is for a C++ record, this will have
   /// C++ specific information about the record.
   CXXRecordLayoutInfo *CXXInfo = nullptr;
 
   ASTRecordLayout(const ASTContext &Ctx, CharUnits size, CharUnits alignment,
+                  CharUnits unadjustedAlignment,
                   CharUnits requiredAlignment, CharUnits datasize,
                   ArrayRef<uint64_t> fieldoffsets);
 
   using BaseOffsetsMapTy = CXXRecordLayoutInfo::BaseOffsetsMapTy;
 
   // Constructor for C++ records.
   ASTRecordLayout(const ASTContext &Ctx,
                   CharUnits size, CharUnits alignment,
+                  CharUnits unadjustedAlignment,
                   CharUnits requiredAlignment,
                   bool hasOwnVFPtr, bool hasExtendableVFPtr,
                   CharUnits vbptroffset,
                   CharUnits datasize,
                   ArrayRef<uint64_t> fieldoffsets,
                   CharUnits nonvirtualsize, CharUnits nonvirtualalignment,
                   CharUnits SizeOfLargestEmptySubobject,
                   const CXXRecordDecl *PrimaryBase,

   
 public:
   ASTRecordLayout(const ASTRecordLayout &) = delete;
   ASTRecordLayout &operator=(const ASTRecordLayout &) = delete;
 
   /// getAlignment - Get the record alignment in characters.
   CharUnits getAlignment() const { return Alignment; }
 
+  /// getUnadjustedAlignment - Get the record alignment in characters, before
+  /// alignment adjustement.
+  CharUnits getUnadjustedAlignment() const { return UnadjustedAlignment; }
+
   /// getSize - Get the record size in characters.
   CharUnits getSize() const { return Size; }
 
   /// getFieldCount - Get the number of fields in the layout.
   unsigned getFieldCount() const { return FieldOffsets.size(); }
 
   /// getFieldOffset - Get the offset of the given field index, in
   /// bits.

lib/AST/ASTContext.cpp

case Type::Pipe:
     Align = Target->getPointerAlign(getTargetAddressSpace(LangAS::opencl_global));
     break;
   }
 
   assert(llvm::isPowerOf2_32(Align) && "Alignment must be power of 2");
   return TypeInfo(Width, Align, AlignIsRequired);
 }
 
+unsigned ASTContext::getTypeUnadjustedAlign(const Type *T) const {
+  UnadjustedAlignMap::iterator I = MemoizedUnadjustedAlign.find(T);
+  if (I != MemoizedUnadjustedAlign.end())
+    return I->second;
+
+  unsigned UnadjustedAlign;
+  if (const auto *RT = T->getAs<RecordType>()) {
+    const RecordDecl *RD = RT->getDecl();
+    const ASTRecordLayout &Layout = getASTRecordLayout(RD);
+    UnadjustedAlign = toBits(Layout.getUnadjustedAlignment());
+  } else if (const auto *ObjCI = T->getAs<ObjCInterfaceType>()) {
+    const ASTRecordLayout &Layout = getASTObjCInterfaceLayout(ObjCI->getDecl());
+    UnadjustedAlign = toBits(Layout.getUnadjustedAlignment());
+  } else {
+    UnadjustedAlign = getTypeAlign(T);
+  }
+
+  MemoizedUnadjustedAlign[T] = UnadjustedAlign;
+  return UnadjustedAlign;
+}
+
 unsigned ASTContext::getOpenMPDefaultSimdAlign(QualType T) const {
   unsigned SimdAlign = getTargetInfo().getSimdDefaultAlign();
   // Target ppc64 with QPX: simd default alignment for pointer to double is 32.
   if ((getTargetInfo().getTriple().getArch() == llvm::Triple::ppc64 ||
        getTargetInfo().getTriple().getArch() == llvm::Triple::ppc64le) &&
       getTargetInfo().getABI() == "elfv1-qpx" &&
       T->isSpecificBuiltinType(BuiltinType::Double))
     SimdAlign = 256;
   return SimdAlign;
 }
 
 /// toCharUnitsFromBits - Convert a size in bits to a size in characters.
 CharUnits ASTContext::toCharUnitsFromBits(int64_t BitSize) const {
   return CharUnits::fromQuantity(BitSize / getCharWidth());
 }
 
 /// toBits - Convert a size in characters to a size in characters.

 /// characters. This method does not work on incomplete types.
 CharUnits ASTContext::getTypeAlignInChars(QualType T) const {
   return toCharUnitsFromBits(getTypeAlign(T));
 }
 CharUnits ASTContext::getTypeAlignInChars(const Type *T) const {
   return toCharUnitsFromBits(getTypeAlign(T));
 }
 
+/// getTypeUnadjustedAlignInChars - Return the ABI-specified alignment of a
+/// type, in characters, before alignment adustments. This method does
+/// not work on incomplete types.
+CharUnits ASTContext::getTypeUnadjustedAlignInChars(QualType T) const {
+  return toCharUnitsFromBits(getTypeUnadjustedAlign(T));
+}
+CharUnits ASTContext::getTypeUnadjustedAlignInChars(const Type *T) const {
+  return toCharUnitsFromBits(getTypeUnadjustedAlign(T));
+}
+
 /// getPreferredTypeAlign - Return the "preferred" alignment of the specified
 /// type for the current target in bits.  This can be different than the ABI
 /// alignment in cases where it is beneficial for performance to overalign
 /// a data type.
 unsigned ASTContext::getPreferredTypeAlign(const Type *T) const {
   TypeInfo TI = getTypeInfo(T);
   unsigned ABIAlign = TI.Align;
 

lib/AST/RecordLayout.cpp

if (CXXInfo) {
     Ctx.Deallocate(CXXInfo);
   }
   this->~ASTRecordLayout();
   Ctx.Deallocate(this);
 }
 
 ASTRecordLayout::ASTRecordLayout(const ASTContext &Ctx, CharUnits size,
                                  CharUnits alignment,
+                                 CharUnits unadjustedAlignment,
                                  CharUnits requiredAlignment,
                                  CharUnits datasize,
                                  ArrayRef<uint64_t> fieldoffsets)
     : Size(size), DataSize(datasize), Alignment(alignment),
+      UnadjustedAlignment(unadjustedAlignment),
       RequiredAlignment(requiredAlignment) {
   FieldOffsets.append(Ctx, fieldoffsets.begin(), fieldoffsets.end());
 }
 
 // Constructor for C++ records.
 ASTRecordLayout::ASTRecordLayout(const ASTContext &Ctx,
                                  CharUnits size, CharUnits alignment,
+                                 CharUnits unadjustedAlignment,
                                  CharUnits requiredAlignment,
                                  bool hasOwnVFPtr, bool hasExtendableVFPtr,
                                  CharUnits vbptroffset,
                                  CharUnits datasize,
                                  ArrayRef<uint64_t> fieldoffsets,
                                  CharUnits nonvirtualsize,
                                  CharUnits nonvirtualalignment,
                                  CharUnits SizeOfLargestEmptySubobject,
                                  const CXXRecordDecl *PrimaryBase,
                                  bool IsPrimaryBaseVirtual,
                                  const CXXRecordDecl *BaseSharingVBPtr,
                                  bool EndsWithZeroSizedObject,
                                  bool LeadsWithZeroSizedBase,
                                  const BaseOffsetsMapTy& BaseOffsets,
                                  const VBaseOffsetsMapTy& VBaseOffsets)
   : Size(size), DataSize(datasize), Alignment(alignment),
+    UnadjustedAlignment(unadjustedAlignment),
     RequiredAlignment(requiredAlignment), CXXInfo(new (Ctx) CXXRecordLayoutInfo)
 {
   FieldOffsets.append(Ctx, fieldoffsets.begin(), fieldoffsets.end());
 
   CXXInfo->PrimaryBase.setPointer(PrimaryBase);
   CXXInfo->PrimaryBase.setInt(IsPrimaryBaseVirtual);
   CXXInfo->NonVirtualSize = nonvirtualsize;
   CXXInfo->NonVirtualAlignment = nonvirtualalignment;

lib/AST/RecordLayoutBuilder.cpp

protected:
   uint64_t Size;
 
   /// Alignment - The current alignment of the record layout.
   CharUnits Alignment;
 
   /// The alignment if attribute packed is not used.
   CharUnits UnpackedAlignment;
 
+  /// \brief The maximum of the alignments of top-level members.
+  CharUnits UnadjustedAlignment;
+
   SmallVector<uint64_t, 16> FieldOffsets;
 
   /// Whether the external AST source has provided a layout for this
   /// record.
   unsigned UseExternalLayout : 1;
 
   /// Whether we need to infer alignment, even when we have an 
   /// externally-provided layout.
protected:
 
   /// Valid if UseExternalLayout is true.
   ExternalLayout External;
 
   ItaniumRecordLayoutBuilder(const ASTContext &Context,
                              EmptySubobjectMap *EmptySubobjects)
       : Context(Context), EmptySubobjects(EmptySubobjects), Size(0),
         Alignment(CharUnits::One()), UnpackedAlignment(CharUnits::One()),
+        UnadjustedAlignment(CharUnits::One()),
         UseExternalLayout(false), InferAlignment(false), Packed(false),
         IsUnion(false), IsMac68kAlign(false), IsMsStruct(false),
         UnfilledBitsInLastUnit(0), LastBitfieldTypeSize(0),
         MaxFieldAlignment(CharUnits::Zero()), DataSize(0),
         NonVirtualSize(CharUnits::Zero()),
         NonVirtualAlignment(CharUnits::One()), PrimaryBase(nullptr),
         PrimaryBaseIsVirtual(false), HasOwnVFPtr(false),
         HasPackedField(false), FirstNearlyEmptyVBase(nullptr) {}
// bits we didn't use.
     // purposes of unfilled bits.
     LastBitfieldTypeSize = 0;
   }
 
   // Update the size.
   setSize(std::max(getSizeInBits(), getDataSizeInBits()));
 
   // Remember max struct/class alignment.
-  UpdateAlignment(Context.toCharUnitsFromBits(FieldAlign), 
+  UnadjustedAlignment =
+      std::max(UnadjustedAlignment, Context.toCharUnitsFromBits(FieldAlign));
+  UpdateAlignment(Context.toCharUnitsFromBits(FieldAlign),
                   Context.toCharUnitsFromBits(UnpackedFieldAlign));
 }
 
 void ItaniumRecordLayoutBuilder::LayoutField(const FieldDecl *D,
                                              bool InsertExtraPadding) {
   if (D->isBitField()) {
     LayoutBitField(D);
     return;
if (IsUnion)
     setDataSize(std::max(getDataSizeInBits(), FieldSizeInBits));
   else
     setDataSize(FieldOffset + FieldSize);
 
   // Update the size.
   setSize(std::max(getSizeInBits(), getDataSizeInBits()));
 
   // Remember max struct/class alignment.
+  UnadjustedAlignment = std::max(UnadjustedAlignment, FieldAlign);
   UpdateAlignment(FieldAlign, UnpackedFieldAlign);
 }
 
 void ItaniumRecordLayoutBuilder::FinishLayout(const NamedDecl *D) {
   // In C++, records cannot be of size 0.
   if (Context.getLangOpts().CPlusPlus && getSizeInBits() == 0) {
     if (const CXXRecordDecl *RD = dyn_cast<CXXRecordDecl>(D)) {
       // Compatibility with gcc requires a class (pod or non-pod)
ASTContext::getASTRecordLayout(const RecordDecl *D) const {
 
   const ASTRecordLayout *NewEntry = nullptr;
 
   if (isMsLayout(*this)) {
     MicrosoftRecordLayoutBuilder Builder(*this);
     if (const auto *RD = dyn_cast<CXXRecordDecl>(D)) {
       Builder.cxxLayout(RD);
       NewEntry = new (*this) ASTRecordLayout(
-          *this, Builder.Size, Builder.Alignment, Builder.RequiredAlignment,
+          *this, Builder.Size, Builder.Alignment, Builder.Alignment,
+          Builder.RequiredAlignment,
           Builder.HasOwnVFPtr, Builder.HasOwnVFPtr || Builder.PrimaryBase,
           Builder.VBPtrOffset, Builder.DataSize, Builder.FieldOffsets,
           Builder.NonVirtualSize, Builder.Alignment, CharUnits::Zero(),
           Builder.PrimaryBase, false, Builder.SharedVBPtrBase,
           Builder.EndsWithZeroSizedObject, Builder.LeadsWithZeroSizedBase,
           Builder.Bases, Builder.VBases);
     } else {
       Builder.layout(D);
       NewEntry = new (*this) ASTRecordLayout(
-          *this, Builder.Size, Builder.Alignment, Builder.RequiredAlignment,
+          *this, Builder.Size, Builder.Alignment, Builder.Alignment,
+          Builder.RequiredAlignment,
           Builder.Size, Builder.FieldOffsets);
     }
   } else {
     if (const auto *RD = dyn_cast<CXXRecordDecl>(D)) {
       EmptySubobjectMap EmptySubobjects(*this, RD);
       ItaniumRecordLayoutBuilder Builder(*this, &EmptySubobjects);
       Builder.Layout(RD);
 
       // In certain situations, we are allowed to lay out objects in the
       // tail-padding of base classes.  This is ABI-dependent.
       // FIXME: this should be stored in the record layout.
       bool skipTailPadding =
           mustSkipTailPadding(getTargetInfo().getCXXABI(), RD);
 
       // FIXME: This should be done in FinalizeLayout.
       CharUnits DataSize =
           skipTailPadding ? Builder.getSize() : Builder.getDataSize();
       CharUnits NonVirtualSize =
           skipTailPadding ? DataSize : Builder.NonVirtualSize;
       NewEntry = new (*this) ASTRecordLayout(
-          *this, Builder.getSize(), Builder.Alignment,
+          *this, Builder.getSize(), Builder.Alignment, Builder.UnadjustedAlignment,
           /*RequiredAlignment : used by MS-ABI)*/
           Builder.Alignment, Builder.HasOwnVFPtr, RD->isDynamicClass(),
           CharUnits::fromQuantity(-1), DataSize, Builder.FieldOffsets,
           NonVirtualSize, Builder.NonVirtualAlignment,
           EmptySubobjects.SizeOfLargestEmptySubobject, Builder.PrimaryBase,
           Builder.PrimaryBaseIsVirtual, nullptr, false, false, Builder.Bases,
           Builder.VBases);
     } else {
       ItaniumRecordLayoutBuilder Builder(*this, /*EmptySubobjects=*/nullptr);
       Builder.Layout(D);
 
       NewEntry = new (*this) ASTRecordLayout(
-          *this, Builder.getSize(), Builder.Alignment,
+          *this, Builder.getSize(), Builder.Alignment, Builder.UnadjustedAlignment,
           /*RequiredAlignment : used by MS-ABI)*/
           Builder.Alignment, Builder.getSize(), Builder.FieldOffsets);
     }
   }
 
   ASTRecordLayouts[D] = NewEntry;
 
   if (getLangOpts().DumpRecordLayouts) {
ASTContext::getObjCLayout(const ObjCInterfaceDecl *D,
   }
 
   ItaniumRecordLayoutBuilder Builder(*this, /*EmptySubobjects=*/nullptr);
   Builder.Layout(D);
 
   const ASTRecordLayout *NewEntry =
     new (*this) ASTRecordLayout(*this, Builder.getSize(),
                                 Builder.Alignment,
+                                Builder.UnadjustedAlignment,
                                 /*RequiredAlignment : used by MS-ABI)*/
                                 Builder.Alignment,
                                 Builder.getDataSize(),
                                 Builder.FieldOffsets);
 
   ObjCLayouts[Key] = NewEntry;
 
   return *NewEntry;

lib/CodeGen/TargetInfo.cpp

ABIArgInfo AArch64ABIInfo::classifyArgumentType(QualType Ty) const {
 
   // Aggregates <= 16 bytes are passed directly in registers or on the stack.
   if (Size <= 128) {
     // On RenderScript, coerce Aggregates <= 16 bytes to an integer array of
     // same size and alignment.
     if (getTarget().isRenderScriptTarget()) {
       return coerceToIntArray(Ty, getContext(), getVMContext());
     }
-    unsigned Alignment = getContext().getTypeAlign(Ty);
+    unsigned Alignment;
+    if (Kind == AArch64ABIInfo::AAPCS) {
+      Alignment = getContext().getTypeUnadjustedAlign(Ty);
+      Alignment = Alignment < 128 ? 64 : 128;
+    } else {
+      Alignment = getContext().getTypeAlign(Ty);
+    }
     Size = llvm::alignTo(Size, 64); // round up to multiple of 8 bytes
 
     // We use a pair of i64 for 16-byte aggregate with 8-byte alignment.
     // For aggregates with 16-byte alignment, we use i128.
     if (Alignment < 128 && Size == 128) {
       llvm::Type *BaseTy = llvm::Type::getInt64Ty(getVMContext());
       return ABIArgInfo::getDirect(llvm::ArrayType::get(BaseTy, Size / 64));
     }
return ABIArgInfo::getIndirect(
         CharUnits::fromQuantity(getContext().getTypeAlign(Ty) / 8), false);
   }
 
   // Support byval for ARM.
   // The ABI alignment for APCS is 4-byte and for AAPCS at least 4-byte and at
   // most 8-byte. We realign the indirect argument if type alignment is bigger
   // than ABI alignment.
   uint64_t ABIAlign = 4;
-  uint64_t TyAlign = getContext().getTypeAlign(Ty) / 8;
+  uint64_t TyAlign;
   if (getABIKind() == ARMABIInfo::AAPCS_VFP ||
-       getABIKind() == ARMABIInfo::AAPCS)
+      getABIKind() == ARMABIInfo::AAPCS) {
+    TyAlign = getContext().getTypeUnadjustedAlignInChars(Ty).getQuantity();
     ABIAlign = std::min(std::max(TyAlign, (uint64_t)4), (uint64_t)8);
-
+  } else {
+    TyAlign = getContext().getTypeAlignInChars(Ty).getQuantity();
+  }
   if (getContext().getTypeSizeInChars(Ty) > CharUnits::fromQuantity(64)) {
     assert(getABIKind() != ARMABIInfo::AAPCS16_VFP && "unexpected byval");
     return ABIArgInfo::getIndirect(CharUnits::fromQuantity(ABIAlign),
                                    /*ByVal=*/true,
                                    /*Realign=*/TyAlign > ABIAlign);
   }
 
   // On RenderScript, coerce Aggregates <= 64 bytes to an integer array of
   // same size and alignment.
   if (getTarget().isRenderScriptTarget()) {
     return coerceToIntArray(Ty, getContext(), getVMContext());
   }
 
   // Otherwise, pass by coercing to a structure of the appropriate size.
   llvm::Type* ElemTy;
   unsigned SizeRegs;
   // FIXME: Try to match the types of the arguments more accurately where
   // we can.
-  if (getContext().getTypeAlign(Ty) <= 32) {
+  if (TyAlign <= 4) {
     ElemTy = llvm::Type::getInt32Ty(getVMContext());
     SizeRegs = (getContext().getTypeSize(Ty) + 31) / 32;
   } else {
     ElemTy = llvm::Type::getInt64Ty(getVMContext());
     SizeRegs = (getContext().getTypeSize(Ty) + 63) / 64;
   }
 
   return ABIArgInfo::getDirect(llvm::ArrayType::get(ElemTy, SizeRegs));

test/CodeGen/aapcs-align.cc

+// REQUIRES: arm-registered-target
+// RUN: %clang_cc1 -triple arm-none-none-eabi \
+// RUN:   -O2 \
+// RUN:   -target-cpu cortex-a8 \
+// RUN:   -emit-llvm -o - %s | FileCheck %s
+
+extern "C" {
+
+// Base case, nothing interesting.
+struct S {
+  int x, y;
+};
+
+void f0(int, S);
+void f0m(int, int, int, int, int, S);
+void g0() {
+  S s = {6, 7};
+  f0(1, s);
+  f0m(1, 2, 3, 4, 5, s);
+}
+// CHECK: define void @g0
+// CHECK: call void @f0(i32 1, [2 x i32] [i32 6, i32 7]
+// CHECK: call void @f0m(i32 1, i32 2, i32 3, i32 4, i32 5, [2 x i32] [i32 6, i32 7]
+// CHECK: declare void @f0(i32, [2 x i32])
+// CHECK: declare void @f0m(i32, i32, i32, i32, i32, [2 x i32])
+
+// Aligned struct, passed according to its natural alignment.
+struct __attribute__((aligned(8))) S8 {
+  int x, y;
+} s8;
+
+void f1(int, S8);
+void f1m(int, int, int, int, int, S8);
+void g1() {
+  S8 s = {6, 7};
+  f1(1, s);
+  f1m(1, 2, 3, 4, 5, s);
+}
+// CHECK: define void @g1
+// CHECK: call void @f1(i32 1, [2 x i32] [i32 6, i32 7]
+// CHECK: call void @f1m(i32 1, i32 2, i32 3, i32 4, i32 5, [2 x i32] [i32 6, i32 7]
+// CHECK: declare void @f1(i32, [2 x i32])
+// CHECK: declare void @f1m(i32, i32, i32, i32, i32, [2 x i32])
+
+// Aligned struct, passed according to its natural alignment.
+struct alignas(16) S16 {
+  int x, y;
+};
+
+extern "C" void f2(int, S16);
+extern "C" void f2m(int, int, int, int, int, S16);
+
+void g2() {
+  S16 s = {6, 7};
+  f2(1, s);
+  f2m(1, 2, 3, 4, 5, s);
+}
+// CHECK: define void @g2
+// CHECK: call void @f2(i32 1, [4 x i32] [i32 6, i32 7
+// CHECK: call void @f2m(i32 1, i32 2, i32 3, i32 4, i32 5, [4 x i32] [i32 6, i32 7
+// CHECK: declare void @f2(i32, [4 x i32])
+// CHECK: declare void @f2m(i32, i32, i32, i32, i32, [4 x i32])
+
+// Increased natural alignment.
+struct SF8 {
+  int x __attribute__((aligned(8)));
+  int y;
+};
+
+void f3(int, SF8);
+void f3m(int, int, int, int, int, SF8);
+void g3() {
+  SF8 s = {6, 7};
+  f3(1, s);
+  f3m(1, 2, 3, 4, 5, s);
+}
+// CHECK: define void @g3
+// CHECK: call void @f3(i32 1, [1 x i64] [i64 30064771078]
+// CHECK: call void @f3m(i32 1, i32 2, i32 3, i32 4, i32 5, [1 x i64] [i64 30064771078]
+// CHECK: declare void @f3(i32, [1 x i64])
+// CHECK: declare void @f3m(i32, i32, i32, i32, i32, [1 x i64])
+
+// Increased natural alignment, capped to 8 though.
+struct SF16 {
+  int x;
+  int y alignas(16);
+  int z, a, b, c, d, e, f, g, h, i, j, k;
+};
+
+void f4(int, SF16);
+void f4m(int, int, int, int, int, SF16);
+void g4() {
+  SF16 s = {6, 7};
+  f4(1, s);
+  f4m(1, 2, 3, 4, 5, s);
+}
+// CHECK: define void @g4
+// CHECK: call void @f4(i32 1, %struct.SF16* byval nonnull align 8
+// CHECK: call void @f4m(i32 1, i32 2, i32 3, i32 4, i32 5, %struct.SF16* byval nonnull align 8
+// CHECK: declare void @f4(i32, %struct.SF16* byval align 8)
+// CHECK: declare void @f4m(i32, i32, i32, i32, i32, %struct.SF16* byval align 8)
+
+// Packed structure.
+struct  __attribute__((packed)) P {
+  int x;
+  long long u;
+};
+
+void f5(int, P);
+void f5m(int, int, int, int, int, P);
+void g5() {
+  P s = {6, 7};
+  f5(1, s);
+  f5m(1, 2, 3, 4, 5, s);
+}
+// CHECK: define void @g5
+// CHECK: call void @f5(i32 1, [3 x i32] [i32 6, i32 7, i32 0])
+// CHECK: call void @f5m(i32 1, i32 2, i32 3, i32 4, i32 5, [3 x i32] [i32 6, i32 7, i32 0])
+// CHECK: declare void @f5(i32, [3 x i32])
+// CHECK: declare void @f5m(i32, i32, i32, i32, i32, [3 x i32])
+
+
+// Packed and aligned, alignement causes padding at the end.
+struct  __attribute__((packed, aligned(8))) P8 {
+  int x;
+  long long u;
+};
+
+void f6(int, P8);
+void f6m(int, int, int, int, int, P8);
+void g6() {
+  P8 s = {6, 7};
+  f6(1, s);
+  f6m(1, 2, 3, 4, 5, s);
+}
+// CHECK: define void @g6
+// CHECK: call void @f6(i32 1, [4 x i32] [i32 6, i32 7, i32 0, i32 0])
+// CHECK: call void @f6m(i32 1, i32 2, i32 3, i32 4, i32 5, [4 x i32] [i32 6, i32 7, i32 0, i32 0])
+// CHECK: declare void @f6(i32, [4 x i32])
+// CHECK: declare void @f6m(i32, i32, i32, i32, i32, [4 x i32])
+}

test/CodeGen/aapcs64-align.cc

+// REQUIRES: arm-registered-target
+// RUN: %clang_cc1 -triple aarch64-none-none-eabi \
+// RUN:   -O2 \
+// RUN:   -emit-llvm -o - %s | FileCheck %s
+
+extern "C" {
+
+// Base case, nothing interesting.
+struct S {
+  long x, y;
+};
+
+void f0(long, S);
+void f0m(long, long, long, long, long, S);
+void g0() {
+  S s = {6, 7};
+  f0(1, s);
+  f0m(1, 2, 3, 4, 5, s);
+}
+// CHECK: define void @g0
+// CHECK: call void @f0(i64 1, [2 x i64] [i64 6, i64 7]
+// CHECK: call void @f0m{{.*}}[2 x i64] [i64 6, i64 7]
+// CHECK: declare void @f0(i64, [2 x i64])
+// CHECK: declare void @f0m(i64, i64, i64, i64, i64, [2 x i64])
+
+// Aligned struct, passed according to its natural alignment.
+struct __attribute__((aligned(16))) S16 {
+  long x, y;
+} s16;
+
+void f1(long, S16);
+void f1m(long, long, long, long, long, S16);
+void g1() {
+  S16 s = {6, 7};
+  f1(1, s);
+  f1m(1, 2, 3, 4, 5, s);
+}
+// CHECK: define void @g1
+// CHECK: call void @f1{{.*}}[2 x i64] [i64 6, i64 7]
+// CHECK: call void @f1m{{.*}}[2 x i64] [i64 6, i64 7]
+// CHECK: declare void @f1(i64, [2 x i64])
+// CHECK: declare void @f1m(i64, i64, i64, i64, i64, [2 x i64])
+
+// Increased natural alignment.
+struct SF16 {
+  long x __attribute__((aligned(16)));
+  long y;
+};
+
+void f3(long, SF16);
+void f3m(long, long, long, long, long, SF16);
+void g3() {
+  SF16 s = {6, 7};
+  f3(1, s);
+  f3m(1, 2, 3, 4, 5, s);
+}
+// CHECK: define void @g3
+// CHECK: call void @f3(i64 1, i128 129127208515966861318)
+// CHECK: call void @f3m(i64 1, i64 2, i64 3, i64 4, i64 5, i128 129127208515966861318)
+// CHECK: declare void @f3(i64, i128)
+// CHECK: declare void @f3m(i64, i64, i64, i64, i64, i128)
+
+
+// Packed structure.
+struct  __attribute__((packed)) P {
+  int x;
+  long u;
+};
+
+void f4(int, P);
+void f4m(int, int, int, int, int, P);
+void g4() {
+  P s = {6, 7};
+  f4(1, s);
+  f4m(1, 2, 3, 4, 5, s);
+}
+// CHECK: define void @g4()
+// CHECK: call void @f4(i32 1, [2 x i64] [i64 30064771078, i64 0])
+// CHECK: void @f4m(i32 1, i32 2, i32 3, i32 4, i32 5, [2 x i64] [i64 30064771078, i64 0])
+// CHECK: declare void @f4(i32, [2 x i64])
+// CHECK: declare void @f4m(i32, i32, i32, i32, i32, [2 x i64])
+
+
+// Packed structure, overaligned, same as above.
+struct  __attribute__((packed, aligned(16))) P16 {
+  int x;
+  long y;
+};
+
+void f5(int, P16);
+void f5m(int, int, int, int, int, P16);
+  void g5() {
+    P16 s = {6, 7};
+    f5(1, s);
+    f5m(1, 2, 3, 4, 5, s);
+}
+// CHECK: define void @g5()
+// CHECK: call void @f5(i32 1, [2 x i64] [i64 30064771078, i64 0])
+// CHECK: void @f5m(i32 1, i32 2, i32 3, i32 4, i32 5, [2 x i64] [i64 30064771078, i64 0])
+// CHECK: declare void @f5(i32, [2 x i64])
+// CHECK: declare void @f5m(i32, i32, i32, i32, i32, [2 x i64])
+
+}

test/CodeGen/arm-arguments.c

 }
 // APCS-GNU-LABEL: define <4 x float> @f35(i32 %i, %struct.s35* byval align 4, %struct.s35* byval align 4)
 // APCS-GNU: %[[a:.*]] = alloca %struct.s35, align 16
 // APCS-GNU: %[[b:.*]] = bitcast %struct.s35* %[[a]] to i8*
 // APCS-GNU: %[[c:.*]] = bitcast %struct.s35* %0 to i8*
 // APCS-GNU: call void @llvm.memcpy.p0i8.p0i8.i32(i8* align {{[0-9]+}} %[[b]], i8* align {{[0-9]+}} %[[c]]
 // APCS-GNU: %[[d:.*]] = bitcast %struct.s35* %[[a]] to <4 x float>*
 // APCS-GNU: load <4 x float>, <4 x float>* %[[d]], align 16
-// AAPCS-LABEL: define arm_aapcscc <4 x float> @f35(i32 %i, %struct.s35* byval align 8, %struct.s35* byval align 8)
-// AAPCS: %[[a:.*]] = alloca %struct.s35, align 16
-// AAPCS: %[[b:.*]] = bitcast %struct.s35* %[[a]] to i8*
-// AAPCS: %[[c:.*]] = bitcast %struct.s35* %0 to i8*
-// AAPCS: call void @llvm.memcpy.p0i8.p0i8.i32(i8* align 16 %[[b]], i8* align 8 %[[c]]
-// AAPCS: %[[d:.*]] = bitcast %struct.s35* %[[a]] to <4 x float>*
-// AAPCS: load <4 x float>, <4 x float>* %[[d]], align 16
+
+// AAPCS-LABEL: define arm_aapcscc <4 x float> @f35(i32 %i, %struct.s35* byval align 4 %s1, %struct.s35* byval align 4 %s2)
+// AAPCS: %[[a_addr:.*]] = alloca <4 x float>, align 16
+// AAPCS: %[[b_addr:.*]] = alloca <4 x float>, align 16
+// AAPCS: %[[p1:.*]] = bitcast %struct.s35* %s1 to <4 x float>*
+// AAPCS: %[[a:.*]] = load <4 x float>, <4 x float>* %[[p1]], align 4
+// AAPCS: %[[p2:.*]] = bitcast %struct.s35* %s2 to <4 x float>*
+// AAPCS: %[[b:.*]] = load <4 x float>, <4 x float>* %[[p2]], align 4
+// AAPCS: store <4 x float> %[[a]], <4 x float>* %[[a_addr]], align 16
+// AAPCS: store <4 x float> %[[b]], <4 x float>* %[[b_addr]], align 16