rjmccall added a comment.
In addition to everyone else's concerns about generating this number randomly,
it's also inherently less testable.
I'm sure there's something else you could use that's reasonably likely to be
unique, like a hash of the input filename or output filename or a combination
of the two.
================
Comment at: clang/include/clang/Driver/Action.h:216
const llvm::opt::Arg &Input;
-
+ unsigned Id;
virtual void anchor();
----------------
Allowing an arbitrary string might be more adaptable.
================
Comment at: clang/lib/AST/ASTContext.cpp:10064
return GVA_StrongODR;
+ // Externalize device side static file-scope variable for HIP.
+ if (Context.getLangOpts().HIP && Context.getLangOpts().HIPCUID &&
----------------
This needs to be a clearer statement of why this is necessary.
================
Comment at: clang/lib/AST/ASTContext.cpp:10068
+ isa<VarDecl>(D) && cast<VarDecl>(D)->isFileVarDecl() &&
+ cast<VarDecl>(D)->getStorageClass() == SC_Static) {
+ return GVA_StrongExternal;
----------------
Are you sure this doesn't apply to e.g. local statics? Can't you have kernel
lambdas, or am I confusing HIP with another language?
================
Comment at: clang/lib/CodeGen/CodeGenModule.cpp:1094
+ if ((VD->hasAttr<CUDADeviceAttr>() || VD->hasAttr<CUDAConstantAttr>()) &&
+ VD->isFileVarDecl() && VD->getStorageClass() == SC_Static) {
+ Out << ".hip.static." << CGM.getLangOpts().HIPCUID;
----------------
Please extract this "(device || constant) && file && static" predicate instead
of repeating it in three different places.
================
Comment at: clang/lib/Driver/Action.cpp:170
+ if (!Id)
+ Id = llvm::sys::Process::GetRandomNumber();
+}
----------------
I'm sure GetRandomNumber can return 0, so this logic is faulty.
This also seems like an unfortunate intrusion of HIP-specific semantics on the
rest of the driver. Maybe HIP can generate the shared id when it's setting up
and cloning the job.
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
