[AIX] Generate unique module id based on PID and timestamp

[Xiangling_L]

Summary

A unique module id, which is a part of sinit and sterm function names, is necessary to be unique. However, getUniqueModuleId will fail if there is no strong external symbol within a module.
We will turn to use PID and timestamp when this happens.

[Xiangling_L]

Rebase on the latest master;

[jasonliu]

llvm/lib/Target/PowerPC/PPCAsmPrinter.cpp

1841

Not sure what this condition is trying to achieve.
isUInt takes uint64_t as parameter, and we are asking if a uint64_t type is going to fit in 64 bit width. The answer is always going to be true.

1867

I think in theory, we only want to take this path when we have a stdin input file.
Other than that, we should be able to come up with a sufficiently unique name using output file name and options, etc.
Could we put a TODO somewhere to indicate that?

llvm/test/CodeGen/PowerPC/aix-static-init-no-unique-module-id.ll

10–24

Need a TODO here as well to indicate that this is not the final solution for this specific case.

[Xiangling_L]

llvm/lib/Target/PowerPC/PPCAsmPrinter.cpp

1841

As we discussed offline, current implementation of std::time_t may fail in 2038. But we will wait until then to see how OS upgrade the implementation and how we can adjust the implementation accordingly if needed.

1867

Sure. good suggestion.

[Xiangling_L]

Addressed the comments;
Fixed the testcase;

[jasonliu]

llvm/lib/Target/PowerPC/PPCAsmPrinter.cpp

1873

time_t is likely to be a signed integral on most target, because specification required return (std::time_t)(-1) on error.
So would it be better if we use llvm::itostr instead?

[Xiangling_L]

Use itostr for signed integer value ;

[jasonliu]

Thanks. LGTM.

[hubert.reinterpretcast]

llvm/lib/Target/PowerPC/PPCAsmPrinter.cpp

1872

I believe concatenating two strings of decimal digits without a separator increases the chance of collisions. Adding a separator is low cost for the value it would bring.

[Xiangling_L]

Add a separator between PID and timestamp;

[hubert.reinterpretcast]

llvm/test/CodeGen/PowerPC/aix-static-init-no-unique-module-id.ll

15

Sorry for not mentioning this earlier, but could we replace clang here with clangPidTime or similar? More generally, I think we should want to indicate the format style in that position.

[Xiangling_L]

llvm/test/CodeGen/PowerPC/aix-static-init-no-unique-module-id.ll

15

Do we only add indicator of format style when getUniqueModuleId[or source file full path later] does not work? i.e. do we also want to add one when getUniqueModuleId[or source file full path] does work and replace clang with something like clangUniqueModId[or clangSrcFullPath]?

[hubert.reinterpretcast]

llvm/test/CodeGen/PowerPC/aix-static-init-no-unique-module-id.ll

15

I think we would also want a format indicator for the other cases. This would help in case we find reasons for having more than one format available.

[Xiangling_L]

llvm/test/CodeGen/PowerPC/aix-static-init-no-unique-module-id.ll

15

I see. Since "adding format indicator" doesn't really belong to the scope of this patch, and we will later have a follow-up patch of using source file full path to generate module id, I am thinking for this "the other case" we can leave a TODO and address it later?

[hubert.reinterpretcast]

llvm/test/CodeGen/PowerPC/aix-static-init-no-unique-module-id.ll

15

That's fine. We can still handle the current case here first though.

[Xiangling_L]

Add a format indicator as a part of function names;

[hubert.reinterpretcast]

Thanks; this LGTM with comments. Please double check to make sure the capitalization of clangPidTime is consistent (including in comments) if making that change.

llvm/lib/Target/PowerPC/PPCAsmPrinter.cpp

158–161

s/as a/to form/; s/part of/part of the/;

1864–1880

Minor nits: s/which are/to be/; s/part of/part of the/;

1875

Add "the" before "PID" and "unique module id".

1880

The LLVM-style for camelCase indicates that acronyms are to be capitalized in the style of regular words. I suggest that we be consistent with that, but (noting that this is not an identifier name in the source) I am okay with the current form as well.

