Skip to content

Commit 3a7487f

Browse files
committed
[FE] Use preferred alignment instead of ABI alignment for complete object when applicable
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
1 parent 2ef7302 commit 3a7487f

File tree

7 files changed

+112
-17
lines changed

7 files changed

+112
-17
lines changed

clang/include/clang/AST/ASTContext.h

Lines changed: 18 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2134,16 +2134,25 @@ class ASTContext : public RefCountedBase<ASTContext> {
21342134
}
21352135
unsigned getTypeUnadjustedAlign(const Type *T) const;
21362136

2137-
/// Return the ABI-specified alignment of a type, in bits, or 0 if
2137+
/// Return the alignment of a type, in bits, or 0 if
21382138
/// the type is incomplete and we cannot determine the alignment (for
2139-
/// example, from alignment attributes).
2140-
unsigned getTypeAlignIfKnown(QualType T) const;
2139+
/// example, from alignment attributes). The returned alignment is the
2140+
/// Preferred alignment if NeedsPreferredAlignment is true, otherwise is the
2141+
/// ABI alignment.
2142+
unsigned getTypeAlignIfKnown(QualType T,
2143+
bool NeedsPreferredAlignment = false) const;
21412144

21422145
/// Return the ABI-specified alignment of a (complete) type \p T, in
21432146
/// characters.
21442147
CharUnits getTypeAlignInChars(QualType T) const;
21452148
CharUnits getTypeAlignInChars(const Type *T) const;
21462149

