[NVPTX] Work around syntax error in PTX assembly

[ldrumm]

Summary

The PTX assembler doesn't like dots in symbol names. We have a pass that
fixes this (NVPTXAssignValidGlobalNames - since 264cd4672d), but there's
a pernicious bug in the way debug info is emitted that means this pass
alone won't work properly if it's ever run as part of the codegen
pipeline involving debug info emission.

AsmPrinter::doInitialization collects all metadata and initializes all
the DWARF DIEs that are later emitted after the main module has been
printed. Naturally, doInitialization is executed *before* any of the
other passes contained in the same PassManager execute their
runOnModule functions that may arbitrarily modify the module.
If any of those functions try and modify the module in such a way that
the debuginfo goes out of date, then the debuginfo *will* be out of date
once the AsmPrinter actually gets to finalize printing the debug info.

As far as I can tell: any code that runs in the same PassManager as the
AsmPrinter could result in bad debug info.

I don't see a simple way around this, unless DWARF debug info is
collected and printed a different way - probably during the
runOnModule method. That will be much more disruptive change, so for
now the small (!) hack introduced here sanitizes the symbol names
referred to by the MCSymbolRefExpr that is earlier constructed by the
AsmPrinter's DWARF handler.

N.B. There's the slight possibility that this also introduces a new bug

• sanitized symbol names referred to by the MCExprs may not be of

private linkage, and so may not actually exist. Given that symbols with
dots in them are are already illegal according to ptxas, I don't think
this is a practical issue to worry about.

Since the NVPTXUtilities are now used by the MC layer, I've hoisted them
into their own Utils lib as per ARM and AMDGPU.

Fixes https://github.com/intel/llvm/issues/5980

[ldrumm]

TODO: Testcase

[tra]

LGTM in principle. It should be good to go once the tests are added.

[ldrumm]

Slight fly in the ointment: The existing tests exposed that that this implementation is too naive and is renaming things it shouldn't. I'll be back with a more mature implementation. Thanks for the reviews and patience

[tra]

In D141072#4036024, @ldrumm wrote:

Slight fly in the ointment: The existing tests exposed that that this implementation is too naive and is renaming things it shouldn't. I'll be back with a more mature implementation. Thanks for the reviews and patience

For what it's worth, we did have a handful of attempts to improve PTX symbol character set compliance in the past, but we've failed to find a much better fix than what we have now. IIRC, the proposal closest to 'generic' solution was to make back-end responsible for providing the legal character set for the symbols and then make all IT transforms use it, instead of the current implicit assumptions we're relying on.

[arsenm]

I'd expect this to be dealt with in Mangler

llvm/lib/Target/NVPTX/CMakeLists.txt

set(NVPTXCodeGen_sources
   NVPTXPeephole.cpp
   NVPTXMCExpr.cpp
   NVPTXPrologEpilogPass.cpp
   NVPTXRegisterInfo.cpp
   NVPTXReplaceImageHandles.cpp
   NVPTXSubtarget.cpp
   NVPTXTargetMachine.cpp
   NVPTXTargetTransformInfo.cpp
-  NVPTXUtilities.cpp
   NVVMIntrRange.cpp
   NVVMReflect.cpp
   NVPTXProxyRegErasure.cpp
   )
 
 add_llvm_target(NVPTXCodeGen
   ${NVPTXCodeGen_sources}
 
   LINK_COMPONENTS
   Analysis
   AsmPrinter
   CodeGen
   Core
   IPO
   MC
   NVPTXDesc
   NVPTXInfo
   Scalar
   SelectionDAG
   Support
   Target
   TargetParser
   TransformUtils
   Vectorize
+  NVPTXUtils
 
   ADD_TO_COMPONENT
   NVPTX
 )
 
 add_subdirectory(MCTargetDesc)
 add_subdirectory(TargetInfo)
+add_subdirectory(Utils)

llvm/lib/Target/NVPTX/MCTargetDesc/CMakeLists.txt

 add_llvm_component_library(LLVMNVPTXDesc
   NVPTXInstPrinter.cpp
   NVPTXMCAsmInfo.cpp
   NVPTXMCTargetDesc.cpp
   NVPTXTargetStreamer.cpp
 
   LINK_COMPONENTS
   MC
   NVPTXInfo
+  NVPTXUtils
   Support
   TargetParser
 
   ADD_TO_COMPONENT
   NVPTX
   )

llvm/lib/Target/NVPTX/MCTargetDesc/NVPTXTargetStreamer.h

