From 3a7487f903e2a6be29de39058eee2372e30798d5 Mon Sep 17 00:00:00 2001 From: Xiangling Liao Date: Wed, 30 Sep 2020 10:35:00 -0400 Subject: [PATCH] [FE] Use preferred alignment instead of ABI alignment for complete object when applicable MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit On some targets, preferred alignment is larger than ABI alignment in some cases. For example, on AIX we have special power alignment rules which would cause that. Previously, to support those cases, we added a “PreferredAlignment” field in the `RecordLayout` to store the AIX special alignment values in “PreferredAlignment” as the community suggested. However, that patch alone is not enough. There are places in the Clang where `PreferredAlignment` should have been used instead of ABI-specified alignment. This patch is aimed at fixing those spots. Differential Revision: https://reviews.llvm.org/D86790 --- clang/include/clang/AST/ASTContext.h | 22 ++++++++++++++---- clang/lib/AST/ASTContext.cpp | 11 +++++---- clang/lib/CodeGen/CGExprCXX.cpp | 7 +++--- clang/lib/CodeGen/ItaniumCXXABI.cpp | 4 ++-- clang/lib/CodeGen/TargetInfo.cpp | 4 ---- clang/test/CodeGen/aix-alignment.c | 41 +++++++++++++++++++++++++++++++++ clang/test/CodeGenCXX/aix-alignment.cpp | 40 ++++++++++++++++++++++++++++++++ 7 files changed, 112 insertions(+), 17 deletions(-) create mode 100644 clang/test/CodeGen/aix-alignment.c create mode 100644 clang/test/CodeGenCXX/aix-alignment.cpp diff --git a/clang/include/clang/AST/ASTContext.h b/clang/include/clang/AST/ASTContext.h index de0d119..d30cf04 100644 --- a/clang/include/clang/AST/ASTContext.h +++ b/clang/include/clang/AST/ASTContext.h @@ -2134,16 +2134,25 @@ public: } unsigned getTypeUnadjustedAlign(const Type *T) const; - /// Return the ABI-specified alignment of a type, in bits, or 0 if + /// Return the 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; + /// example, from alignment attributes). The returned alignment is the + /// Preferred alignment if NeedsPreferredAlignment is true, otherwise is the + /// ABI alignment. + unsigned getTypeAlignIfKnown(QualType T, + bool NeedsPreferredAlignment = false) 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; + /// Return the PreferredAlignment of a (complete) type \p T, in + /// characters. + CharUnits getPreferredTypeAlignInChars(QualType T) const { + return toCharUnitsFromBits(getPreferredTypeAlign(T)); + } + /// getTypeUnadjustedAlignInChars - Return the ABI-specified alignment of a type, /// in characters, before alignment adjustments. This method does not work on /// incomplete types. @@ -2166,7 +2175,12 @@ public: /// 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. + /// beneficial for performance or backwards compatibility preserving to + /// overalign a data type. (Note: despite the name, the preferred alignment + /// is ABI-impacting, and not an optimization.) + unsigned getPreferredTypeAlign(QualType T) const { + return getPreferredTypeAlign(T.getTypePtr()); + } unsigned getPreferredTypeAlign(const Type *T) const; /// Return the default alignment for __attribute__((aligned)) on diff --git a/clang/lib/AST/ASTContext.cpp b/clang/lib/AST/ASTContext.cpp index fc7abea..376a0b0 100644 --- a/clang/lib/AST/ASTContext.cpp +++ b/clang/lib/AST/ASTContext.cpp @@ -1836,7 +1836,8 @@ bool ASTContext::isAlignmentRequired(QualType T) const { return isAlignmentRequired(T.getTypePtr()); } -unsigned ASTContext::getTypeAlignIfKnown(QualType T) const { +unsigned ASTContext::getTypeAlignIfKnown(QualType T, + bool NeedsPreferredAlignment) const { // An alignment on a typedef overrides anything else. if (const auto *TT = T->getAs()) if (unsigned Align = TT->getDecl()->getMaxAlignment()) @@ -1845,7 +1846,7 @@ unsigned ASTContext::getTypeAlignIfKnown(QualType T) const { // If we have an (array of) complete type, we're done. T = getBaseElementType(T); if (!T->isIncompleteType()) - return getTypeAlign(T); + return NeedsPreferredAlignment ? getPreferredTypeAlign(T) : getTypeAlign(T); // If we had an array type, its element type might be a typedef // type with an alignment attribute. @@ -2402,7 +2403,8 @@ CharUnits ASTContext::getTypeUnadjustedAlignInChars(const Type *T) const { /// 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 or backwards -/// compatibility preserving to overalign a data type. +/// compatibility preserving to overalign a data type. (Note: despite the name, +/// the preferred alignment is ABI-impacting, and not an optimization.) unsigned ASTContext::getPreferredTypeAlign(const Type *T) const { TypeInfo TI = getTypeInfo(T); unsigned ABIAlign = TI.Align; @@ -2458,7 +2460,8 @@ unsigned ASTContext::getTargetDefaultAlignForAttributeAligned() const { /// to a global variable of the specified type. unsigned ASTContext::getAlignOfGlobalVar(QualType T) const { uint64_t TypeSize = getTypeSize(T.getTypePtr()); - return std::max(getTypeAlign(T), getTargetInfo().getMinGlobalAlign(TypeSize)); + return std::max(getPreferredTypeAlign(T), + getTargetInfo().getMinGlobalAlign(TypeSize)); } /// getAlignOfGlobalVarInChars - Return the alignment in characters that diff --git a/clang/lib/CodeGen/CGExprCXX.cpp b/clang/lib/CodeGen/CGExprCXX.cpp index e33730b..c8b059f 100644 --- a/clang/lib/CodeGen/CGExprCXX.cpp +++ b/clang/lib/CodeGen/CGExprCXX.cpp @@ -1570,7 +1570,7 @@ llvm::Value *CodeGenFunction::EmitCXXNewExpr(const CXXNewExpr *E) { llvm::Value *allocSize = EmitCXXNewAllocSize(*this, E, minElements, numElements, allocSizeWithoutCookie); - CharUnits allocAlign = getContext().getTypeAlignInChars(allocType); + CharUnits allocAlign = getContext().getPreferredTypeAlignInChars(allocType); // Emit the allocation call. If the allocator is a global placement // operator, just "inline" it directly. @@ -1820,8 +1820,9 @@ void CodeGenFunction::EmitDeleteCall(const FunctionDecl *DeleteFD, // Pass the alignment if the delete function has an align_val_t parameter. if (Params.Alignment) { QualType AlignValType = *ParamTypeIt++; - CharUnits DeleteTypeAlign = getContext().toCharUnitsFromBits( - getContext().getTypeAlignIfKnown(DeleteTy)); + CharUnits DeleteTypeAlign = + getContext().toCharUnitsFromBits(getContext().getTypeAlignIfKnown( + DeleteTy, true /* NeedsPreferredAlignment */)); llvm::Value *Align = llvm::ConstantInt::get(ConvertType(AlignValType), DeleteTypeAlign.getQuantity()); DeleteArgs.add(RValue::get(Align), AlignValType); diff --git a/clang/lib/CodeGen/ItaniumCXXABI.cpp b/clang/lib/CodeGen/ItaniumCXXABI.cpp index 69825a0..cfb736c 100644 --- a/clang/lib/CodeGen/ItaniumCXXABI.cpp +++ b/clang/lib/CodeGen/ItaniumCXXABI.cpp @@ -2111,7 +2111,7 @@ CharUnits ItaniumCXXABI::getArrayCookieSizeImpl(QualType elementType) { // The array cookie is a size_t; pad that up to the element alignment. // The cookie is actually right-justified in that space. return std::max(CharUnits::fromQuantity(CGM.SizeSizeInBytes), - CGM.getContext().getTypeAlignInChars(elementType)); + CGM.getContext().getPreferredTypeAlignInChars(elementType)); } Address ItaniumCXXABI::InitializeArrayCookie(CodeGenFunction &CGF, @@ -2128,7 +2128,7 @@ Address ItaniumCXXABI::InitializeArrayCookie(CodeGenFunction &CGF, // The size of the cookie. CharUnits CookieSize = - std::max(SizeSize, Ctx.getTypeAlignInChars(ElementType)); + std::max(SizeSize, Ctx.getPreferredTypeAlignInChars(ElementType)); assert(CookieSize == getArrayCookieSizeImpl(ElementType)); // Compute an offset to the cookie. diff --git a/clang/lib/CodeGen/TargetInfo.cpp b/clang/lib/CodeGen/TargetInfo.cpp index 5c052b7..f39ded3 100644 --- a/clang/lib/CodeGen/TargetInfo.cpp +++ b/clang/lib/CodeGen/TargetInfo.cpp @@ -4512,8 +4512,6 @@ ABIArgInfo AIXABIInfo::classifyReturnType(QualType RetTy) const { if (RetTy->isVoidType()) return ABIArgInfo::getIgnore(); - // TODO: Evaluate if AIX power alignment rule would have an impact on the - // alignment here. if (isAggregateTypeForABI(RetTy)) return getNaturalAlignIndirect(RetTy); @@ -4530,8 +4528,6 @@ ABIArgInfo AIXABIInfo::classifyArgumentType(QualType Ty) const { if (Ty->isVectorType()) llvm::report_fatal_error("vector type is not supported on AIX yet"); - // TODO: Evaluate if AIX power alignment rule would have an impact on the - // alignment here. if (isAggregateTypeForABI(Ty)) { // Records with non-trivial destructors/copy-constructors should not be // passed by value. diff --git a/clang/test/CodeGen/aix-alignment.c b/clang/test/CodeGen/aix-alignment.c new file mode 100644 index 0000000..fdb0bad --- /dev/null +++ b/clang/test/CodeGen/aix-alignment.c @@ -0,0 +1,41 @@ +// REQUIRES: powerpc-registered-target +// RUN: %clang_cc1 -triple powerpc-unknown-aix -emit-llvm -o - %s | \ +// RUN: FileCheck %s --check-prefixes=AIX,AIX32 +// RUN: %clang_cc1 -triple powerpc64-unknown-aix -emit-llvm -o - %s | \ +// RUN: FileCheck %s --check-prefixes=AIX,AIX64 + +// AIX: @d = global double 0.000000e+00, align 8 +double d; + +typedef struct { + double d; + int i; +} StructDouble; + +// AIX: @d1 = global %struct.StructDouble zeroinitializer, align 8 +StructDouble d1; + +// AIX: double @retDouble(double %x) +// AIX: %x.addr = alloca double, align 8 +// AIX: store double %x, double* %x.addr, align 8 +// AIX: load double, double* %x.addr, align 8 +// AIX: ret double %0 +double retDouble(double x) { return x; } + +// AIX32: define void @bar(%struct.StructDouble* noalias sret align 4 %agg.result, %struct.StructDouble* byval(%struct.StructDouble) align 4 %x) +// AIX64: define void @bar(%struct.StructDouble* noalias sret align 4 %agg.result, %struct.StructDouble* byval(%struct.StructDouble) align 8 %x) +// AIX: %0 = bitcast %struct.StructDouble* %agg.result to i8* +// AIX: %1 = bitcast %struct.StructDouble* %x to i8* +// AIX32: call void @llvm.memcpy.p0i8.p0i8.i32(i8* align 4 %0, i8* align 4 %1, i32 16, i1 false) +// AIX64: call void @llvm.memcpy.p0i8.p0i8.i64(i8* align 4 %0, i8* align 8 %1, i64 16, i1 false) +StructDouble bar(StructDouble x) { return x; } + +// AIX: define void @foo(double* %out, double* %in) +// AIX32: %0 = load double*, double** %in.addr, align 4 +// AIX64: %0 = load double*, double** %in.addr, align 8 +// AIX: %1 = load double, double* %0, align 4 +// AIX: %mul = fmul double %1, 2.000000e+00 +// AIX32: %2 = load double*, double** %out.addr, align 4 +// AIX64: %2 = load double*, double** %out.addr, align 8 +// AIX: store double %mul, double* %2, align 4 +void foo(double *out, double *in) { *out = *in * 2; } diff --git a/clang/test/CodeGenCXX/aix-alignment.cpp b/clang/test/CodeGenCXX/aix-alignment.cpp new file mode 100644 index 0000000..4c8330b --- /dev/null +++ b/clang/test/CodeGenCXX/aix-alignment.cpp @@ -0,0 +1,40 @@ +// REQUIRES: powerpc-registered-target +// RUN: %clang_cc1 -triple powerpc-unknown-aix \ +// RUN: -emit-llvm -o - -x c++ %s | \ +// RUN: FileCheck %s --check-prefixes=AIX,AIX32 +// RUN: %clang_cc1 -triple powerpc64-unknown-aix \ +// RUN: -emit-llvm -o - %s -x c++| \ +// RUN: FileCheck %s --check-prefixes=AIX,AIX64 + +struct B { + double d; + ~B() {} +}; + +// AIX32: %call = call noalias nonnull i8* @_Znam(i32 8) +// AIX64: %call = call noalias nonnull i8* @_Znam(i64 8) +B *allocBp() { return new B[0]; } + +// AIX-LABEL: delete.notnull: +// AIX32: %0 = bitcast %struct.B* %call to i8* +// AIX32: %1 = getelementptr inbounds i8, i8* %0, i32 -8 +// AIX32: %2 = getelementptr inbounds i8, i8* %1, i32 4 +// AIX32: %3 = bitcast i8* %2 to i32* +// AIX64: %0 = bitcast %struct.B* %call to i8* +// AIX64: %1 = getelementptr inbounds i8, i8* %0, i64 -8 +// AIX64: %2 = bitcast i8* %1 to i64* +void bar() { delete[] allocBp(); } + +typedef struct D { + double d; + int i; + + ~D(){}; +} D; + +// AIX: define void @_Z3foo1D(%struct.D* noalias sret align 4 %agg.result, %struct.D* %x) +// AIX: %1 = bitcast %struct.D* %agg.result to i8* +// AIX: %2 = bitcast %struct.D* %x to i8* +// AIX32 call void @llvm.memcpy.p0i8.p0i8.i32(i8* align 4 %1, i8* align 4 %2, i32 16, i1 false) +// AIX64: call void @llvm.memcpy.p0i8.p0i8.i64(i8* align 4 %1, i8* align 4 %2, i64 16, i1 false) +D foo(D x) { return x; } -- 2.7.4