[PATCH] D80858: [CUDA][HIP] Support accessing static device variable in host code for -fno-gpu-rdc

tra added a comment. > The fix is here https://reviews.llvm.org/D85686 Thank you. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D80858/new/ https://reviews.llvm.org/D80858 ___ cfe-commits mailing list cf

[PATCH] D80858: [CUDA][HIP] Support accessing static device variable in host code for -fno-gpu-rdc

yaxunl added a comment. In D80858#2207898 , @tra wrote: >> We can restrict externalization to constant variables with explicit >> 'constant' attributes only, which should fix this issue. > > SGTM. If it does not have explicit device-side attribute, it's n

[PATCH] D80858: [CUDA][HIP] Support accessing static device variable in host code for -fno-gpu-rdc

tra added a comment. > We can restrict externalization to constant variables with explicit > 'constant' attributes only, which should fix this issue. SGTM. If it does not have explicit device-side attribute, it's never going to need to cross host/device boundary. I guess this applies to vars wi

[PATCH] D80858: [CUDA][HIP] Support accessing static device variable in host code for -fno-gpu-rdc

yaxunl added a comment. In D80858#2207699 , @tra wrote: > Sam, just a FYI that the patch has a couple of unintended consequences. > > We now end up with various things instantiated as device-side __constant__ > objects when they were not before, when we c

[PATCH] D80858: [CUDA][HIP] Support accessing static device variable in host code for -fno-gpu-rdc

tra added a comment. Sam, just a FYI that the patch has a couple of unintended consequences. We now end up with various things instantiated as device-side __constant__ objects when they were not before, when we compile with -std=c++17 (especially with libc++): https://godbolt.org/z/KbTM9M That

[PATCH] D80858: [CUDA][HIP] Support accessing static device variable in host code for -fno-gpu-rdc

This revision was not accepted when it landed; it landed in state "Needs Review". This revision was automatically updated to reflect the committed changes. Closed by commit rG45f2a56856e2: [CUDA][HIP] Support accessing static device variable in host code for -fno-gpu… (authored by yaxunl). Herald

[PATCH] D80858: [CUDA][HIP] Support accessing static device variable in host code for -fno-gpu-rdc

yaxunl added a comment. In D80858#2193970 , @tra wrote: > What is expected to happen to device statics in anonymous name spaces? It may > be worth adding them to the tests. > > LGTM otherwise. static device var in anonymous name space will have external

[PATCH] D80858: [CUDA][HIP] Support accessing static device variable in host code for -fno-gpu-rdc

tra accepted this revision. tra added a comment. What is expected to happen to device statics in anonymous name spaces? It may be worth adding them to the tests. LGTM otherwise. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D80858/new/ https://reviews.llvm.org/D80858 _

[PATCH] D80858: [CUDA][HIP] Support accessing static device variable in host code for -fno-gpu-rdc

yaxunl updated this revision to Diff 282952. yaxunl retitled this revision from "[CUDA][HIP] Support accessing static device variable in host code" to "[CUDA][HIP] Support accessing static device variable in host code for -fno-gpu-rdc". yaxunl edited the summary of this revision. yaxunl added a c

[PATCH] D80858: [CUDA][HIP] Support accessing static device variable in host code

tra added a comment. I think Sam's approach is reasonable. The ability to supply CUID to sub-compilations is useful by itself and should probably be split into a separate patch as it's largely independent of the externalization of file-scope static vars. CHANGES SINCE LAST ACTION https://re

[PATCH] D80858: [CUDA][HIP] Support accessing static device variable in host code

hliao added a comment. In D80858#2177159 , @tra wrote: > It's a good point. Perhaps this is one of the cases where we should *not* > follow nvcc. > We can't have our cake (preserve static behavior) and eat it (treat it as > non-static in case something o

[PATCH] D80858: [CUDA][HIP] Support accessing static device variable in host code

hliao added a comment. In D80858#2177104 , @yaxunl wrote: > In D80858#2171266 , @hliao wrote: > >> We may try to solve the issue without RDC firstly, where we don't need to >> change that static variable name (if t

[PATCH] D80858: [CUDA][HIP] Support accessing static device variable in host code

yaxunl added a comment. In D80858#2177159 , @tra wrote: > It's a good point. Perhaps this is one of the cases where we should *not* > follow nvcc. > We can't have our cake (preserve static behavior) and eat it (treat it as > non-static in case something

[PATCH] D80858: [CUDA][HIP] Support accessing static device variable in host code

tra added a comment. It's a good point. Perhaps this is one of the cases where we should *not* follow nvcc. We can't have our cake (preserve static behavior) and eat it (treat it as non-static in case something on the host side may decide to use an API which uses symbol names). Something's got

[PATCH] D80858: [CUDA][HIP] Support accessing static device variable in host code

yaxunl added a comment. In D80858#2171266 , @hliao wrote: > We may try to solve the issue without RDC firstly, where we don't need to > change that static variable name (if the runtime maintains the device > binaries correctly.) We only need to ensure th

[PATCH] D80858: [CUDA][HIP] Support accessing static device variable in host code

hliao added a comment. In D80858#2170821 , @tra wrote: > In D80858#2170781 , @hliao wrote: > > > The problem is not whether we have solution to tell them but when we need > > to add that. Not all `static` device va

[PATCH] D80858: [CUDA][HIP] Support accessing static device variable in host code

tra added a comment. In D80858#2170781 , @hliao wrote: > The problem is not whether we have solution to tell them but when we need to > add that. Not all `static` device variables need to be visible to the host > side. `Externalizing` them adds the overh