public:
   /// functions.
   void emitDwarfFileDirective(StringRef Directive) override;
   void changeSection(const MCSection *CurSection, MCSection *Section,
                      const MCExpr *SubSection, raw_ostream &OS) override;
   /// Emit the bytes in \p Data into the output.
   ///
   /// This is used to emit bytes in \p Data as sequence of .byte directives.
   void emitRawBytes(StringRef Data) override;
+  void emitValue(const MCExpr *Value) override;
 };
 
 class NVPTXAsmTargetStreamer : public NVPTXTargetStreamer {
 public:
   NVPTXAsmTargetStreamer(MCStreamer &S);
   ~NVPTXAsmTargetStreamer() override;
 };
 
 } // end namespace llvm
 
 #endif

llvm/lib/Target/NVPTX/MCTargetDesc/NVPTXTargetStreamer.cpp

 //=====- NVPTXTargetStreamer.cpp - NVPTXTargetStreamer class ------------=====//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 //
 // This file implements the NVPTXTargetStreamer class.
 //
 //===----------------------------------------------------------------------===//
 
 #include "NVPTXTargetStreamer.h"
+#include "Utils/NVPTXUtilities.h"
 #include "llvm/MC/MCAsmInfo.h"
 #include "llvm/MC/MCContext.h"
+#include "llvm/MC/MCExpr.h"
 #include "llvm/MC/MCObjectFileInfo.h"
 
 using namespace llvm;
 
 //
 // NVPTXTargetStreamer Implemenation
 //
 NVPTXTargetStreamer::NVPTXTargetStreamer(MCStreamer &S) : MCTargetStreamer(S) {}
Section->printSwitchToSection(*getStreamer().getContext().getAsmInfo(),
                                   getStreamer().getContext().getTargetTriple(),
                                   OS, SubSection);
     // DWARF sections are enclosed into braces - emit the open one.
     OS << "\t{\n";
     HasSections = true;
   }
 }
 
+void NVPTXTargetStreamer::emitValue(const MCExpr *Value) {
+  if (Value->getKind() != MCExpr::ExprKind::SymbolRef)
+    return MCTargetStreamer::emitValue(Value);
+
+  SmallString<128> Str;
+  raw_svector_ostream OS(Str);
+  Value->print(OS, Streamer.getContext().getAsmInfo());
+
+  Streamer.emitRawText(sanitizePTXSymName(Str));
+}
+
 void NVPTXTargetStreamer::emitRawBytes(StringRef Data) {
   MCTargetStreamer::emitRawBytes(Data);
   // TODO: enable this once the bug in the ptxas with the packed bytes is
   // resolved. Currently, (it is confirmed by NVidia) it causes a crash in
   // ptxas.
 #if 0
   const MCAsmInfo *MAI = Streamer.getContext().getAsmInfo();
   const char *Directive = MAI->getData8bitsDirective();

llvm/lib/Target/NVPTX/NVPTXAsmPrinter.cpp

 #include "MCTargetDesc/NVPTXMCAsmInfo.h"
 #include "MCTargetDesc/NVPTXTargetStreamer.h"
 #include "NVPTX.h"
 #include "NVPTXMCExpr.h"
 #include "NVPTXMachineFunctionInfo.h"
 #include "NVPTXRegisterInfo.h"
 #include "NVPTXSubtarget.h"
 #include "NVPTXTargetMachine.h"
-#include "NVPTXUtilities.h"
+#include "Utils/NVPTXUtilities.h"
 #include "TargetInfo/NVPTXTargetInfo.h"
 #include "cl_common_defines.h"
 #include "llvm/ADT/APFloat.h"
 #include "llvm/ADT/APInt.h"
 #include "llvm/ADT/DenseMap.h"
 #include "llvm/ADT/DenseSet.h"
 #include "llvm/ADT/SmallString.h"
 #include "llvm/ADT/SmallVector.h"

llvm/lib/Target/NVPTX/NVPTXAssignValidGlobalNames.cpp

 // Currently NVPTX, like other backends, relies on generic symbol name
 // sanitizing done by MC. However, the ptxas assembler is more stringent and
 // disallows some additional characters in symbol names. This pass makes sure
 // such names do not reach MC at all.
 //
 //===----------------------------------------------------------------------===//
 
 #include "NVPTX.h"
+#include "Utils/NVPTXUtilities.h"
 #include "llvm/IR/Function.h"
 #include "llvm/IR/GlobalVariable.h"
 #include "llvm/IR/LegacyPassManager.h"
 #include "llvm/IR/Module.h"
 #include "llvm/Support/raw_ostream.h"
 #include <string>
 
 using namespace llvm;

 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()));
