Honor __unaligned in codegen for declarations and expressions

[rogfer01]

Summary

See related proposal in http://lists.llvm.org/pipermail/cfe-dev/2017-February/052739.html

This patch honors the unaligned type qualifier (currently available through he keyword __unaligned and -fms-extensions) in CodeGen. In the current form the patch affects declarations and expressions. It does not affect fields of classes.

[ahatanak]

include/clang/AST/ASTContext.h

1910 ↗

(On Diff #89116)

Is it better to call TargetInfo::getCharWidth() instead of assigning a hardcoded number here?

lib/AST/ExprConstant.cpp

5678 ↗

(On Diff #89116)

You can remove an else after return.

[rogfer01]

Updated patch. Use TargetInfo::getCharWidth and remove unnecessary else.

[rogfer01]

Ping? :-)

Kind regards

[aaron.ballman]

include/clang/AST/ASTContext.h

1909 ↗

(On Diff #89172)

This makes me a bit uncomfortable -- wouldn't we rather have the distinction between unqualified and qualified types instead of a mixture of both when calling getTypeInfo()?

[rogfer01]

Avoid altering results of TypeInfo due to the __unaligned qualifier. Instead override the natural alignment to 1 if the type is __unaligned-qualified.

[aaron.ballman]

I think this LGTM, but you should wait for confirmation from one of the other reviewers before committing.

[rogfer01]

Ping? :-)

Kind regards

[rnk]

Looks good! Sorry for the delay, I was out last week.

cfe/trunk/lib/AST/ASTContext.cpp

else if (!BaseT->isIncompleteType()) {
           if (isa<VariableArrayType>(arrayType))
             Align = std::max(Align, Target->getLargeArrayAlign());
           else if (isa<ConstantArrayType>(arrayType) &&
                    MinWidth <= getTypeSize(cast<ConstantArrayType>(arrayType)))
             Align = std::max(Align, Target->getLargeArrayAlign());
         }
       }
       Align = std::max(Align, getPreferredTypeAlign(T.getTypePtr()));
+      if (BaseT.getQualifiers().hasUnaligned())
+        Align = Target->getCharWidth();
       if (const VarDecl *VD = dyn_cast<VarDecl>(D)) {
         if (VD->hasGlobalStorage() && !ForAlignof)
           Align = std::max(Align, getTargetInfo().getMinGlobalAlign());
       }
     }
 
     // Fields can be subject to extra alignment constraints, like if
     // the field is packed, the struct is packed, or the struct has a