[PATCH] D80858: [CUDA][HIP] Support accessing static device variable in host code

hliao added a comment. In D80858#2170668 , @tra wrote: > In D80858#2170604 , @hliao wrote: > > > In D80858#2170547 , @tra wrote: > > > > > Would it work if we generate a glob

[PATCH] D80858: [CUDA][HIP] Support accessing static device variable in host code

tra added a comment. In D80858#2170604 , @hliao wrote: > In D80858#2170547 , @tra wrote: > > > Would it work if we generate a globally unique visible aliases for the > > static vars and use the alias' name to regis

[PATCH] D80858: [CUDA][HIP] Support accessing static device variable in host code

hliao added a comment. In D80858#2170547 , @tra wrote: > Would it work if we generate a globally unique visible aliases for the static > vars and use the alias' name to register device-side entities without > changing their visibility? We still need to

[PATCH] D80858: [CUDA][HIP] Support accessing static device variable in host code

hliao added a comment. In D80858#2170533 , @yaxunl wrote: > In D80858#2170328 , @hliao wrote: > > > In D80858#2169534 , @yaxunl wrote: > > > > > Another reason is that we nee

[PATCH] D80858: [CUDA][HIP] Support accessing static device variable in host code

tra added a comment. Would it work if we generate a globally unique visible aliases for the static vars and use the alias' name to register device-side entities without changing their visibility? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D80858/new/ https://reviews.llvm.org/D80858

[PATCH] D80858: [CUDA][HIP] Support accessing static device variable in host code

yaxunl added a comment. In D80858#2170328 , @hliao wrote: > In D80858#2169534 , @yaxunl wrote: > > > Another reason is that we need to support it in rdc mode, where different > > TU can have static var with the sam

[PATCH] D80858: [CUDA][HIP] Support accessing static device variable in host code

yaxunl added a comment. In D80858#2170311 , @hliao wrote: > > 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 v

[PATCH] D80858: [CUDA][HIP] Support accessing static device variable in host code

hliao added a comment. In D80858#2169534 , @yaxunl wrote: > Another reason is that we need to support it in rdc mode, where different TU > can have static var with the same name. That's an issue of our current RDC support through LLVM IR instead of nati

[PATCH] D80858: [CUDA][HIP] Support accessing static device variable in host code

hliao added a comment. In D80858#2169399 , @yaxunl wrote: > In D80858#2169295 , @hliao wrote: > > > I don't that's proper way to support file-scope static device variables. As > > we discuss APIs like cudaMemcpyToS

[PATCH] D80858: [CUDA][HIP] Support accessing static device variable in host code

yaxunl added a comment. Another reason is that we need to support it in rdc mode, where different TU can have static var with the same name. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D80858/new/ https://reviews.llvm.org/D80858 ___ cfe-c

[PATCH] D80858: [CUDA][HIP] Support accessing static device variable in host code

yaxunl added a comment. In 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

[PATCH] D80858: [CUDA][HIP] Support accessing static device variable in host code

hliao requested changes to this revision. hliao added a comment. This revision now requires changes to proceed. 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 speci

[PATCH] D80858: [CUDA][HIP] Support accessing static device variable in host code

yaxunl added a comment. ping 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

[PATCH] D80858: [CUDA][HIP] Support accessing static device variable in host code

yaxunl updated this revision to Diff 277272. yaxunl marked 3 inline comments as done. yaxunl added a comment. Only allow cuid to be alphanumeric and underscore. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D80858/new/ https://reviews.llvm.org/D80858 Files: clang/include/clang/AST/AST

[PATCH] D80858: [CUDA][HIP] Support accessing static device variable in host code

yaxunl marked 9 inline comments as done. yaxunl added inline comments. Herald added a subscriber: dang. Comment at: clang/lib/AST/ASTContext.cpp:10068 +isa(D) && cast(D)->isFileVarDecl() && +cast(D)->getStorageClass() == SC_Static) { + return GVA_StrongExtern

[PATCH] D80858: [CUDA][HIP] Support accessing static device variable in host code

rjmccall added inline comments. Comment at: clang/lib/AST/ASTContext.cpp:10068 +isa(D) && cast(D)->isFileVarDecl() && +cast(D)->getStorageClass() == SC_Static) { + return GVA_StrongExternal; JonChesterfield wrote: > yaxunl wrote: > > rjmccall

[PATCH] D80858: [CUDA][HIP] Support accessing static device variable in host code

JonChesterfield added inline comments. Comment at: clang/lib/AST/ASTContext.cpp:10068 +isa(D) && cast(D)->isFileVarDecl() && +cast(D)->getStorageClass() == SC_Static) { + return GVA_StrongExternal; yaxunl wrote: > rjmccall wrote: > > Are you

[PATCH] D80858: [CUDA][HIP] Support accessing static device variable in host code

tra added inline comments. Comment at: clang/lib/CodeGen/CodeGenModule.cpp:6069 +llvm::raw_ostream &OS) const { + OS << ".static." << getLangOpts().CUID; +} I suspect that will have interesting issues if CUID is an arbitrary user-supplied string. We may wan

[PATCH] D80858: [CUDA][HIP] Support accessing static device variable in host code

yaxunl updated this revision to Diff 275685. yaxunl marked 5 inline comments as done. yaxunl retitled this revision from "[HIP] Support accessing static device variable in host code" to "[CUDA][HIP] Support accessing static device variable in host code". yaxunl edited the summary of this revision