Mangle __unaligned in Itanium ABI

[rogfer01]

Summary

__unaligned is not currently mangled in any way in the Itanium ABI. This causes failures when using -fms-extensions and C++ in targets using Itanium ABI.

As suggested by @rsmith the simplest thing to do here is actually mangle the qualifier as a vendor extension.

As an alternative, we could continue not mangling this qualifier. But if I read the code right, in order to get substitutions correctly, we would have to rebuild some types and stripe the __unaligned qualifier prior any mangling and substitution.

This patch also removes the change done in D31976 and updates its test to the new reality.

[rsmith]

lib/AST/ItaniumMangle.cpp

2329–2333 ↗

(On Diff #99727)

I don't think this is the right place/way to handle this: given

void f(struct X __unaligned *p, struct X *q) {}

it looks like we'll mangle as _Z1fP1XP1X with this patch, which seems wrong: this should presumably instead be _Z1fP1XS0_.

But regardless, I think the right thing to do is to invent a mangling for __unaligned, since we support overloading on it; the most appropriate mangling would be U11__unaligned, per http://itanium-cxx-abi.github.io/cxx-abi/abi.html#mangling-type. (You'll need to extend mangleQualifiers to emit this and hasMangledSubstitutionQualifiers to ignore it.)

[rogfer01]

lib/AST/ItaniumMangle.cpp

2329–2333 ↗

(On Diff #99727)

Oh. I see. Thanks for the review.

I'll update this patch and then I'll post another one for the mangling of unaligned.

[rogfer01]

Abandon the old approach which cannot possibly work and just opt to mangle __unaligned.

[aaron.ballman]

test/CodeGenCXX/pr33080.cpp

19 ↗

(On Diff #99876)

Can we also get a test like struct A * __unaligned * __unaligned *? Also, perhaps a test case involving templates would be good.

[rogfer01]

Thanks @aaron.ballman for the review.

I have extended the test with your suggestions.

[aaron.ballman]

Can you run clang-format over both the test files? Aside from that, looks good to me, but you should wait for @rsmith or @majnemer to sign off before committing.

[rogfer01]

Fix formatting of tests.

Thanks for the review @aaron.ballman !

[rnk]

This was filed as https://bugs.llvm.org/show_bug.cgi?id=33178

Isn't there a space in the mangling for vendor extensions? Can we use that here?

[rsmith]

In D33398#769152, @rnk wrote:

Isn't there a space in the mangling for vendor extensions? Can we use that here?

We are using that here: that's what mangleVendorQualifier does.

lib/AST/ItaniumMangle.cpp

2210 ↗

(On Diff #100868)

Too much indentation here. Also, the ABI requires the "unordered" vendor qualifiers to be emitted in reverse alphabetical order, so this should be emitted after __weak and __strong but before __autoreleasing.

[rogfer01]

Changelog:

• Fix formatting.
• Emit __unaligned in the right order (as defined by the Itanium ABI) when there are Objective-C++ ARC vendor qualifiers.
• New test for __unaligned and ARC's __weak, __strong and __autorelease

[rogfer01]

lib/AST/ItaniumMangle.cpp

2210 ↗

(On Diff #100868)

I think you meant after __weak but before __strong and __autoreleasing? Maybe I'm misinterpreting something here.

The current patch emits __weak, then __unaligned and then the remaining ARC ones.

[rsmith]

Looks great, thanks!

lib/AST/ItaniumMangle.cpp

2184 ↗

(On Diff #100974)

first __weak -> __weak first

2210 ↗

(On Diff #100868)

The alphabet is hard, apparently :) Yes, thanks!

cfe/trunk/lib/AST/ItaniumMangle.cpp

void CXXNameMangler::mangleNestedName(const NamedDecl *ND,
 
   Out << 'N';
   if (const CXXMethodDecl *Method = dyn_cast<CXXMethodDecl>(ND)) {
     Qualifiers MethodQuals =
         Qualifiers::fromCVRUMask(Method->getTypeQualifiers());
     // We do not consider restrict a distinguishing attribute for overloading
     // purposes so we must not mangle it.
     MethodQuals.removeRestrict();
-    // __unaligned is not currently mangled in any way, so remove it.
-    MethodQuals.removeUnaligned();
     mangleQualifiers(MethodQuals);
     mangleRefQualifier(Method->getRefQualifier());
   }
   
   // Check if we have a template.
   const TemplateArgumentList *TemplateArgs = nullptr;
   if (const TemplateDecl *TD = isTemplate(ND, TemplateArgs)) {
     mangleTemplatePrefix(TD, NoFunction);
CXXNameMangler::mangleOperatorName(OverloadedOperatorKind OO, unsigned Arity) {
 
   case OO_None:
   case NUM_OVERLOADED_OPERATORS:
     llvm_unreachable("Not an overloaded operator");
   }
 }
 
 void CXXNameMangler::mangleQualifiers(Qualifiers Quals) {
-  // Vendor qualifiers come first.
+  // Vendor qualifiers come first and if they are order-insensitive they must
+  // be emitted in reversed alphabetical order, see Itanium ABI 5.1.5.
 
   // Address space qualifiers start with an ordinary letter.
   if (Quals.hasAddressSpace()) {
     // Address space extension:
     //
     //   <type> ::= U <target-addrspace>
     //   <type> ::= U <OpenCL-addrspace>
     //   <type> ::= U <CUDA-addrspace>
if (Context.getASTContext().addressSpaceMapManglingFor(AS)) {
       case LangAS::cuda_constant:   ASString = "CUconstant"; break;
       case LangAS::cuda_shared:     ASString = "CUshared";   break;
       }
     }
     mangleVendorQualifier(ASString);
   }
 
   // The ARC ownership qualifiers start with underscores.
-  switch (Quals.getObjCLifetime()) {
   // Objective-C ARC Extension:
   //
   //   <type> ::= U "__strong"
   //   <type> ::= U "__weak"
   //   <type> ::= U "__autoreleasing"
+  //
+  // Note: we emit __weak first to preserve the order as
+  // required by the Itanium ABI.
+  if (Quals.getObjCLifetime() == Qualifiers::OCL_Weak)
+    mangleVendorQualifier("__weak");
+
+  // __unaligned (from -fms-extensions)
+  if (Quals.hasUnaligned())
+    mangleVendorQualifier("__unaligned");
+
+  // Remaining ARC ownership qualifiers.
+  switch (Quals.getObjCLifetime()) {
   case Qualifiers::OCL_None:
     break;
     
   case Qualifiers::OCL_Weak:
-    mangleVendorQualifier("__weak");
+    // Do nothing as we already handled this case above.
     break;
     
   case Qualifiers::OCL_Strong:
     mangleVendorQualifier("__strong");
     break;
     
   case Qualifiers::OCL_Autoreleasing:
     mangleVendorQualifier("__autoreleasing");
bool CXXNameMangler::mangleSubstitution(const NamedDecl *ND) {
   ND = cast<NamedDecl>(ND->getCanonicalDecl());
   return mangleSubstitution(reinterpret_cast<uintptr_t>(ND));
 }
 
 /// Determine whether the given type has any qualifiers that are relevant for
 /// substitutions.
 static bool hasMangledSubstitutionQualifiers(QualType T) {
   Qualifiers Qs = T.getQualifiers();
-  return Qs.getCVRQualifiers() || Qs.hasAddressSpace();
+  return Qs.getCVRQualifiers() || Qs.hasAddressSpace() || Qs.hasUnaligned();
 }
 
 bool CXXNameMangler::mangleSubstitution(QualType T) {
   if (!hasMangledSubstitutionQualifiers(T)) {
     if (const RecordType *RT = T->getAs<RecordType>())
       return mangleSubstitution(RT->getDecl());
   }
 

cfe/trunk/test/CodeGenCXX/pr33080.cpp

+// RUN: %clang_cc1 -triple %itanium_abi_triple -fms-extensions -emit-llvm -o- %s | FileCheck %s
+
+void fa(__unaligned struct A *) {}
+// CHECK: define void @_Z2faPU11__unaligned1A(
+
+void ga(struct A *, struct A *) {}
+// CHECK: define void @_Z2gaP1AS0_(
+
+void gb(__unaligned struct A *, struct A *) {}
+// CHECK: define void @_Z2gbPU11__unaligned1APS_(
+
+void gc(struct A *, __unaligned struct A *) {}
+// CHECK: define void @_Z2gcP1APU11__unalignedS_(
+
+void gd(__unaligned struct A *, __unaligned struct A *) {}
+// CHECK: define void @_Z2gdPU11__unaligned1AS1_(
+
+void hb(__unaligned struct A *, __unaligned const struct A *) {}
+// CHECK: define void @_Z2hbPU11__unaligned1APU11__unalignedKS_(
+
+void ja(__unaligned struct A *, __unaligned struct A *__unaligned *, __unaligned struct A *__unaligned *__unaligned *) {}
+// CHECK: define void @_Z2jaPU11__unaligned1APU11__unalignedS1_PU11__unalignedS3_(
+
+void jb(__unaligned struct A *, __unaligned struct A **, __unaligned struct A *__unaligned *__unaligned *) {}
+// CHECK: @_Z2jbPU11__unaligned1APS1_PU11__unalignedPU11__unalignedS1_(
+
+template <typename T, typename Q>
+void ta(T &, Q *) {}
+
+void ia(__unaligned struct A &a) {
+  ta(a, &a);
+}
+// CHECK: @_Z2taIU11__unaligned1AS1_EvRT_PT0_(

cfe/trunk/test/CodeGenCXX/unaligned-member-qualifier.cpp

+// RUN: %clang_cc1 -triple %itanium_abi_triple -fms-extensions -emit-llvm %s -o- | FileCheck %s
+
+struct A {
+  void foo() __unaligned;
+  void foo() const __unaligned;
+  void foo() volatile __unaligned;
+  void foo() const volatile __unaligned;
+};
+
+void A::foo() __unaligned {}
+// CHECK: define void @_ZNU11__unaligned1A3fooEv(
+
+void A::foo() const __unaligned {}
+// CHECK: define void @_ZNU11__unalignedK1A3fooEv(
+
+void A::foo() volatile __unaligned {}
+// CHECK: define void @_ZNU11__unalignedV1A3fooEv(
+
+void A::foo() const volatile __unaligned {}
+// CHECK: define void @_ZNU11__unalignedVK1A3fooEv(

cfe/trunk/test/CodeGenObjCXX/arc-mangle.mm

 // RUN: %clang_cc1 -fobjc-arc -fobjc-runtime-has-weak -triple %itanium_abi_triple -emit-llvm -fblocks -o - %s | FileCheck %s
+// RUN: %clang_cc1 -DTEST_UNALIGNED -fms-extensions -fobjc-arc -fobjc-runtime-has-weak -triple %itanium_abi_triple -emit-llvm -fblocks -o - %s | FileCheck %s --check-prefix=UNALIGNED
 
 // CHECK-LABEL: define {{.*}}void @_Z1fPU8__strongP11objc_object(i8**)
 void f(__strong id *) {}
 // CHECK-LABEL: define {{.*}}void @_Z1fPU6__weakP11objc_object(i8**)
 void f(__weak id *) {}
 // CHECK-LABEL: define {{.*}}void @_Z1fPU15__autoreleasingP11objc_object(i8**)
 void f(__autoreleasing id *) {}
 // CHECK-LABEL: define {{.*}}void @_Z1fPP11objc_object(i8**)

 // CHECK-LABEL: define {{.*}}void @_Z1fU13block_pointerFvU11ns_consumedP11objc_objectE
 void f(void (^)(__attribute__((ns_consumed)) id)) {}
 
 template<unsigned N> struct unsigned_c { };
 
 // CHECK-LABEL: define weak_odr {{.*}}void @_Z1gIKvEvP10unsigned_cIXplszv1U8__bridgecvPT_v1U8__bridgecvP11objc_objectcvS3_Li0ELi1EEE
 template<typename T>void g(unsigned_c<sizeof((__bridge T*)(__bridge id)(T*)0) + 1>*) {}
 template void g<const void>(unsigned_c<sizeof(id) + 1> *);
+
+#if TEST_UNALIGNED
+// UNALIGNED-LABEL: define {{.*}}void @_Z1gPU6__weakU11__unalignedP11objc_object(i8**)
+void g(__weak __unaligned id *) {}
+// UNALIGNED-LABEL: define {{.*}}void @_Z1gPU11__unalignedU8__strongP11objc_object(i8**)
+void g(__strong __unaligned id *) {}
+// UNALIGNED-LABEL: define {{.*}}void @_Z1gPU11__unalignedU15__autoreleasingP11objc_object(i8**)
+void g(__autoreleasing __unaligned id *) {}
+#endif // TEST_UNALIGNED