cfe/trunk/lib/AST/ExprConstant.cpp

 static CharUnits GetAlignOfType(EvalInfo &Info, QualType T) {
   // C++ [expr.alignof]p3:
   //     When alignof is applied to a reference type, the result is the
   //     alignment of the referenced type.
   if (const ReferenceType *Ref = T->getAs<ReferenceType>())
     T = Ref->getPointeeType();
 
   // __alignof is defined to return the preferred alignment.
+  if (T.getQualifiers().hasUnaligned())
+    return CharUnits::One();
   return Info.Ctx.toCharUnitsFromBits(
     Info.Ctx.getPreferredTypeAlign(T.getTypePtr()));
 }
 
 static CharUnits GetAlignOfExpr(EvalInfo &Info, const Expr *E) {
   E = E->IgnoreParens();
 
   // The kinds of expressions that we have special-case logic here for

cfe/trunk/lib/CodeGen/CodeGenFunction.cpp

if (T->isIncompleteType()) {
     // For C++ class pointees, we don't know whether we're pointing at a
     // base or a complete object, so we generally need to use the
     // non-virtual alignment.
     const CXXRecordDecl *RD;
     if (forPointeeType && (RD = T->getAsCXXRecordDecl())) {
       Alignment = CGM.getClassPointerAlignment(RD);
     } else {
       Alignment = getContext().getTypeAlignInChars(T);
+      if (T.getQualifiers().hasUnaligned())
+        Alignment = CharUnits::One();
     }
 
     // Cap to the global maximum type alignment unless the alignment
     // was somehow explicit on the type.
     if (unsigned MaxAlign = getLangOpts().MaxTypeAlign) {
       if (Alignment.getQuantity() > MaxAlign &&
           !getContext().isAlignmentRequired(T))
         Alignment = CharUnits::fromQuantity(MaxAlign);

cfe/trunk/test/CodeGen/unaligned-decl.c

+// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -fms-extensions -emit-llvm < %s | FileCheck %s
+
+// CHECK: @a1 = global i32 1, align 1
+__unaligned int a1 = 1;
+
+// CHECK: @a2 = global i32 1, align 1
+int __unaligned a2 = 1;
+
+// CHECK: @a3 = {{.*}} align 1
+__unaligned int a3[10];
+
+// CHECK: @a4 = {{.*}} align 1
+int __unaligned a4[10];
+
+// CHECK: @p1 = {{.*}} align 1
+int *__unaligned p1;
+
+// CHECK: @p2 = {{.*}} align 8
+int __unaligned *p2;
+
+// CHECK: @p3 = {{.*}} align 1
+int __unaligned *__unaligned p3;

cfe/trunk/test/CodeGen/unaligned-expr.c

+// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -fms-extensions -emit-llvm < %s | FileCheck %s
+
+// -------------
+// Scalar integer
+// -------------
+__unaligned int x;
+void test1(void) {
+  // CHECK: {{%.*}} = load i32, i32* @x, align 1
+  // CHECK: store i32 {{%.*}}, i32* @x, align 1
+  x++;
+}
+
+void test2(void) {
+  // CHECK: %y = alloca i32, align 1
+  // CHECK: {{%.*}} = load i32, i32* %y, align 1
+  // CHECK: store i32 {{%.*}}, i32* %y, align 1
+  __unaligned int y;
+  y++;
+}
+
+void test2_1(void) {
+  // CHECK: %y = alloca i32, align 1
+  // CHECK: store i32 1, i32* %y, align 1
+  __unaligned int y = 1;
+}
+
+// -------------
+// Global pointer
+// -------------
+int *__unaligned p1;
+void test3(void) {
+
+  // CHECK: {{%.*}} = load i32*, i32** @p1, align 1
+  // CHECK: {{%.*}} = load i32, i32* {{%.*}}, align 4
+  // CHECK: store i32 {{%.*}}, i32* {{%.*}}, align 4
+  (*p1)++;
+}
+
+int __unaligned *p2;
+void test4(void) {
+  // CHECK: {{%.*}} = load i32*, i32** @p2, align 8
+  // CHECK: {{%.*}} = load i32, i32* {{%.*}}, align 1
+  // CHECK: store i32 {{%.*}}, i32* {{%.*}}, align 1
+  (*p2)++;
+}
+
+int __unaligned *__unaligned p3;
+void test5(void) {
+  // CHECK: {{%.*}} = load i32*, i32** @p3, align 1
+  // CHECK: {{%.*}} = load i32, i32* {{%.*}}, align 1
+  // CHECK: store i32 {{%.*}}, i32* {{%.*}}, align 1
+  (*p3)++;
+}
+
+// -------------
+// Local pointer
+// -------------
+void test6(void) {
+  // CHECK: %lp1 = alloca i32*, align 1
+  // CHECK: {{%.*}} = load i32*, i32** %lp1, align 1
+  // CHECK: {{%.*}} = load i32, i32* {{%.*}}, align 4
+  // CHECK: store i32 {{%.*}}, i32* {{%.*}}, align 4
+  int *__unaligned lp1;
+  (*lp1)++;
+}
+
+void test7(void) {
+  // CHECK: %lp2 = alloca i32*, align 8
+  // CHECK: {{%.*}} = load i32*, i32** %lp2, align 8
+  // CHECK: {{%.*}} = load i32, i32* {{%.*}}, align 1
+  // CHECK: store i32 {{%.*}}, i32* {{%.*}}, align 1
+  int __unaligned *lp2;
+  (*lp2)++;
+}
+
+void test8(void) {
+  // CHECK: %lp3 = alloca i32*, align 1
+  // CHECK: {{%.*}} = load i32*, i32** %lp3, align 1
+  // CHECK: {{%.*}} = load i32, i32* {{%.*}}, align 1
+  // CHECK: store i32 {{%.*}}, i32* {{%.*}}, align 1
+  int __unaligned *__unaligned lp3;
+  (*lp3)++;
+}
+
+// -------------
+// Global array
+// -------------
+__unaligned int a[10];
+void test9(void) {
+  // CHECK: {{%.*}} = load i32, i32* getelementptr inbounds ([10 x i32], [10 x i32]* @a, i64 0, i64 3), align 1
+  // CHECK: store i32 {{%.*}}, i32* getelementptr inbounds ([10 x i32], [10 x i32]* @a, i64 0, i64 3), align 1
+  (a[3])++;
+}
+
+// -------------
+// Local array
+// -------------
+void test10(void) {
+  // CHECK: %la = alloca [10 x i32], align 1
+  // CHECK: {{%.*}} = getelementptr inbounds [10 x i32], [10 x i32]* %la, i64 0, i64 3
+  // CHECK: {{%.*}} = load i32, i32* {{%.*}}, align 1
+  // CHECK: store i32 {{%.*}}, i32* {{%.*}}, align 1
+  __unaligned int la[10];
+  (la[3])++;
+}
+
+// --------
+// Typedefs
+// --------
+
+typedef __unaligned int UnalignedInt;
+void test13() {
+  // CHECK: %i = alloca i32, align 1
+  // CHECK: {{%.*}} = load i32, i32* %i, align 1
+  // CHECK: store i32 {{%.*}}, i32* %i, align 1
+  UnalignedInt i;
+  i++;
+}
+
+typedef int Aligned;
+typedef __unaligned Aligned UnalignedInt2;
+void test14() {
+  // CHECK: %i = alloca i32, align 1
+  // CHECK: {{%.*}} = load i32, i32* %i, align 1
+  // CHECK: store i32 {{%.*}}, i32* %i, align 1
+  UnalignedInt2 i;
+  i++;
+}
+
+typedef UnalignedInt UnalignedInt3;
+void test15() {
+  // CHECK: %i = alloca i32, align 1
+  // CHECK: {{%.*}} = load i32, i32* %i, align 1
+  // CHECK: store i32 {{%.*}}, i32* %i, align 1
+  UnalignedInt3 i;
+  i++;
+}
+
+// -------------
+// Decayed types
+// -------------
+void test16(__unaligned int c[10]) {
+  // CHECK: {{%.*}} = alloca i32*, align 8
+  // CHECK: store i32* %c, i32** {{%.*}}, align 8
+  // CHECK: {{%.*}} = load i32*, i32** {{%.*}}, align 8
+  // CHECK: {{%.*}} = getelementptr inbounds i32, i32* {{%.*}}, i64 3
+  // CHECK: {{%.*}} = load i32, i32* {{%.*}}, align 1
+  // CHECK: store i32 {{%.*}}, i32* {{%.*}}, align 1
+  c[3]++;
+}
+
+// -----------
+// __alignof__
+// -----------
+int test17(void) {
+  // CHECK: ret i32 1
+  return __alignof__(__unaligned int);
+}
+
+int test18(void) {
+  // CHECK: ret i32 1
+  __unaligned int a;
+  return __alignof__(a);
+}
+
+int test19(void) {
+  // CHECK: ret i32 1
+  __unaligned int a[10];
+  return __alignof__(a);
+}
+
+// -----------
+// structs
+// -----------
+typedef
+struct S1 {
+    char c;
+    int x;
+} S1;
+
+__unaligned S1 s1;
+void test20(void) {
+    // CHECK: {{%.*}} = load i32, i32* getelementptr inbounds (%struct.S1, %struct.S1* @s1, i32 0, i32 1), align 1
+    // CHECK: store i32 {{%.*}}, i32* getelementptr inbounds (%struct.S1, %struct.S1* @s1, i32 0, i32 1), align 1
+    s1.x++;
+}
+
+void test21(void) {
+  // CHECK: {{%.*}} = alloca %struct.S1, align 1
+  // CHECK: {{%.*}} = getelementptr inbounds %struct.S1, %struct.S1* {{%.*}}, i32 0, i32 1
+  // CHECK: {{%.*}} = load i32, i32* {{%.*}}, align 1
+  // CHECK: store i32 {{%.*}}, i32* {{%.*}}, align 1
+  __unaligned S1 s1_2;
+  s1_2.x++;
+}
+
+typedef
+struct __attribute__((packed)) S2 {
+    char c;
+    int x;
+} S2;
+
+__unaligned S2 s2;
+void test22(void) {
+    // CHECK: {{%.*}} = load i32, i32* getelementptr inbounds (%struct.S2, %struct.S2* @s2, i32 0, i32 1), align 1
+    // CHECK: store i32 {{%.*}}, i32* getelementptr inbounds (%struct.S2, %struct.S2* @s2, i32 0, i32 1), align 1
+    s2.x++;
+}
+
+void test23(void) {
+  // CHECK: {{%.*}} = alloca %struct.S2, align 1
+  // CHECK: {{%.*}} = getelementptr inbounds %struct.S2, %struct.S2* {{%.*}}, i32 0, i32 1
+  // CHECK: {{%.*}} = load i32, i32* {{%.*}}, align 1
+  // CHECK: store i32 {{%.*}}, i32* {{%.*}}, align 1
+  __unaligned S2 s2_2;
+  s2_2.x++;
+}

cfe/trunk/test/CodeGen/unaligned-field.c

+// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -fms-extensions -emit-llvm < %s | FileCheck %s
+// Test that __unaligned does not impact the layout of the fields.
+
+struct A
+{
+    char a;
+    __unaligned int b;
+} a;
+// CHECK: %struct.A = type { i8, i32 }
+
+struct A2
+{
+    int b;
+    char a;
+    __unaligned int c;
+} a2;
+// CHECK: %struct.A2 = type { i32, i8, i32 }

cfe/trunk/test/CodeGenCXX/unaligned.cpp

+// RUN: %clang_cc1 -x c++ -std=c++11 -triple x86_64-unknown-linux-gnu -fms-extensions -emit-llvm < %s | FileCheck %s
+
+int foo() {
+  // CHECK: ret i32 1
+  return alignof(__unaligned int);
+}