llvm/test/CodeGen/PowerPC/aix-static-init-no-unique-module-id.ll

12

s/PID/the PID/;

19–20

We should check that the alias descriptor label immediately follows the start of the descriptor csect. Please adjust the alignment of the other lines to match.

21–22

Same comment for the entry point labels.

llvm/lib/Target/PowerPC/PPCAsmPrinter.cpp

 #include "llvm/MC/MCSymbol.h"
 #include "llvm/MC/MCSymbolELF.h"
 #include "llvm/MC/MCSymbolXCOFF.h"
 #include "llvm/MC/SectionKind.h"
 #include "llvm/Support/Casting.h"
 #include "llvm/Support/CodeGen.h"
 #include "llvm/Support/Debug.h"
 #include "llvm/Support/ErrorHandling.h"
+#include "llvm/Support/Process.h"
 #include "llvm/Support/TargetRegistry.h"
 #include "llvm/Support/raw_ostream.h"
 #include "llvm/Target/TargetMachine.h"
 #include "llvm/Transforms/Utils/ModuleUtils.h"
 #include <algorithm>
 #include <cassert>
 #include <cstdint>
 #include <memory>

 };
 
 class PPCAIXAsmPrinter : public PPCAsmPrinter {
 private:
   /// Symbols lowered from ExternalSymbolSDNodes, we will need to emit extern
   /// linkage for them in AIX.
   SmallPtrSet<MCSymbol *, 8> ExtSymSDNodeSymbols;
 
-  /// A unique trailing identifier as a part of sinit/sterm functions.
-  std::string GlobalUniqueModuleId;
+  /// A format indicator and unique trailing identifier to form part of the
+  /// sinit/sterm function names.
+  std::string FormatIndicatorAndUniqueModId;
 
   static void ValidateGV(const GlobalVariable *GV);
   // Record a list of GlobalAlias associated with a GlobalObject.
   // This is used for AIX's extra-label-at-definition aliasing strategy.
   DenseMap<const GlobalObject *, SmallVector<const GlobalAlias *, 1>>
       GOAliasMap;
 
 public:
   PPCAIXAsmPrinter(TargetMachine &TM, std::unique_ptr<MCStreamer> Streamer)
for (auto &I : TOC) {
     OutStreamer->emitLabel(I.second);
     if (TS != nullptr)
       TS->emitTCEntry(*I.first);
   }
 }
 
 bool PPCAIXAsmPrinter::doInitialization(Module &M) {
   const bool Result = PPCAsmPrinter::doInitialization(M);
 
   auto setCsectAlignment = [this](const GlobalObject *GO) {
     // Declarations have 0 alignment which is set by default.
     if (GO->isDeclarationForLinker())
       return;
 
     SectionKind GOKind = getObjFileLowering().getKindForGlobal(GO, TM);
     MCSectionXCOFF *Csect = cast<MCSectionXCOFF>(
         getObjFileLowering().SectionForGlobal(GO, GOKind, TM));
 
     Align GOAlign = getGVAlignment(GO, GO->getParent()->getDataLayout());
     if (GOAlign > Csect->getAlignment())
       Csect->setAlignment(GOAlign);
   };
 
   // We need to know, up front, the alignment of csects for the assembly path,
   // because once a .csect directive gets emitted, we could not change the
   // alignment value on it.
   for (const auto &G : M.globals()) {
     if (isSpecialLLVMGlobalArrayToSkip(&G))
       continue;
 
     if (isSpecialLLVMGlobalArrayForStaticInit(&G)) {
-      // Generate a unique module id which is a part of sinit and sterm function
-      // names.
-      if (GlobalUniqueModuleId.empty()) {
-        GlobalUniqueModuleId = getUniqueModuleId(&M);
-        // FIXME: We need to figure out what to hash on or encode into the
-        // unique ID we need.
-        if (GlobalUniqueModuleId.compare("") == 0)
-          llvm::report_fatal_error(
-              "cannot produce a unique identifier for this module based on"
-              " strong external symbols");
-        GlobalUniqueModuleId = GlobalUniqueModuleId.substr(1);
+      // Generate a format indicator and a unique module id to be a part of
+      // the sinit and sterm function names.
+      if (FormatIndicatorAndUniqueModId.empty()) {
+        std::string UniqueModuleId = getUniqueModuleId(&M);
+        if (UniqueModuleId.compare("") != 0)
+          // TODO: Use source file full path to generate the unique module id
+          // and add a format indicator as a part of function name in case we
+          // will support more than one format.
+          FormatIndicatorAndUniqueModId = "clang_" + UniqueModuleId.substr(1);
+        else
+          // Use the Pid and current time as the unique module id when we cannot
+          // generate one based on a module's strong external symbols.
+          // FIXME: Adjust the comment accordingly after we use source file full
+          // path instead.
+          FormatIndicatorAndUniqueModId =
+              "clangPidTime_" + llvm::itostr(sys::Process::getProcessId()) +
+              "_" + llvm::itostr(time(nullptr));
       }
 
       emitSpecialLLVMGlobal(&G);
       continue;
     }
 
     setCsectAlignment(&G);
   }
void PPCAIXAsmPrinter::emitXXStructorList(const DataLayout &DL,
   for (Structor &S : Structors) {
     if (S.Priority != 65535)
       report_fatal_error(
           "prioritized sinit and sterm functions are not yet supported on AIX");
 
     llvm::GlobalAlias::create(
         GlobalValue::ExternalLinkage,
         (IsCtor ? llvm::Twine("__sinit") : llvm::Twine("__sterm")) +
-            llvm::Twine("80000000_clang_", GlobalUniqueModuleId) +
+            llvm::Twine("80000000_", FormatIndicatorAndUniqueModId) +
             llvm::Twine("_", llvm::utostr(Index++)),
         cast<Function>(S.Func));
   }
 }
 
 /// createPPCAsmPrinterPass - Returns a pass that prints the PPC assembly code
 /// for a MachineFunction to the given output stream, in a format that the
 /// Darwin assembler can deal with.

llvm/test/CodeGen/PowerPC/aix-static-init-no-unique-module-id.ll

-; RUN: not --crash llc -mtriple powerpc-ibm-aix-xcoff -o - %s 2>&1 | FileCheck %s
-; RUN: not --crash llc -mtriple powerpc64-ibm-aix-xcoff -o - %s 2>&1 | FileCheck %s
+; RUN: llc -mtriple powerpc-ibm-aix-xcoff < %s | FileCheck %s
+; RUN: llc -mtriple powerpc64-ibm-aix-xcoff < %s | FileCheck %s
 
 @llvm.global_ctors = appending global [1 x { i32, void ()*, i8* }] [{ i32, void ()*, i8* } { i32 65535, void ()* @foo, i8* null }]
 
 define internal void @foo() {
   ret void
 }
 
-; CHECK: LLVM ERROR: cannot produce a unique identifier for this module based on strong external symbols
+; FIXME: Adjust the comment after we use source file full path to generate unique
+; module id instead.
+; Use the Pid and timestamp to generate a unique module id when strong external
+; symbols are not available in current module. The module id generated in this
+; way is not reproducible. A function name sample would be:
+; __sinit80000000_clangPidTime_119189_1597348415_0
+
+; CHECK:              .lglobl        foo[DS]
+; CHECK:              .lglobl        .foo
+; CHECK:              .csect foo[DS]
+; CHECK-NEXT: __sinit80000000_clangPidTime_[[PID:[0-9]+]]_[[TIMESTAMP:[0-9]+]]_0:
+; CHECK:      .foo:
+; CHECK-NEXT: .__sinit80000000_clangPidTime_[[PID]]_[[TIMESTAMP]]_0:
+; CHECK:      .globl  __sinit80000000_clangPidTime_[[PID]]_[[TIMESTAMP]]_0
+; CHECK:      .globl  .__sinit80000000_clangPidTime_[[PID]]_[[TIMESTAMP]]_0