+      GV.setName(sanitizePTXSymName(GV.getName()));
     }
   }
 
   // Do the same for local functions.
   for (Function &F : M.functions())
     if (F.hasLocalLinkage())
-      F.setName(cleanUpName(F.getName()));
+      F.setName(sanitizePTXSymName(F.getName()));
 
   return true;
 }
 
-std::string NVPTXAssignValidGlobalNames::cleanUpName(StringRef Name) {
-  std::string ValidName;
-  raw_string_ostream ValidNameStream(ValidName);
-  for (char C : Name) {
-    if (C == '.' || C == '@') {
-      ValidNameStream << "_$_";
-    } else {
-      ValidNameStream << C;
-    }
-  }
-
-  return ValidNameStream.str();
-}
-
 ModulePass *llvm::createNVPTXAssignValidGlobalNamesPass() {
   return new NVPTXAssignValidGlobalNames();
 }

llvm/lib/Target/NVPTX/NVPTXGenericToNVVM.cpp

 //===-- GenericToNVVM.cpp - Convert generic module to NVVM module - C++ -*-===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 //
 // Convert generic global variables into either .global or .const access based
 // on the variable's "constant" qualifier.
 //
 //===----------------------------------------------------------------------===//
 
 #include "MCTargetDesc/NVPTXBaseInfo.h"
 #include "NVPTX.h"
-#include "NVPTXUtilities.h"
+#include "Utils/NVPTXUtilities.h"
 #include "llvm/CodeGen/ValueTypes.h"
 #include "llvm/IR/Constants.h"
 #include "llvm/IR/DerivedTypes.h"
 #include "llvm/IR/IRBuilder.h"
 #include "llvm/IR/Instructions.h"
 #include "llvm/IR/Intrinsics.h"
 #include "llvm/IR/LegacyPassManager.h"
 #include "llvm/IR/Module.h"

llvm/lib/Target/NVPTX/NVPTXISelDAGToDAG.cpp

 //===-- NVPTXISelDAGToDAG.cpp - A dag to dag inst selector for NVPTX ------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 //
 // This file defines an instruction selector for the NVPTX target.
 //
 //===----------------------------------------------------------------------===//
 
 #include "NVPTXISelDAGToDAG.h"
 #include "MCTargetDesc/NVPTXBaseInfo.h"
-#include "NVPTXUtilities.h"
+#include "Utils/NVPTXUtilities.h"
 #include "llvm/Analysis/ValueTracking.h"
 #include "llvm/IR/GlobalValue.h"
 #include "llvm/IR/Instructions.h"
 #include "llvm/IR/IntrinsicsNVPTX.h"
 #include "llvm/Support/AtomicOrdering.h"
 #include "llvm/Support/CommandLine.h"
 #include "llvm/Support/Debug.h"
 #include "llvm/Support/ErrorHandling.h"

llvm/lib/Target/NVPTX/NVPTXISelLowering.cpp

 //===----------------------------------------------------------------------===//
 
 #include "NVPTXISelLowering.h"
 #include "MCTargetDesc/NVPTXBaseInfo.h"
 #include "NVPTX.h"
 #include "NVPTXSubtarget.h"
 #include "NVPTXTargetMachine.h"
 #include "NVPTXTargetObjectFile.h"
-#include "NVPTXUtilities.h"
+#include "Utils/NVPTXUtilities.h"
 #include "llvm/ADT/APInt.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/SmallVector.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/CodeGen/Analysis.h"
 #include "llvm/CodeGen/MachineFunction.h"
 #include "llvm/CodeGen/MachineMemOperand.h"
 #include "llvm/CodeGen/SelectionDAG.h"

llvm/lib/Target/NVPTX/NVPTXImageOptimizer.cpp

 //===-- NVPTXImageOptimizer.cpp - Image optimization pass -----------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 //
 // This pass implements IR-level optimizations of image access code,
 // including:
 //
 // 1. Eliminate istypep intrinsics when image access qualifier is known
 //
 //===----------------------------------------------------------------------===//
 
 #include "NVPTX.h"
