[clang] 3a7487f - [FE] Use preferred alignment instead of ABI alignment for complete object when applicable

Xiangling Liao via cfe-commits cfe-commits at lists.llvm.org
Wed Sep 30 07:49:09 PDT 2020


Author: Xiangling Liao
Date: 2020-09-30T10:48:28-04:00
New Revision: 3a7487f903e2a6be29de39058eee2372e30798d5

URL: https://github.com/llvm/llvm-project/commit/3a7487f903e2a6be29de39058eee2372e30798d5
DIFF: https://github.com/llvm/llvm-project/commit/3a7487f903e2a6be29de39058eee2372e30798d5.diff

LOG: [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

Added: 
    clang/test/CodeGen/aix-alignment.c
    clang/test/CodeGenCXX/aix-alignment.cpp

Modified: 
    clang/include/clang/AST/ASTContext.h
    clang/lib/AST/ASTContext.cpp
    clang/lib/CodeGen/CGExprCXX.cpp
    clang/lib/CodeGen/ItaniumCXXABI.cpp
    clang/lib/CodeGen/TargetInfo.cpp

Removed: 
    


################################################################################
diff  --git a/clang/include/clang/AST/ASTContext.h b/clang/include/clang/AST/ASTContext.h
index de0d1198b6d4..d30cf045f104 100644
--- a/clang/include/clang/AST/ASTContext.h
+++ b/clang/include/clang/AST/ASTContext.h
@@ -2134,16 +2134,25 @@ class ASTContext : public RefCountedBase<ASTContext> {
   }
   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 @@ class ASTContext : public RefCountedBase<ASTContext> {
   /// the current target, in bits.
   ///
   /// This can be 
diff erent 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 fc7abeaae9b1..376a0b044010 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<TypedefType>())
     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 
diff erent 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 e33730b9ae90..c8b059fd7db0 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 69825a036a1e..cfb736ce0ff1 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 5c052b7fb84b..f39ded3dc31c 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 000000000000..fdb0bad197bb
--- /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 000000000000..4c8330b42e92
--- /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; }


        


More information about the cfe-commits mailing list