[PATCH] D55715: Add AddressSpace mangling to MS mode

[Reid Kleckner via Phabricator via cfe-commits]

rnk added inline comments.


================
Comment at: lib/AST/MicrosoftMangle.cpp:1806-1836
+    Extra.mangleSourceName("AS_");
+    Extra.mangleIntegerLiteral(llvm::APSInt::getUnsigned(TargetAS),
+                               /*IsBoolean*/ false);
+  } else {
+    switch (AS) {
+    default:
+      llvm_unreachable("Not a language specific address space");
----------------
majnemer wrote:
> Don't these need to be _AS_Foobar to avoid collisions with normal code?
I think we're in __clang::, so it's OK? In any case, it's what's done for Itanium, IIUC. It's also not like we have to worry about conflicts with user macros.


================
Comment at: test/CodeGenOpenCL/address-spaces-mangling.cl:7
+// RUN: %clang_cc1 %s -ffake-address-space-map -faddress-space-map-mangling=yes -triple x86_64-windows-pc -emit-llvm -o - | FileCheck -check-prefixes=MS_ASMANG,MS_ASMAN10 %s
+// RUN: %clang_cc1 %s -cl-std=CL2.0 -ffake-address-space-map -faddress-space-map-mangling=yes -triple x86_64-windows-pc -emit-llvm -o - | FileCheck -check-prefixes=MS_ASMANG,MS_ASMAN20 %s
+// RUN: %clang_cc1 %s -ffake-address-space-map -faddress-space-map-mangling=no -triple x86_64-windows-pc -emit-llvm -o - | FileCheck -check-prefixes=MS_NOASMANG,MS_NOASMAN10 %s
----------------
erichkeane wrote:
> erichkeane wrote:
> > rnk wrote:
> > > I would replace `-triple x86_64-windows-pc` here and everywhere with `-triple x86_64-windows-itanium` (or gnu instead of itanium) to test the Itanium mangling on Windows. I don't think `x86_64-windows-pc` parses into a real triple. After all, "pc" is parsed as the vendor, which is supposed to be second.
> > Woops! I didn't mean to check this file in.  It has some other nasty problems that need to be fixed up, so I tested this feature in the mangle-address-space.  I'll still do this above.
> @rnk I tried this, but it changes off of the MicrosoftMangler and switches back to the Itanium mangling.  Is there a better triple for testing microsoft mangling?
Tacking on "-msvc" or "-itanium" or "-gnu" is the more explicit way to the C++ ABI. I guess I was confused, it looks like this file expects Itanium manglings from the -pc triple.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D55715/new/

https://reviews.llvm.org/D55715