[CodeGen] Fix Bug 47499: __unaligned extension inconsistent behaviour with C and C++

[j0le]

Summary

For the language C++ the keyword __unaligned (a Microsoft extension) had no effect on pointers.

The reason, why there was a difference between C and C++ for the keyword __unaligned:
For C, the Method getAsCXXREcordDecl() returns nullptr. That guarantees that hasUnaligned() is called.
If the language is C++, it is not guaranteed, that hasUnaligend() is called and evaluated.

Here are some links:

The Bug: https://bugs.llvm.org/show_bug.cgi?id=47499
Thread on the cfe-dev mailing list: http://lists.llvm.org/pipermail/cfe-dev/2020-September/066783.html
Diff, that introduced the check hasUnaligned() in getNaturalTypeAlignment(): https://reviews.llvm.org/D30166

[rnk]

Thanks, this basically looks correct to me, aside from some formatting details. Do you want me to apply those fixes and land this for you? I see that was done for your last patch, but it's best to ask first.

clang/lib/CodeGen/CodeGenModule.cpp

6153–6155

It's pretty uncommon to see comment blocks between the closing if curly and the next else if. Please move this comment inside the relevant else-if block.

[j0le]

Moved comment into the else-if block.
Changed Summary of the diff, so that it is better suited for a commit message.

[j0le]

In D90630#2368984, @rnk wrote:

Thanks, this basically looks correct to me, aside from some formatting details. Do you want me to apply those fixes and land this for you? I see that was done for your last patch, but it's best to ask first.

Yes, that would be nice, if you could land this for me.

clang/lib/CodeGen/CodeGenModule.cpp

if (BaseInfo)
       *BaseInfo = LValueBaseInfo(AlignmentSource::Type);
     return CharUnits::One();
   }
 
   if (BaseInfo)
     *BaseInfo = LValueBaseInfo(AlignmentSource::Type);
 
   CharUnits Alignment;
+  const CXXRecordDecl *RD;
+  if (T.getQualifiers().hasUnaligned()) {
+    Alignment = CharUnits::One();
+  }
   // 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 && !AlignForArray && (RD = T->getAsCXXRecordDecl())) {
+  else if (forPointeeType && !AlignForArray && (RD = T->getAsCXXRecordDecl())) {
     Alignment = 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);

clang/test/CodeGen/unaligned-struct-copy.c

+// RUN: %clang_cc1 -xc   -O2 -triple thumbv7a-unknown-windows-eabi -fms-extensions -emit-llvm < %s | FileCheck %s
+// RUN: %clang_cc1 -xc++ -O2 -triple thumbv7a-unknown-windows-eabi -fms-extensions -emit-llvm < %s | FileCheck %s
+// RUN: %clang_cc1 -xc   -O2 -triple x86_64-unknown-linux-gnu -fms-extensions -emit-llvm < %s | FileCheck %s
+// RUN: %clang_cc1 -xc++ -O2 -triple x86_64-unknown-linux-gnu -fms-extensions -emit-llvm < %s | FileCheck %s
+
+struct S1 {
+  unsigned long x;
+};
+
+// CHECK: define
+// CHECK-SAME: void
+// CHECK-SAME: test1
+
+void test1(__unaligned struct S1 *out) {
+  // CHECK: store
+  // CHECK-SAME: align 1
+  out->x = 5;
+  // CHECK: ret void
+}
+
+// CHECK: define
+// CHECK-SAME: void
+// CHECK-SAME: test2
+
+void test2(__unaligned struct S1 *out, __unaligned struct S1 *in) {
+  // CHECK: load
+  // CHECK-SAME: align 1
+  // CHECK: store
+  // CHECK-SAME: align 1
+  *out = *in;
+  // CHECK: ret void
+}