hliao added a comment.

In D80858#2169399 <https://reviews.llvm.org/D80858#2169399>, @yaxunl wrote:

> In D80858#2169295 <https://reviews.llvm.org/D80858#2169295>, @hliao wrote:
>
> > I don't that's proper way to support file-scope static device variables. As 
> > we discuss APIs like cudaMemcpyToSymol, that's a runtime API instead of 
> > driver API. The later needs to specify the module (or code object) id in 
> > addition to a symbol name and won't have the conflict issues. For the 
> > runtime API, all named device variables (static or not) are identified at 
> > the host side by their host stub variables. That stub variable is used to 
> > register the corresponding device variables associated with a module id to 
> > unique identify that variables across all TUs. As long as we look up device 
> > variables using their host stub variable pointers, they are uniquely 
> > identified already. The runtime implementation needs to find the module id 
> > and the variable symbol from the pointer of its host stub variable. It's 
> > not the compiler job to fabricate name uniquely across TUs.
>
>
> The problem is that even though the static variable is registered through 
> `__hipRigisterVariable`, the runtime still relies on looking up symbol name 
> to get the address of the device variable. That's why we need to externalize 
> the static variable.


If so, the runtime should be fixed as the variable name. I remembered I fixed 
the global one so that each one is uniquely identified by module id plus the 
name. For runtime APIs, all host-side references to device variables should 
look through the host stub variables instead of its name. If runtime or API 
doesn't follow that, we should fix them instead of asking the compiler to do 
the favor.


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

https://reviews.llvm.org/D80858



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

Reply via email to