yaxunl marked an inline comment as done.
yaxunl added inline comments.

================
Comment at: lib/CodeGen/CGCUDANV.cpp:444
+    auto HandleValue =
+        CtorBuilder.CreateAlignedLoad(GpuBinaryHandle, CGM.getPointerAlign());
+    llvm::Constant *Zero = 
llvm::Constant::getNullValue(HandleValue->getType());
----------------
rjmccall wrote:
> yaxunl wrote:
> > rjmccall wrote:
> > > yaxunl wrote:
> > > > rjmccall wrote:
> > > > > Do you not need to worry about concurrency here?
> > > > The ctor functions are executed by the dynamic loader before the 
> > > > program gains the control. The dynamic loader cannot excute the ctor 
> > > > functions concurrently since doing that would not gurantee thread 
> > > > safety of the loaded program. Therefore we can assume sequential 
> > > > execution of ctor functions here.
> > > Okay.  That's worth a comment.
> > > 
> > > Is the name here specified by some ABI document, or is it just a 
> > > conventional name that we're picking now?
> > Will add a comment for that.
> > 
> > You mean `__hip_gpubin_handle`? It is an implementation detail. It is not 
> > defined by ABI or other documentation. Since it is only used internally by 
> > ctor functions, it is not a visible elf symbol. Its name is by convention 
> > since the cuda corresponding one was named __cuda_gpubin_handle.
> Well, it *is* ABI, right?  It's necessary for all translation units to agree 
> to use the same symbol here or else the registration will happen multiple 
> times.
Right. I created a pull request for HIP to document this 
https://github.com/ROCm-Developer-Tools/HIP/pull/580/files


https://reviews.llvm.org/D49083



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to