[PATCH] D149451: [NVPTX] Add NVPTXCtorDtorLoweringPass to handle global ctors / dtors

[Artem Belevich via Phabricator via cfe-commits]

tra added inline comments.


================
Comment at: llvm/lib/Target/NVPTX/NVPTXCtorDtorLowering.cpp:58
+        ((IsCtor ? "__init_array_object_" : "__fini_array_object_") +
+         F->getName() + "_" + getHash(M.getName()) + "_" +
+         std::to_string(Priority))
----------------
tra wrote:
> jhuber6 wrote:
> > tra wrote:
> > > Source file name may be a little bit better, though it's still easy to clash if someone does `cd A; clang ./foo.c; cd ../B; clang ./foo.c` and the file name uses relative paths.
> > > 
> > > I think we'll need a way to override this unique suffix explicitly as an escape hatch for cases where someone runs into a clash.
> > I figured it'd be good enough since this is admittedly *very* niche. So someone would need to have a file called `foo.c` that also had a constructor called `foo` in it. For it to clash. Isn't it too late to grab the source filename while we're in the backend lowering stage?
> > someone would need to have a file called foo.c that also had a constructor called foo in it
> 
> Unlikely != impossible.  It's a trade-off between the hassle of implementing the plan B and the hassle of debugging and working around the clash for someone who runs into this.
> On one hand that's indeed unlikely to happen, but, given enough exposure, someone/somewhere will run into it and they will likely be ill-equipped to even tell what's going on.
> In general, compiler options are logistically much easier to deal with compared to having to change the source code.
> 
> > Isn't it too late to grab the source filename while we're in the backend lowering stage?
> 
> The module already has the file name recorded and available via `llvm::Module::getSourceFileName()`, so it's as easy to get as the module name.
On the second thought, do you think we'll ever end up running this pass with a module created purely in memory w/o having a source file name. Or, perhaps even without the module name either?

Even the hash of the IR itself will not be sufficient. Users are allowed to compile and link completely identical TUs as long as they don't have conflicting names.  I can imagine some sort of "plugin" module with only private symbols, but which has initializers to register stuff on startup. Two identical instances of such a module should be able to work, but they would end up with identical hash in this scheme. I do not see any way to automatically disambiguate them, short of using random numbers, but that would make compilation results unstable.

I still think we need to be able to provide the uniquifier manually via an option.




Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D149451/new/

https://reviews.llvm.org/D149451