-#include "NVPTXUtilities.h"
+#include "Utils/NVPTXUtilities.h"
 #include "llvm/Analysis/ConstantFolding.h"
 #include "llvm/IR/Instructions.h"
 #include "llvm/IR/Intrinsics.h"
 #include "llvm/IR/IntrinsicsNVPTX.h"
 #include "llvm/IR/Module.h"
 #include "llvm/Pass.h"
 
 using namespace llvm;

llvm/lib/Target/NVPTX/NVPTXLowerAlloca.cpp

 //   %Generic = addrspacecast i32 addrspace(5)* %A to i32*
 //   store i32 0, i32 addrspace(5)* %Generic ; emits st.local.u32
 //
 // And we will rely on NVPTXInferAddressSpaces to combine the last two
 // instructions.
 //
 //===----------------------------------------------------------------------===//
 
-#include "NVPTX.h"
-#include "NVPTXUtilities.h"
 #include "MCTargetDesc/NVPTXBaseInfo.h"
+#include "NVPTX.h"
+#include "Utils/NVPTXUtilities.h"
 #include "llvm/IR/Function.h"
 #include "llvm/IR/Instructions.h"
 #include "llvm/IR/IntrinsicInst.h"
 #include "llvm/IR/Module.h"
 #include "llvm/IR/Type.h"
 #include "llvm/Pass.h"
 
 using namespace llvm;

llvm/lib/Target/NVPTX/NVPTXLowerArgs.cpp

 //
 // TODO: merge this pass with NVPTXInferAddressSpaces so that other passes don't
 // cancel the addrspacecast pair this pass emits.
 //===----------------------------------------------------------------------===//
 
 #include "MCTargetDesc/NVPTXBaseInfo.h"
 #include "NVPTX.h"
 #include "NVPTXTargetMachine.h"
-#include "NVPTXUtilities.h"
+#include "Utils/NVPTXUtilities.h"
 #include "llvm/Analysis/ValueTracking.h"
 #include "llvm/IR/Function.h"
 #include "llvm/IR/Instructions.h"
 #include "llvm/IR/Module.h"
 #include "llvm/IR/Type.h"
 #include "llvm/Pass.h"
 #include <numeric>
 #include <queue>

llvm/lib/Target/NVPTX/NVPTXTargetTransformInfo.cpp

 //===-- NVPTXTargetTransformInfo.cpp - NVPTX specific TTI -----------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 #include "NVPTXTargetTransformInfo.h"
-#include "NVPTXUtilities.h"
+#include "Utils/NVPTXUtilities.h"
 #include "llvm/Analysis/LoopInfo.h"
 #include "llvm/Analysis/TargetTransformInfo.h"
 #include "llvm/Analysis/ValueTracking.h"
 #include "llvm/CodeGen/BasicTTIImpl.h"
 #include "llvm/CodeGen/CostTable.h"
 #include "llvm/CodeGen/TargetLowering.h"
 #include "llvm/IR/IntrinsicsNVPTX.h"
 #include "llvm/Support/Debug.h"

llvm/lib/Target/NVPTX/Utils/CMakeLists.txt

+add_llvm_component_library(LLVMNVPTXUtils
+  NVPTXUtilities.cpp
+
+  LINK_COMPONENTS
+  Support
+
+  ADD_TO_COMPONENT
+  NVPTX
+  )

llvm/lib/Target/NVPTX/Utils/NVPTXUtilities.h

if (size <= 32)
     return 32;
   else if (size <= 64)
     return 64;
   else
     return size;
 }
 
 bool shouldEmitPTXNoReturn(const Value *V, const TargetMachine &TM);
-}
+/// `ptxas` doesn't like dots in symbol names. These show up for static locals
+/// (among others). Get a legal replacement name
+std::string sanitizePTXSymName(StringRef Name);
+} // namespace llvm
 
 #endif

llvm/lib/Target/NVPTX/Utils/NVPTXUtilities.cpp

return CallI->doesNotReturn() &&
            CallI->getFunctionType()->getReturnType()->isVoidTy();
 
   const Function *F = cast<Function>(V);
   return F->doesNotReturn() &&
          F->getFunctionType()->getReturnType()->isVoidTy() &&
          !isKernelFunction(*F);
 }
 
+std::string sanitizePTXSymName(StringRef Name) {
+  std::string ValidName;
+  raw_string_ostream ValidNameStream(ValidName);
+  for (char C : Name) {
+    if (C == '.' || C == '@') {
+      ValidNameStream << "_$_";
+    } else {
+      ValidNameStream << C;
+    }
+  }
+  return ValidNameStream.str();
+}
 } // namespace llvm