[FIX] 26194 - LLVM crash in CXXNameMangler::mangleType

[• ichesnokov]

Summary

Please review and submit.

[• ichesnokov]

The patch crash LLVM crash in CXXNameMangler.mangleType.patch

[asl]

test/CodeGenOpenCL/generic_type_cl20.cl

1 ↗

(On Diff #45961)

Are you sure you need this cmdline? Please reduce down to minimum subset.

5 ↗

(On Diff #45961)

You need to test the mangling. Please implement it.

[• ichesnokov]

test/CodeGenOpenCL/generic_type_cl20.cl

1 ↗

(On Diff #45961)

Bug author wrote: "...and it's still crashing in the same place. Please try invoking with the precise arguments i wrote in "To reproduce".. and on the attached CL file, not pocl codebase - i've mentioned pocl for reference, but pocl doesn't currently work with 3.8 (yet)..."

[asl]

test/CodeGenOpenCL/generic_type_cl20.cl

1 ↗

(On Diff #45961)

The author might wrote whatever he / she wants. But this does not mean that you should not reduce the testcase.

[pxli168]

lib/AST/ItaniumMangle.cpp

1799

As you said in the bug commit, you can try to fix the windows part too.

Try it in MicrosoftMangle.cpp

test/CodeGenOpenCL/generic_type_cl20.cl

1 ↗

(On Diff #45961)

I think you can merge this test in test/CodeGenOpenCL/address-spaces-mangling.c. You need not to reproduce the bug case here.

You can refer to other test relate to opencl 2.0 and use something like

#ifdef CL20
void test(int4 x)
#endif

[• ichesnokov]

test/CodeGenOpenCL/generic_mangling.cl

5 ↗

(On Diff #46251)

I found no way to register the operation inside CL function, thus I post simplier diff, without mangling verification.

[• ichesnokov]

lib/AST/ItaniumMangle.cpp

1799

The Windows mangler is pretty different and doesn't have this issue.

[• ichesnokov]

Local variables are always generic if not specified explicitly.
And I found no method to supply address space of local variable, only of whole function.

[• ichesnokov]

Please review the patch and commit.
BugZilla link: https://llvm.org/bugs/show_bug.cgi?id=26194

[• ichesnokov]

Please review and commit.

[• ichesnokov]

lib/AST/ItaniumMangle.cpp

1799

I'll try to fix on the Windows.

[• ichesnokov]

BugZilla page: https://llvm.org/bugs/show_bug.cgi?id=26194

[• ichesnokov]

Added generic variable support and checking address space to Microsoft mangler.
Test case updated.

[pxli168]

test/CodeGenOpenCL/generic_mangling.cl

1 ↗

(On Diff #46532)

Is this enough for the new added MicrosoftMangle?

tools/driver/driver.cpp

367 ↗

(On Diff #46532)

I think this dubug use shuold not be in the patch.

[• ichesnokov]

Please support discussion about MicrosoftMangle+CL at
https://llvm.org/bugs/show_bug.cgi?id=26194

tools/driver/driver.cpp

367 ↗

(On Diff #46532)

Yes, this is enough to work with MicrosoftMangler.
However I am in doubt about this implentation.
Please forward discussion at https://llvm.org/bugs/show_bug.cgi?id=26194

[• ichesnokov]

This diff adds mangling check test cases for Itanium and Microsoft manglers

[• ichesnokov]

I noticed that MicrosoftMangler already has many extension functions. I.e. this MicrosoftMangler.cpp change is Ok.
Please review and accept/commit.

[• ichesnokov]

Ping.

[• ichesnokov]

Ping.

[• ichesnokov]

Please review the last version of patch.

[Anastasia]

lib/AST/MicrosoftMangle.cpp

1386

I was just wondering since this code appears to be a repetition from ItaniumMangler.cpp, could we just factor it out to a common function.

I think Mangler.cpp could be a good place for having its definition then next to ObjC specific functions (see for example mangleObjCMethodName()). We would just have to update comments in this file accordingly to reflect that it now contains OpenCL and CUDA functionality as well.

test/CodeGenOpenCL/generic-mangling-itanium.cl

1

Could we extend test/CodeGenOpenCL/address-spaces-mangling.cl instead of adding another file?

5

Did you mean to test generic not global?

test/CodeGenOpenCL/generic-mangling-microsoft.cl

1

I would very much like to have test/CodeGenOpenCL/address-spaces-mangling.cl extended here too. You can just add a new RUN line along with a new -check-prefix option.

lib/AST/ItaniumMangle.cpp

       case LangAS::opencl_global:   ASString = "CLglobal";   break;
       case LangAS::opencl_local:    ASString = "CLlocal";    break;
       case LangAS::opencl_constant: ASString = "CLconstant"; break;
+      case LangAS::opencl_generic:  ASString = "CLgeneric";  break;
       //  <CUDA-addrspace> ::= "CU" [ "device" | "constant" | "shared" ]
       case LangAS::cuda_device:     ASString = "CUdevice";   break;
       case LangAS::cuda_constant:   ASString = "CUconstant"; break;

lib/AST/MicrosoftMangle.cpp

     }
   }
 
+  if (Quals.hasAddressSpace()) {
+    // Address space extension:
+    //
+    //   <type> ::= U <target-addrspace>
+    //   <type> ::= U <OpenCL-addrspace>
+    //   <type> ::= U <CUDA-addrspace>
+
+    SmallString<64> ASString;
+    unsigned AS = Quals.getAddressSpace();
+
+    if (Context.getASTContext().addressSpaceMapManglingFor(AS)) {
+      //  <target-addrspace> ::= "AS" <address-space-number>
+      unsigned TargetAS = Context.getASTContext().getTargetAddressSpace(AS);
+      ASString = "AS" + llvm::utostr(TargetAS);
+    } else {
+      switch (AS) {
+      default: llvm_unreachable("Not a language specific address space");
+        //  <OpenCL-addrspace> ::= "CL" [ "global" | "local" | "constant" ]
+      case LangAS::opencl_global:   ASString = "CLglobal";   break;
+      case LangAS::opencl_local:    ASString = "CLlocal";    break;
+      case LangAS::opencl_constant: ASString = "CLconstant"; break;
+      case LangAS::opencl_generic:  ASString = "CLgeneric";  break;
+        //  <CUDA-addrspace> ::= "CU" [ "device" | "constant" | "shared" ]
+      case LangAS::cuda_device:     ASString = "CUdevice";   break;
+      case LangAS::cuda_constant:   ASString = "CUconstant"; break;
+      case LangAS::cuda_shared:     ASString = "CUshared";   break;
+      }
+    }
+    Out << 'U' << ASString.size() << ASString;
+  }
+
   // FIXME: For now, just drop all extension qualifiers on the floor.
 }
 

test/CodeGenOpenCL/generic-mangling-itanium.cl

+// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -emit-llvm -x cl -cl-std=CL2.0 -o - %s | FileCheck %s
+
+// CHECK: _Z30atomic_compare_exchange_strongPVU8CLglobalU7_AtomiciPU9CLgenericii
+bool __attribute__((__overloadable__)) atomic_compare_exchange_strong(
+  volatile  __global atomic_int *object,
+  int  *expected,
+  int  desired)
+{
+  return atomic_compare_exchange_strong_explicit(
+        object, expected, desired, 1, 2);
+}

test/CodeGenOpenCL/generic-mangling-microsoft.cl

+// RUN: %clang_cc1 -triple i686-pc-windows-msvc -emit-llvm -x cl -cl-std=CL2.0 -o - %s | FileCheck %s
+
+// CHECK: define zeroext i1 @"\01?atomic_compare_exchange_strong@@$$J0YA_NPCU8CLglobalU?$_Atomic@H@__clang@@PAU9CLgenericHH@Z"(i32* %object, i32* %expected, i32 %desired)
+bool __attribute__((__overloadable__)) atomic_compare_exchange_strong(
+  volatile  __global atomic_int *object,
+  int  *expected,
+  int  desired)
+{
+  return atomic_compare_exchange_strong_explicit(
+        object, expected, desired, 1, 2);
+}