[PATCH] D149340: [AMDGPU] Place global constructors in .init_array and .fini_array

[Yaxun Liu via Phabricator via llvm-commits]

yaxunl added inline comments.


================
Comment at: llvm/lib/Target/AMDGPU/AMDGPUCtorDtorLowering.cpp:76
+    if (!AlreadyRegistered)
+      IRB.CreateCall(ConstructorTy, CS->getOperand(1));
   }
----------------
jhuber6 wrote:
> yaxunl wrote:
> > jhuber6 wrote:
> > > yaxunl wrote:
> > > > jhuber6 wrote:
> > > > > yaxunl wrote:
> > > > > > I noticed these functions are called not following the priority.
> > > > > > 
> > > > > > However, I guess that is out of the scope of this patch.
> > > > > True, it was like that when I got here. Do you know who the current user is for this feature?
> > > > Currently, all HIP programs use this feature when -fsanitize=addr is used.
> > > > 
> > > > But they do not care about priority yet.
> > > FWIW we could change this to be a single kernel that calls an ASAN library function, which implements the method in the commit header to traverse the list in priority order.
> > We want to keep this feature as a generic approach to support dynamic initialization.
> This will also cause a duplicate symbol if someone compiles without monolithic LTO. I'm assuming that's the expected behavior.
That is fine. HIP runtime will launch all of them as long as they have init kernel metadata. It is not based on the kernel name.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149340