jhuber6 added a subscriber: ronlieb. jhuber6 added a comment. In D117362#3254681 <https://reviews.llvm.org/D117362#3254681>, @JonChesterfield wrote:
> If I'm following correctly, this broke the amdgpu buildbot and it has been > moved into staging as a workaround. I haven't debugged what breaks but > 'DefaultVisibility' is relatively likely to behave differently on cuda vs hsa > systems so that's my first guess. Yes, this patch is necessary because of the different handling of `hidden` visibility by NVIDIA and AMDGPU. Without this patch we cannot run any code that uses `#pragma omp declare target` on AMDGPU because they are all hidden. The runtime will try to load it and fail so we will crash. ================ Comment at: clang/lib/AST/Decl.cpp:792 + if (OMPDeclareTargetDeclAttr::isDeclareTargetDeclaration(Var)) + return LinkageInfo::external(); + ---------------- JonChesterfield wrote: > Would this change static variables to non-static and thus introduce multiple > definition errors? Not immediately obvious to me that the variables have to > be directly visible to other translation units That was my first guess, I had @ronlieb run an alternate approach where we run everything as normal, but at the end we inherit the linkage but set the visibility to default. he said it still caused problems. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D117362/new/ https://reviews.llvm.org/D117362 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