2150+
/// Return the PreferredAlignment of a (complete) type \p T, in
2151+
/// characters.
2152+
CharUnits getPreferredTypeAlignInChars(QualType T) const {
2153+
return toCharUnitsFromBits(getPreferredTypeAlign(T));
2154+
}
2155+
21472156
/// getTypeUnadjustedAlignInChars - Return the ABI-specified alignment of a type,
21482157
/// in characters, before alignment adjustments. This method does not work on
21492158
/// incomplete types.
@@ -2166,7 +2175,12 @@ class ASTContext : public RefCountedBase<ASTContext> {
21662175
/// the current target, in bits.
21672176
///
21682177
/// This can be different than the ABI alignment in cases where it is
2169-
/// beneficial for performance to overalign a data type.
2178+
/// beneficial for performance or backwards compatibility preserving to
2179+
/// overalign a data type. (Note: despite the name, the preferred alignment
2180+
/// is ABI-impacting, and not an optimization.)
2181+
unsigned getPreferredTypeAlign(QualType T) const {
2182+
return getPreferredTypeAlign(T.getTypePtr());
2183+
}
21702184
unsigned getPreferredTypeAlign(const Type *T) const;
21712185

21722186
/// Return the default alignment for __attribute__((aligned)) on

clang/lib/AST/ASTContext.cpp

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1836,7 +1836,8 @@ bool ASTContext::isAlignmentRequired(QualType T) const {
18361836
return isAlignmentRequired(T.getTypePtr());
18371837
}
18381838

1839-
unsigned ASTContext::getTypeAlignIfKnown(QualType T) const {
1839+
unsigned ASTContext::getTypeAlignIfKnown(QualType T,
1840+
bool NeedsPreferredAlignment) const {
18401841
// An alignment on a typedef overrides anything else.
18411842
if (const auto *TT = T->getAs<TypedefType>())
18421843
if (unsigned Align = TT->getDecl()->getMaxAlignment())
@@ -1845,7 +1846,7 @@ unsigned ASTContext::getTypeAlignIfKnown(QualType T) const {
18451846
// If we have an (array of) complete type, we're done.
18461847
T = getBaseElementType(T);
18471848
if (!T->isIncompleteType())
1848-
return getTypeAlign(T);
1849+
return NeedsPreferredAlignment ? getPreferredTypeAlign(T) : getTypeAlign(T);
18491850

18501851
// If we had an array type, its element type might be a typedef
18511852
// type with an alignment attribute.
@@ -2402,7 +2403,8 @@ CharUnits ASTContext::getTypeUnadjustedAlignInChars(const Type *T) const {
24022403
/// getPreferredTypeAlign - Return the "preferred" alignment of the specified
24032404
/// type for the current target in bits. This can be different than the ABI
24042405
/// alignment in cases where it is beneficial for performance or backwards
2405-
/// compatibility preserving to overalign a data type.
2406+
/// compatibility preserving to overalign a data type. (Note: despite the name,
2407+
/// the preferred alignment is ABI-impacting, and not an optimization.)
24062408
unsigned ASTContext::getPreferredTypeAlign(const Type *T) const {
24072409
TypeInfo TI = getTypeInfo(T);
24082410
unsigned ABIAlign = TI.Align;
@@ -2458,7 +2460,8 @@ unsigned ASTContext::getTargetDefaultAlignForAttributeAligned() const {
24582460
/// to a global variable of the specified type.
24592461
unsigned ASTContext::getAlignOfGlobalVar(QualType T) const {
24602462
uint64_t TypeSize = getTypeSize(T.getTypePtr());
2461-
return std::max(getTypeAlign(T), getTargetInfo().getMinGlobalAlign(TypeSize));
2463+
return std::max(getPreferredTypeAlign(T),
2464+
getTargetInfo().getMinGlobalAlign(TypeSize));
24622465
}
24632466

24642467
/// getAlignOfGlobalVarInChars - Return the alignment in characters that

clang/lib/CodeGen/CGExprCXX.cpp

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1570,7 +1570,7 @@ llvm::Value *CodeGenFunction::EmitCXXNewExpr(const CXXNewExpr *E) {
15701570
llvm::Value *allocSize =
15711571
EmitCXXNewAllocSize(*this, E, minElements, numElements,
15721572
allocSizeWithoutCookie);
1573-
CharUnits allocAlign = getContext().getTypeAlignInChars(allocType);
1573+
CharUnits allocAlign = getContext().getPreferredTypeAlignInChars(allocType);
15741574

15751575
// Emit the allocation call. If the allocator is a global placement
15761576
// operator, just "inline" it directly.
@@ -1820,8 +1820,9 @@ void CodeGenFunction::EmitDeleteCall(const FunctionDecl *DeleteFD,
18201820
// Pass the alignment if the delete function has an align_val_t parameter.
18211821
if (Params.Alignment) {
18221822
QualType AlignValType = *ParamTypeIt++;
1823-
CharUnits DeleteTypeAlign = getContext().toCharUnitsFromBits(
1824-
getContext().getTypeAlignIfKnown(DeleteTy));
1823+
CharUnits DeleteTypeAlign =
1824+
getContext().toCharUnitsFromBits(getContext().getTypeAlignIfKnown(
1825+
DeleteTy, true /* NeedsPreferredAlignment */));
18251826
llvm::Value *Align = llvm::ConstantInt::get(ConvertType(AlignValType),
18261827
DeleteTypeAlign.getQuantity());
18271828
DeleteArgs.add(RValue::get(Align), AlignValType);

clang/lib/CodeGen/ItaniumCXXABI.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2111,7 +2111,7 @@ CharUnits ItaniumCXXABI::getArrayCookieSizeImpl(QualType elementType) {
21112111
// The array cookie is a size_t; pad that up to the element alignment.
21122112
// The cookie is actually right-justified in that space.
21132113
return std::max(CharUnits::fromQuantity(CGM.SizeSizeInBytes),
2114-
CGM.getContext().getTypeAlignInChars(elementType));
2114+
CGM.getContext().getPreferredTypeAlignInChars(elementType));
21152115
}
21162116

21172117
Address ItaniumCXXABI::InitializeArrayCookie(CodeGenFunction &CGF,
@@ -2128,7 +2128,7 @@ Address ItaniumCXXABI::InitializeArrayCookie(CodeGenFunction &CGF,
21282128

21292129
// The size of the cookie.
21302130
CharUnits CookieSize =
2131-
std::max(SizeSize, Ctx.getTypeAlignInChars(ElementType));
2131+
std::max(SizeSize, Ctx.getPreferredTypeAlignInChars(ElementType));
21322132
assert(CookieSize == getArrayCookieSizeImpl(ElementType));
21332133

21342134
// Compute an offset to the cookie.

clang/lib/CodeGen/TargetInfo.cpp

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -4512,8 +4512,6 @@ ABIArgInfo AIXABIInfo::classifyReturnType(QualType RetTy) const {
45124512
if (RetTy->isVoidType())
45134513
return ABIArgInfo::getIgnore();
45144514

4515-
// TODO: Evaluate if AIX power alignment rule would have an impact on the
4516-
// alignment here.
45174515
if (isAggregateTypeForABI(RetTy))
45184516
return getNaturalAlignIndirect(RetTy);
45194517

@@ -4530,8 +4528,6 @@ ABIArgInfo AIXABIInfo::classifyArgumentType(QualType Ty) const {
45304528
if (Ty->isVectorType())
45314529
llvm::report_fatal_error("vector type is not supported on AIX yet");
45324530

4533-
// TODO: Evaluate if AIX power alignment rule would have an impact on the
4534-
// alignment here.
45354531
if (isAggregateTypeForABI(Ty)) {
45364532
// Records with non-trivial destructors/copy-constructors should not be
45374533
// passed by value.

clang/test/CodeGen/aix-alignment.c

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,41 @@
1+
// REQUIRES: powerpc-registered-target
2+
// RUN: %clang_cc1 -triple powerpc-unknown-aix -emit-llvm -o - %s | \
3+
// RUN: FileCheck %s --check-prefixes=AIX,AIX32
4+
// RUN: %clang_cc1 -triple powerpc64-unknown-aix -emit-llvm -o - %s | \
5+
// RUN: FileCheck %s --check-prefixes=AIX,AIX64
6+
7+
// AIX: @d = global double 0.000000e+00, align 8
8+
double d;
9+
10+
typedef struct {
11+
double d;
12+
int i;
13+
} StructDouble;
14+
15+
// AIX: @d1 = global %struct.StructDouble zeroinitializer, align 8
16+
StructDouble d1;
17+
18+
// AIX: double @retDouble(double %x)
19+
// AIX: %x.addr = alloca double, align 8
20+
// AIX: store double %x, double* %x.addr, align 8
21+
// AIX: load double, double* %x.addr, align 8
22+
// AIX: ret double %0
23+
double retDouble(double x) { return x; }
24+
25+
// AIX32: define void @bar(%struct.StructDouble* noalias sret align 4 %agg.result, %struct.StructDouble* byval(%struct.StructDouble) align 4 %x)
26+
// AIX64: define void @bar(%struct.StructDouble* noalias sret align 4 %agg.result, %struct.StructDouble* byval(%struct.StructDouble) align 8 %x)
27+
// AIX: %0 = bitcast %struct.StructDouble* %agg.result to i8*
28+
// AIX: %1 = bitcast %struct.StructDouble* %x to i8*
29+
// AIX32: call void @llvm.memcpy.p0i8.p0i8.i32(i8* align 4 %0, i8* align 4 %1, i32 16, i1 false)
30+
// AIX64: call void @llvm.memcpy.p0i8.p0i8.i64(i8* align 4 %0, i8* align 8 %1, i64 16, i1 false)
31+
StructDouble bar(StructDouble x) { return x; }
32+
33+
// AIX: define void @foo(double* %out, double* %in)
34+
// AIX32: %0 = load double*, double** %in.addr, align 4
35+
// AIX64: %0 = load double*, double** %in.addr, align 8
36+
// AIX: %1 = load double, double* %0, align 4
37+
// AIX: %mul = fmul double %1, 2.000000e+00
38+
// AIX32: %2 = load double*, double** %out.addr, align 4
39+
// AIX64: %2 = load double*, double** %out.addr, align 8
40+
// AIX: store double %mul, double* %2, align 4
41+
void foo(double *out, double *in) { *out = *in * 2; }
Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,40 @@
1+
// REQUIRES: powerpc-registered-target
2+
// RUN: %clang_cc1 -triple powerpc-unknown-aix \
3+
// RUN: -emit-llvm -o - -x c++ %s | \
4+
// RUN: FileCheck %s --check-prefixes=AIX,AIX32
5+
// RUN: %clang_cc1 -triple powerpc64-unknown-aix \
6+
// RUN: -emit-llvm -o - %s -x c++| \
7+
// RUN: FileCheck %s --check-prefixes=AIX,AIX64
8+
9+
struct B {
10+
double d;
11+
~B() {}
12+
};
13+
14+
// AIX32: %call = call noalias nonnull i8* @_Znam(i32 8)
15+
// AIX64: %call = call noalias nonnull i8* @_Znam(i64 8)
16+
B *allocBp() { return new B[0]; }
17+
18+
// AIX-LABEL: delete.notnull:
19+
// AIX32: %0 = bitcast %struct.B* %call to i8*
20+
// AIX32: %1 = getelementptr inbounds i8, i8* %0, i32 -8
21+
// AIX32: %2 = getelementptr inbounds i8, i8* %1, i32 4
22+
// AIX32: %3 = bitcast i8* %2 to i32*
23+
// AIX64: %0 = bitcast %struct.B* %call to i8*
24+
// AIX64: %1 = getelementptr inbounds i8, i8* %0, i64 -8
25+
// AIX64: %2 = bitcast i8* %1 to i64*
26+
void bar() { delete[] allocBp(); }
27+
28+
typedef struct D {
29+
double d;
30+
int i;
31+
32+
~D(){};
33+
} D;
34+
35+
// AIX: define void @_Z3foo1D(%struct.D* noalias sret align 4 %agg.result, %struct.D* %x)
36+
// AIX: %1 = bitcast %struct.D* %agg.result to i8*
37+
// AIX: %2 = bitcast %struct.D* %x to i8*
38+
// AIX32 call void @llvm.memcpy.p0i8.p0i8.i32(i8* align 4 %1, i8* align 4 %2, i32 16, i1 false)
39+
// AIX64: call void @llvm.memcpy.p0i8.p0i8.i64(i8* align 4 %1, i8* align 4 %2, i64 16, i1 false)
40+
D foo(D x) { return x; }

0 commit comments

Comments
 (0)