[NVPTX] Improve mangling of illegal PTX characters in global names.

[tra]

Summary

• replace illegal characters in all classes of global symbols
• deal with illegal symbols potentially introduced when setName() has to append unique suffix (it is currently separated by '.')

[jlebar]

I think D17738 is similar? We really need a solution for this, but unfortunately this one may not fly.

[tra]

In D19126#401521, @jlebar wrote:

I think D17738 is similar? We really need a solution for this, but unfortunately this one may not fly.

Hmm. I was unaware of that patch.
I'm mostly interested in mangling functions. I'll remove the change that deals with '.' introduced by setName() and let D17738 handle it.

lib/Target/NVPTX/NVPTXAssignValidGlobalNames.cpp

 /// \brief NVPTXAssignValidGlobalNames
 class NVPTXAssignValidGlobalNames : public ModulePass {
 public:
   static char ID;
   NVPTXAssignValidGlobalNames() : ModulePass(ID) {}
 
   bool runOnModule(Module &M) override;
 
+private:
   /// \brief Clean up the name to remove symbols invalid in PTX.
   std::string cleanUpName(StringRef Name);
+
+  /// \brief Helper function that calls cleanUpName() if \p GV has
+  /// local linkage.
+  void cleanUpLinkLocalValue(GlobalValue &GV) {
+    // We are only allowed to rename local symbols.
+    if (GV.hasLocalLinkage()) {
+      // setName doesn't do extra work if the name does not change.
+      // Note: this does not create collisions - if setName is asked
+      // to set the name to something that already exists, it adds a
+      // proper postfix to avoid collisions. Alas, this may add more
+      // unwanted symbols, so we need to repeat the process until the
+      // name is clean.
+      while (GV.getName().find_first_of(".@") != StringRef::npos)
+        GV.setName(cleanUpName(GV.getName()));
+    }
+}
+
 };
 }
 
 char NVPTXAssignValidGlobalNames::ID = 0;
 
 namespace llvm {
 void initializeNVPTXAssignValidGlobalNamesPass(PassRegistry &);
 }
 
 INITIALIZE_PASS(NVPTXAssignValidGlobalNames, "nvptx-assign-valid-global-names",
                 "Assign valid PTX names to globals", false, false)
-
 bool NVPTXAssignValidGlobalNames::runOnModule(Module &M) {
-  for (GlobalVariable &GV : M.globals()) {
-    // We are only allowed to rename local symbols.
-    if (GV.hasLocalLinkage()) {
-      // setName doesn't do extra work if the name does not change.
-      // Note: this does not create collisions - if setName is asked to set the
-      // name to something that already exists, it adds a proper postfix to
-      // avoid collisions.
-      GV.setName(cleanUpName(GV.getName()));
-    }
-  }
+  for (GlobalValue &GV : M.globals())
+    cleanUpLinkLocalValue(GV);
+
+  for (GlobalValue &GV : M.functions())
+    cleanUpLinkLocalValue(GV);
+
+  for (GlobalValue &GV : M.aliases())
+    cleanUpLinkLocalValue(GV);
+
+  for (GlobalValue &GV : M.ifuncs())
+    cleanUpLinkLocalValue(GV);
 
   return true;
 }
 
 std::string NVPTXAssignValidGlobalNames::cleanUpName(StringRef Name) {
   std::string ValidName;
   raw_string_ostream ValidNameStream(ValidName);
   for (unsigned I = 0, E = Name.size(); I != E; ++I) {

test/CodeGen/NVPTX/symbol-naming.ll

-; RUN: llc < %s -march=nvptx -mcpu=sm_20 | FileCheck %s --check-prefix=PTX32
-; RUN: llc < %s -march=nvptx64 -mcpu=sm_20 | FileCheck %s --check-prefix=PTX64
+; RUN: llc < %s -march=nvptx -mcpu=sm_20 | FileCheck %s
+; RUN: llc < %s -march=nvptx64 -mcpu=sm_20 | FileCheck %s
 
 ; Verify that the NVPTX target removes invalid symbol names prior to emitting
 ; PTX.
 
-; PTX32-NOT: .str
-; PTX64-NOT: .str
-
-; PTX32-DAG: _$_str.1
-; PTX32-DAG: _$_str
-
-; PTX64-DAG: _$_str.1
-; PTX64-DAG: _$_str
-
-target datalayout = "e-i64:64-v16:16-v32:32-n16:32:64"
-target triple = "nvptx64-unknown-unknown"
-
-
-@.str = private unnamed_addr constant [13 x i8] c"%d %f %c %d\0A\00", align 1
-@_$_str = private unnamed_addr constant [13 x i8] c"%d %f %c %d\0A\00", align 1
-
+; CHECK-NOT: .var
+; CHECK-NOT: _$_var.
+; CHECK-NOT: .internal_func
+; CHECK-NOT: _$_internal_func.
+
+; CHECK-DAG: _$_var_$_1
+; CHECK-DAG: _$_var
+@.var = private unnamed_addr constant i8 1
+@_$_var = private unnamed_addr constant  i8 2
+
+; CHECK-DAG: _$_internal_func_$_2()
+define internal i8 @.internal_func() {
+  ret i8 1
+}
 
-; Function Attrs: nounwind
-define void @foo(i32 %a, float %b, i8 signext %c, i32 %e) {
-entry:
-  %call = call i32 (i8*, ...) @printf(i8* getelementptr inbounds ([13 x i8], [13 x i8]* @.str, i32 0, i32 0))
-  ret void
+; CHECK-DAG: _$_internal_func()
+define internal i8 @_$_internal_func() {
+  ret i8 2
 }
 
-declare i32 @printf(i8*, ...)
+; Make sure we don't change global names
+; CHECK-DAG: .global_var
+@.global_var = unnamed_addr constant i8 3
+
+; CHECK-DAG: .externally_visible_func()
+define i8 @.externally_visible_func() {
+  ret i8 3
+}