[PATCH] D82280: Fix for NVPTX module asm regression

[Julia Tatz via Phabricator via llvm-commits]

jdtatz created this revision.
Herald added subscribers: llvm-commits, hiraditya, jholewinski.
Herald added a project: LLVM.

When the new NVPTX backend was added in commit ae556d3ef72dfe5f40a337b7071f42b7bf5b66a4 the NVPTXAsmPrinter::doInitialization method did not call it's parent AsmPrinter::doInitialization, which would emit module-level inline asm if it existed.

Emitting of module-level inline asm was then added in commit d2bbdf05e0b88524226589d89ffb2bfdc53ef3c8 but this was done without calling AsmPrinter::doInitialization and instead fixed by copying the module-level asm emitting code into NVPTXAsmPrinter::doInitialization after calling NVPTXAsmPrinter::emitHeader

Calling AsmPrinter::doInitialization was then added to NVPTXAsmPrinter::doInitialization in commit 68f2218e1efdb74442987a2040876e2e41f7d90e but NVPTXAsmPrinter::emitHeader was called afterwards, which meant that any module-level inline asm was added before the header which results in invalid ptx.
Also the copied module-level inline asm emitting code was left in, so module-level inline asm was added to the final ptx file twice, once before the header during AsmPrinter::doInitialization in an invalid position and once again after the header during NVPTXAsmPrinter::doInitialization in the correct position.

I have fixed this error by removing the copied module-level inline asm emitting code and adding the NVPTXAsmPrinter::emitHeader call to a new NVPTXAsmPrinter instantiation of the virtual method AsmPrinter::emitStartOfAsmFile which was designed for this use case. I did not add a unit test as one was added in commit d2bbdf05e0b88524226589d89ffb2bfdc53ef3c8 but without calling ptxas -v on every emitted ptx file we can't not verify the correctness, so I'm not sure of the usefulness of the unit test other than to ensure it doesn't cause any internal llvm issues.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D82280

Files:
  llvm/lib/Target/NVPTX/NVPTXAsmPrinter.cpp
  llvm/lib/Target/NVPTX/NVPTXAsmPrinter.h


Index: llvm/lib/Target/NVPTX/NVPTXAsmPrinter.h
===================================================================
--- llvm/lib/Target/NVPTX/NVPTXAsmPrinter.h
+++ llvm/lib/Target/NVPTX/NVPTXAsmPrinter.h
@@ -200,6 +200,7 @@
   const Function *F;
   std::string CurrentFnName;
 
+  void emitStartOfAsmFile(Module &M) override;
   void emitBasicBlockStart(const MachineBasicBlock &MBB) override;
   void emitFunctionEntryLabel() override;
   void emitFunctionBodyStart() override;
Index: llvm/lib/Target/NVPTX/NVPTXAsmPrinter.cpp
===================================================================
--- llvm/lib/Target/NVPTX/NVPTXAsmPrinter.cpp
+++ llvm/lib/Target/NVPTX/NVPTXAsmPrinter.cpp
@@ -762,13 +762,21 @@
   return InitList->getNumOperands() == 0;
 }
 
-bool NVPTXAsmPrinter::doInitialization(Module &M) {
+void NVPTXAsmPrinter::emitStartOfAsmFile(Module &M) {
   // Construct a default subtarget off of the TargetMachine defaults. The
   // rest of NVPTX isn't friendly to change subtargets per function and
   // so the default TargetMachine will have all of the options.
   const NVPTXTargetMachine &NTM = static_cast<const NVPTXTargetMachine &>(TM);
   const auto* STI = static_cast<const NVPTXSubtarget*>(NTM.getSubtargetImpl());
+  SmallString<128> Str1;
+  raw_svector_ostream OS1(Str1);
 
+  // Emit header before any dwarf directives are emitted below.
+  emitHeader(M, OS1, *STI);
+  OutStreamer->emitRawText(OS1.str());
+}
+
+bool NVPTXAsmPrinter::doInitialization(Module &M) {
   if (M.alias_size()) {
     report_fatal_error("Module has aliases, which NVPTX does not support.");
     return true; // error
@@ -784,26 +792,9 @@
     return true;  // error
   }
 
-  SmallString<128> Str1;
-  raw_svector_ostream OS1(Str1);
-
   // We need to call the parent's one explicitly.
   bool Result = AsmPrinter::doInitialization(M);
 
-  // Emit header before any dwarf directives are emitted below.
-  emitHeader(M, OS1, *STI);
-  OutStreamer->emitRawText(OS1.str());
-
-  // Emit module-level inline asm if it exists.
-  if (!M.getModuleInlineAsm().empty()) {
-    OutStreamer->AddComment("Start of file scope inline assembly");
-    OutStreamer->AddBlankLine();
-    OutStreamer->emitRawText(StringRef(M.getModuleInlineAsm()));
-    OutStreamer->AddBlankLine();
-    OutStreamer->AddComment("End of file scope inline assembly");
-    OutStreamer->AddBlankLine();
-  }
-
   GlobalsEmitted = false;
 
   return Result;


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D82280.272315.patch
Type: text/x-patch
Size: 2429 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20200621/3c3e7087/attachment.bin>