[PATCH] D121466: [OpenMP] Replace math headers with OpenMP wrapper calls

jdoerfert added a comment. We want one copy of this, yes. s/omp/llvm_gpu/, sure, though gpu might also not the best name. Generators, macros, etc. I'm not sold. Even if we want to do this, let's not conflate that. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://revie

[PATCH] D121468: [OpenMP] Add linking of OpenMP math library

jdoerfert added inline comments. Comment at: clang/include/clang/Driver/Options.td:1056 +def libomptarget_nvptx_math_bc_path_EQ : Joined<["--"], "libomptarget-nvptx-math-bc-path=">, + Group, HelpText<"Path to libomptarget-nvptx math bitcode library">; def dD : Flag<["-"], "dD"

[PATCH] D120271: [Clang] Add offload kind to embedded offload object

jdoerfert accepted this revision. jdoerfert added a comment. This revision is now accepted and ready to land. Adjust commit message. LG, we can address outstanding comments for different encodings as we go. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.

[PATCH] D121837: [OpenMP][FIX] Allow device constructors for AMD GPU

jdoerfert created this revision. jdoerfert added a reviewer: JonChesterfield. Herald added subscribers: guansong, bollu, yaxunl, jvesely. Herald added a project: All. jdoerfert requested review of this revision. Herald added subscribers: cfe-commits, sstefan1. Herald added a project: clang. In AMD

[PATCH] D121837: [OpenMP][FIX] Allow device constructors for AMD GPU

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG07b176646134: [OpenMP][FIX] Allow device constructors for AMD GPU (authored by jdoerfert). Changed prior to commit: https://reviews.llvm.org/D1218

[PATCH] D121837: [OpenMP][FIX] Allow device constructors for AMD GPU

jdoerfert updated this revision to Diff 416218. jdoerfert added a comment. Fix AS(0) -> AS(0) cast error Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D121837/new/ https://reviews.llvm.org/D121837 Files: clang/lib/CodeGen/CGOpenMPRuntime.cpp cl

[PATCH] D121837: [OpenMP][FIX] Allow device constructors for AMD GPU

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGf02550bdd9b7: Reapply "[OpenMP][FIX] Allow device constructors for AMD GPU" (authored by jdoerfert). Changed prior to commit: https://reviews.llvm

[PATCH] D121837: [OpenMP][FIX] Allow device constructors for AMD GPU

jdoerfert added a comment. In D121837#3390042 , @thakis wrote: > Looks like this breaks tests: http://45.33.8.238/linux/71358/step_7.txt > > Please take a look and revert for now if it takes a while to fix. I relaxed the check line. Should resolve the pr

[PATCH] D121962: [clang] [OpenMP] Extend OpenMP variant declaration tests.

jdoerfert accepted this revision. jdoerfert added a comment. This revision is now accepted and ready to land. LG Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D121962/new/ https://reviews.llvm.org/D121962 ___

[PATCH] D121963: [clang] [OpenMP] Diagnose use of 'target_clones' in OpenMP variant declarations.

jdoerfert accepted this revision. jdoerfert added a comment. This revision is now accepted and ready to land. LG Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D121963/new/ https://reviews.llvm.org/D121963 ___

[PATCH] D121837: [OpenMP][FIX] Allow device constructors for AMD GPU

jdoerfert added a comment. In D121837#3392346 , @kamaub wrote: > The test cases added with this commit failed on clang-ppc64be-linux-lnt # > 13809 could you > please revert this change, and rec

[PATCH] D122089: [CUDA] Add getTargetFeatures for the NVPTX toolchain

jdoerfert added inline comments. Comment at: clang/lib/Driver/ToolChains/Cuda.cpp:635 + const llvm::opt::ArgList &Args, + std::vector &Features, + Optional CudaVersion) { ---

[PATCH] D122089: [CUDA] Add getTargetFeatures for the NVPTX toolchain

jdoerfert accepted this revision. jdoerfert added a comment. This revision is now accepted and ready to land. LG Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D122089/new/ https://reviews.llvm.org/D122089 ___

[PATCH] D122255: Meta directive runtime support

jdoerfert added subscribers: ggeorgakoudis, mikerice, cchen. jdoerfert added a comment. This contains a lot of unrelated changes, leftover comments, etc. As you clean up the code, please also include tests. Clang format the patch, go over the new code and make sure you follow the coding style. R

[PATCH] D122255: Meta directive runtime support

jdoerfert added a comment. In D122255#3400636 , @abidmalikwaterloo wrote: > Is it okay if I can put comments on the patch? Not sure what you mean but feel free. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.or

[PATCH] D122683: [OpenMP] Use new offloading binary when embedding offloading images

jdoerfert added inline comments. Comment at: clang/test/Frontend/embed-object.ll:6 +; CHECK: @[[OBJECT:.+]] = private constant [120 x i8] c"\10\FF\10\AD{{.*}}\00", section ".llvm.offloading", align 8 +; CHECK: @llvm.compiler.used = appending global [2 x i8*] [i8* @x, i8* getele

[PATCH] D122683: [OpenMP] Use new offloading binary when embedding offloading images

jdoerfert accepted this revision. jdoerfert added a comment. This revision is now accepted and ready to land. LG Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D122683/new/ https://reviews.llvm.org/D122683 ___

[PATCH] D113359: [Libomptarget][WIP] Introduce VGPU Plugin

jdoerfert accepted this revision. jdoerfert added a comment. This revision is now accepted and ready to land. LG, with some things to address before the merge though. Didn't we have a pass to expand shared memory (and such)? Comment at: clang/lib/Basic/TargetInfo.cpp:155 + +

[PATCH] D113359: [Libomptarget][WIP] Introduce VGPU Plugin

jdoerfert added a comment. We can merge runtime first, build it in isolation, then libomptarget host runtime, then clang. Also make sure to adjust the commit messages Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D113359/new/ https://reviews.llvm.

[PATCH] D116637: [Clang][Sema][OpenMP] Sema support for `atomic compare`

jdoerfert added inline comments. Comment at: clang/test/OpenMP/atomic_ast_print.cpp:964 + // CHECK-51-NEXT: } + // omp51-note@+1 {{in instantiation of function template specialization 'foo' requested here}} return foo(a); tianshilei1992 wrote: > I cannot fi

[PATCH] D118858: [OpenMP] Don't use bound architecture when checking cache on the host

jdoerfert accepted this revision. jdoerfert added a comment. This revision is now accepted and ready to land. LG Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D118858/new/ https://reviews.llvm.org/D118858 ___

[PATCH] D118934: [OpenMP] Completely remove old device runtime

jdoerfert added a comment. Don't backport. Deprecate the flag and that's fine. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D118934/new/ https://reviews.llvm.org/D118934 ___ cfe-commits mailing list cfe-

[PATCH] D116637: [Clang][Sema][OpenMP] Sema support for `atomic compare`

jdoerfert added a comment. In D116637#3297790 , @tianshilei1992 wrote: > In D116637#3297785 , @kda wrote: > >> This still seems to be breaking sanitizers fast: >> https://lab.llvm.org/buildbot/#/builders/5/build

[PATCH] D119216: [AMDGPU] replace hostcall module flag with function attribute

jdoerfert added inline comments. Comment at: llvm/lib/Target/AMDGPU/AMDGPUAttributor.cpp:566 + return false; +}; + You can use AAPointerInfo for the call site return IRPosition. It will (through the iterations) gather all accesses and put them into "bin

[PATCH] D119216: [AMDGPU] replace hostcall module flag with function attribute

jdoerfert added inline comments. Comment at: llvm/lib/Target/AMDGPU/AMDGPUAttributor.cpp:581-584 + if (ArgUsedToRetrieveHostcallPtr(I)) { +return false; + } + return true; I'd suggest to switch the return value of ArgUsed... so it matches t

[PATCH] D119216: [AMDGPU] replace hostcall module flag with function attribute

jdoerfert added inline comments. Comment at: llvm/lib/Target/AMDGPU/AMDGPUAttributor.cpp:566 + return false; +}; + jdoerfert wrote: > sameerds wrote: > > jdoerfert wrote: > > > You can use AAPointerInfo for the call site return IRPosition. It will > > >

[PATCH] D119216: [AMDGPU] replace hostcall module flag with function attribute

jdoerfert added inline comments. Comment at: llvm/lib/Target/AMDGPU/AMDGPUAttributor.cpp:566 + return false; +}; + sameerds wrote: > jdoerfert wrote: > > jdoerfert wrote: > > > sameerds wrote: > > > > jdoerfert wrote: > > > > > You can use AAPointerInfo

[PATCH] D119216: [AMDGPU] replace hostcall module flag with function attribute

jdoerfert requested changes to this revision. jdoerfert added a comment. This revision now requires changes to proceed. @kpyzhov Usually people don't accept without comment, especially if there is clearly ongoing discussion. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION htt

[PATCH] D119216: [AMDGPU] replace hostcall module flag with function attribute

jdoerfert added inline comments. Comment at: llvm/lib/Target/AMDGPU/AMDGPUAttributor.cpp:566 + return false; +}; + sameerds wrote: > jdoerfert wrote: > > sameerds wrote: > > > jdoerfert wrote: > > > > jdoerfert wrote: > > > > > sameerds wrote: > > > > >

[PATCH] D119216: [AMDGPU] replace hostcall module flag with function attribute

jdoerfert accepted this revision. jdoerfert added a comment. This revision is now accepted and ready to land. Attributor use looks good. @arsenm will need to look at the rest. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D119216/new/ https://review

[PATCH] D119613: [OpenMP] Add support for CPU offloading in new driver

jdoerfert added a comment. We should be able to test this with unit tests, no? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D119613/new/ https://reviews.llvm.org/D119613 ___ cfe-commits mailing list cfe-

[PATCH] D119841: [OpenMP] Pass AMDGPU math libraries into the linker wrapper

jdoerfert added inline comments. Comment at: clang/lib/Driver/ToolChains/Clang.cpp:8195 + // Get the AMDGPU math libraries. + // FIXME: This method is bad, remove once AMDGPU has a proper math library. + for (auto &I : llvm::make_range(OpenMPTCRange.first, OpenMPTCRange.second

[PATCH] D119613: [OpenMP] Add support for CPU offloading in new driver

jdoerfert accepted this revision. jdoerfert added a comment. This revision is now accepted and ready to land. LG Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D119613/new/ https://reviews.llvm.org/D119613 ___

[PATCH] D119841: [OpenMP] Pass AMDGPU math libraries into the linker wrapper

jdoerfert added a subscriber: arsenm. jdoerfert added inline comments. Comment at: clang/lib/Driver/ToolChains/Clang.cpp:8195 + // Get the AMDGPU math libraries. + // FIXME: This method is bad, remove once AMDGPU has a proper math library. + for (auto &I : llvm::make_range(Ope

[PATCH] D119841: [OpenMP] Pass AMDGPU math libraries into the linker wrapper

jdoerfert added inline comments. Comment at: clang/lib/Driver/ToolChains/Clang.cpp:8205 +if (llvm::find(LibraryArgs, "m") == LibraryArgs.end() && !D.CCCIsCXX()) + continue; + JonChesterfield wrote: > jhuber6 wrote: > > jdoerfert wrote: > > > I'd switch t

[PATCH] D119841: [OpenMP] Pass AMDGPU math libraries into the linker wrapper

jdoerfert accepted this revision. jdoerfert added a comment. This revision is now accepted and ready to land. LG, with or without the -lm lookup Comment at: clang/lib/Driver/ToolChains/Clang.cpp:8195 + // Get the AMDGPU math libraries. + // FIXME: This method is bad, remove o

[PATCH] D119979: [OpenMP] Diagnose bad 'omp declare variant' that references itself

jdoerfert reopened this revision. jdoerfert added a comment. This revision is now accepted and ready to land. Why is this supposed to be a user error? I don't remember the standard disallowing this and I can see use cases for this. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTIO

[PATCH] D119979: [OpenMP] Diagnose bad 'omp declare variant' that references itself

jdoerfert added a comment. In D119979#3330343 , @mikerice wrote: > What's the use case? I wasn't aware of any. We saw it from someone who did > it accidently and caused the compiler to crash in codegen. If you have N overloads/variants and you want to

[PATCH] D124158: [Clang][Attr] Skip adding noundef attribute to arguments when function has convergent attribute

jdoerfert added a comment. > For Ex: SimplifyCFG pass removes the branch leading to a BB which has an > incoming value that will always trigger undefined behavior. This basically > modifies the CFG and combines the basic blocks. This works for CPU execution. > But on a GPU, there are intrinsics

[PATCH] D124525: [OpenMP][ClangLinkerWrapper] Extending linker wrapper to embed metadata for multi-arch fat binaries

jdoerfert added inline comments. Comment at: openmp/libomptarget/include/omptarget.h:163 + int32_t image_number; // Image number in image library starting from 0 + int32_t number_images; // Number of images, used for initial allocation + char *offload_arch;//

[PATCH] D124158: [Clang][Attr] Skip adding noundef attribute to arguments when function has convergent attribute

jdoerfert added a comment. In D124158#3477649 , @efriedma wrote: > The issue you're describing sounds like it's specific to @__shfl_sync. In > general, in C++, you aren't allowed to read from an uninitialized variable; > see [basic.indet] in the standa

[PATCH] D120273: [OpenMP] Allow CUDA to be linked with OpenMP using the new driver

jdoerfert accepted this revision. jdoerfert added a comment. This revision is now accepted and ready to land. LG Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D120273/new/ https://reviews.llvm.org/D120273 ___

[PATCH] D124158: [Clang][Attr] Skip adding noundef attribute to arguments when function has convergent attribute

jdoerfert added a comment. In D124158#3480566 , @efriedma wrote: > In D124158#3480384 , @arsenm wrote: > >> I'm thinking noundef is a bit of red herring here. The real problem seems to >> be arising from the assu

[PATCH] D124724: [Clang][OpenMP] Add the support for floating-point variables for specific atomic clauses

jdoerfert accepted this revision. jdoerfert added a comment. This revision is now accepted and ready to land. In D124724#3486921 , @tianshilei1992 wrote: > always use `UIToFP` because that is the only case that `UpdateVal` is a > `ConstantInt`, where we

[PATCH] D125092: [OpenMP] Add basic support for properly handling static libraries

jdoerfert accepted this revision. jdoerfert added a comment. This revision is now accepted and ready to land. Typo(s) in the commit message, otherwise this seems reasonable. LG Comment at: clang/tools/clang-linker-wrapper/ClangLinkerWrapper.cpp:1153 + for (auto &File : Library

[PATCH] D123812: [CUDA] Add wrapper code generation for registering CUDA images

jdoerfert added a comment. Are there unresolved concerns we should address in this review? Comment at: clang/tools/clang-linker-wrapper/OffloadWrapper.cpp:351 +/// required for texture / surface / managed variables. +Function *createRegisterGlobalsFunction(Module &M) { +

[PATCH] D129536: [CUDA][FIX] Make shfl[_sync] for unsigned long long non-recursive

jdoerfert created this revision. jdoerfert added a reviewer: tra. Herald added subscribers: mattd, bollu, yaxunl. Herald added a project: All. jdoerfert requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. A copy-paste error caused UB in the def

[PATCH] D122255: Meta directive runtime support

jdoerfert added inline comments. Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:10852 +def err_omp_misplaced_default_clause : Error< + "misplaced default clause! Only one default clause is allowed in" + "metadirective in the end">; ABataev wrote:

[PATCH] D102107: [OpenMP] Codegen aggregate for outlined function captures

jdoerfert added a comment. It seems the buildbot didn't actually test this patch but an old one, still: The checks for this tests are not updated: target_teams_distribute_parallel_for_order_codegen.cpp target_in_reduction_codegen.cpp nvptx_lambda_capturing.cpp nvptx_lambda_pointer_capturing.c

[PATCH] D128907: [Clang] Disable noundef attribute for languages which allow uninitialized function arguments

jdoerfert requested changes to this revision. jdoerfert added a comment. This revision now requires changes to proceed. Wasn't there a discussion about this before, or some other patch? If so, could we please link such things (e.g., in the commit message) so people don't have to remember and fin

[PATCH] D129694: [OPENMP] Make declare target static global externally visible

jdoerfert added inline comments. Comment at: clang/test/OpenMP/target_update_messages.cpp:17 -static int y; -#pragma omp declare target(y) - -void yyy() { -#pragma omp target update to(y) // expected-error {{the host cannot update a declare target variable that is not external

[PATCH] D128907: [Clang] Disable noundef attribute for languages which allow uninitialized function arguments

jdoerfert added subscribers: RaviNarayanaswamy, Anastasia, tra, bader. jdoerfert added a comment. In D128907#3652077 , @arsenm wrote: > In D128907#3650750 , @jdoerfert > wrote: > >> That said, I doubt this is eve

[PATCH] D128907: [Clang] Disable noundef attribute for languages which allow uninitialized function arguments

jdoerfert added a comment. @arsenm put this on the GPU working group agenda for tomorrow: https://docs.google.com/document/d/1m_oSe1HwtWdQ2JUmMRTAVHbUS7Dv4MRsqptiYcgK6iI/edit?usp=sharing Join in if you can! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.

[PATCH] D129536: [CUDA][FIX] Make shfl[_sync] for unsigned long long non-recursive

jdoerfert updated this revision to Diff 445864. jdoerfert added a comment. Address comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D129536/new/ https://reviews.llvm.org/D129536 Files: clang/lib/Headers/__clang_cuda_intrinsics.h clang/tes

[PATCH] D129536: [CUDA][FIX] Make shfl[_sync] for unsigned long long non-recursive

jdoerfert added a comment. @tra, unsure about the crash. For me this passes fine (no gpu), is anything missing? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D129536/new/ https://reviews.llvm.org/D129536 ___

[PATCH] D129536: [CUDA][FIX] Make shfl[_sync] for unsigned long long non-recursive

jdoerfert added a comment. In D129536#3666257 , @tra wrote: > In D129536#3663957 , @jdoerfert > wrote: > >> @tra, unsure about the crash. For me this passes fine (no gpu), is anything >> missing? > > The tests i

[PATCH] D129536: [CUDA][FIX] Make shfl[_sync] for unsigned long long non-recursive

jdoerfert updated this revision to Diff 446285. jdoerfert added a comment. Address comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D129536/new/ https://reviews.llvm.org/D129536 Files: clang/lib/Headers/__clang_cuda_intrinsics.h clang/tes

[PATCH] D129536: [CUDA][FIX] Make shfl[_sync] for unsigned long long non-recursive

jdoerfert added a comment. In D129536#3666884 , @tra wrote: > In D129536#3666860 , @jdoerfert > wrote: > >> The assertion is arguably not great but doesn't really matter, does it? How >> would I detect if they a

[PATCH] D129536: [CUDA][FIX] Make shfl[_sync] for unsigned long long non-recursive

jdoerfert updated this revision to Diff 446286. jdoerfert added a comment. Use <...> Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D129536/new/ https://reviews.llvm.org/D129536 Files: clang/lib/Headers/__clang_cuda_intrinsics.h clang/test/CodeG

[PATCH] D129536: [CUDA][FIX] Make shfl[_sync] for unsigned long long non-recursive

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG48d6f5240187: [CUDA][FIX] Make shfl[_sync] for unsigned long long non-recursive (authored by jdoerfert). Repository: rG LLVM Github Monorepo CHAN

[PATCH] D130224: [Clang][Attribute] Introduce maybe_undef attribute for function arguments which accepts undef values

jdoerfert added a comment. Generating freeze seems an option, just not sure we can easily get all locations with this approach. Anyway, some code comments below. Comment at: clang/lib/CodeGen/CGCall.cpp:2061 +} + } + return ArgHasMayBeUndefAttr; Early ex

[PATCH] D130224: [Clang][Attribute] Introduce maybe_undef attribute for function arguments which accepts undef values

jdoerfert added a comment. > I'm still not seeing the issue fully. My understanding of the situation > (which may be wrong) is that Clang lowers to LLVM IR and adds noundef > markings at call sites that this patch attempts to let the user then undo. > However, why can Clang's CodeGen not notice

[PATCH] D125165: [Clang] Introduce clang-offload-binary tool to bundle device files

jdoerfert added a comment. Conceptually fine with me, @tra? Comment at: clang/docs/ClangOffloadBinary.rst:8 + +.. _clang-offload-binary: + jhuber6 wrote: > tra wrote: > > Naming nit: `binary` may not be the best term for what we're trying to do > > here. Perha

[PATCH] D125314: [OpenMP] Don't set device runtime debugging flags if using '-nogpulib'

jdoerfert added a comment. Not actually tested, is it? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D125314/new/ https://reviews.llvm.org/D125314 ___ cfe-commits mailing list cfe-commits@lists.llvm.org h

[PATCH] D125314: [OpenMP] Don't set device runtime debugging flags if using '-nogpulib'

jdoerfert accepted this revision. jdoerfert added a comment. This revision is now accepted and ready to land. LG Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D125314/new/ https://reviews.llvm.org/D125314 ___

[PATCH] D125378: [Attribute] Introduce shuffle attribute to be used for __shfl_sync like cross-lane APIs

jdoerfert requested changes to this revision. jdoerfert added a comment. In D125378#3506001 , @nikic wrote: > Please specify the semantics of the new LLVM attribute in LangRef -- though I > don't really understand why you need an LLVM-side attribute at a

[PATCH] D125378: [Attribute] Introduce shuffle attribute to be used for __shfl_sync like cross-lane APIs

jdoerfert added a comment. In D125378#3506383 , @skc7 wrote: > In D125378#3506215 , @jdoerfert > wrote: > >> In D125378#3506001 , @nikic wrote: >> >>> Please specify the

[PATCH] D125378: [Attribute] Introduce shuffle attribute to be used for __shfl_sync like cross-lane APIs

jdoerfert added a comment. In D125378#3506954 , @aaron.ballman wrote: > In D125378#3506446 , @jdoerfert > wrote: > >> What I would suggest, if you want to use C/C++ attributes (which makes sense >> to me), is a

[PATCH] D125333: [OpenMP] Use the new OpenMP device static library when doing LTO

jdoerfert accepted this revision. jdoerfert added a comment. This revision is now accepted and ready to land. LG, nice! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D125333/new/ https://reviews.llvm.org/D125333

[PATCH] D125705: [OpenMP] Don't build the offloading driver without a source input

jdoerfert accepted this revision. jdoerfert added a comment. This revision is now accepted and ready to land. LG Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D125705/new/ https://reviews.llvm.org/D125705 ___

[PATCH] D66673: [OPENMP][NVPTX]Fix critical region codegen.

jdoerfert added a comment. I guess IR test should be affected already and it would be good to have the run time test that breaks with barriers. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66673/new/ https://reviews.llvm.org/D66673 __

[PATCH] D66673: [OPENMP][NVPTX]Fix critical region codegen.

jdoerfert accepted this revision. jdoerfert added a comment. This revision is now accepted and ready to land. LGTM Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66673/new/ https://reviews.llvm.org/D66673 ___ cfe-comm

[PATCH] D59922: [Attributor] Deduce "no-capture" argument attribute

jdoerfert updated this revision to Diff 217306. jdoerfert added a comment. rebase and update tests Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59922/new/ https://reviews.llvm.org/D59922 Files: llvm/include/llvm/Transforms/IPO/Attributor.h ll

[PATCH] D64375: [OpenMP][Docs] Provide implementation status details

jdoerfert updated this revision to Diff 217631. jdoerfert marked 2 inline comments as done. jdoerfert added a comment. Update according to comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D64375/new/ https://reviews.llvm.org/D64375 Files: c

[PATCH] D59922: [Attributor] Deduce "no-capture" argument attribute

jdoerfert updated this revision to Diff 218109. jdoerfert added a comment. Update tests Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59922/new/ https://reviews.llvm.org/D59922 Files: llvm/include/llvm/Transforms/IPO/Attributor.h llvm/lib/Tran

[PATCH] D59922: [Attributor] Deduce "no-capture" argument attribute

jdoerfert marked 6 inline comments as done. jdoerfert added inline comments. Comment at: llvm/lib/Transforms/IPO/Attributor.cpp:2618 +else + Attrs.emplace_back(Attribute::get(Ctx, "no-capture-maybe-returned")); + } uenoku wrote: > Maybe we need an exact

[PATCH] D59922: [Attributor] Deduce "no-capture" argument attribute

This revision was automatically updated to reflect the committed changes. jdoerfert marked an inline comment as done. Closed by commit rL370817: [Attributor] Deduce "no-capture" argument attribute (authored by jdoerfert, committed by ). Changed prior to commit: https://reviews.llvm.org/D59922?v

[PATCH] D64375: [OpenMP][Docs] Provide implementation status details

jdoerfert marked an inline comment as done. jdoerfert added inline comments. Comment at: clang/docs/OpenMPSupport.rst:204 ++--+--+--+---

[PATCH] D64375: [OpenMP][Docs] Provide implementation status details

This revision was automatically updated to reflect the committed changes. jdoerfert marked an inline comment as done. Closed by commit rL370930: [OpenMP][Docs] Provide implementation status details (authored by jdoerfert, committed by ). Herald added a project: LLVM. Herald added a subscriber: llv

[PATCH] D58531: [clang] Specify type of pthread_create builtin

jdoerfert accepted this revision. jdoerfert added a comment. This revision is now accepted and ready to land. I like the patch and I think it is fine. Small nits: Could we have a test for " we can detect dodgy pthread_create declarations" and maybe `pthread[_attr]_t`? There is an unresolved com

[PATCH] D71948: [OpenMP] Use the OpenMPIRBuilder for `cancel` directives

jdoerfert created this revision. jdoerfert added reviewers: kiranchandramohan, ABataev, RaviNarayanaswamy, gtbercea, grokos, sdmitriev, JonChesterfield, fghanim. Herald added subscribers: guansong, bollu, hiraditya. Herald added a project: LLVM. jdoerfert added a child revision: D70290: [OpenMP] U

[PATCH] D70290: [OpenMP] Use the OpenMPIRBuilder for "omp parallel"

jdoerfert updated this revision to Diff 235449. jdoerfert added a comment. Rebased on D71948 and combined with OpenMPIRBuilder cancel code gen Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D70290/new/ https://revie

[PATCH] D71969: [OpenMP] diagnose zero-length array section in the depend clause

jdoerfert added a comment. I would put this check right next to the one that issues `err_omp_section_length_negative`. SemaExpr.cpp +4668 CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71969/new/ https://reviews.llvm.org/D71969 ___ cfe-comm

[PATCH] D71948: [OpenMP] Use the OpenMPIRBuilder for `cancel` directives

jdoerfert marked 2 inline comments as done. jdoerfert added inline comments. Comment at: llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp:241 + // This seems to be used only once without much change of reuse, could live in + // OMPKinds.def but seems not necessary. + Value *CancelKi

[PATCH] D70290: [OpenMP] Use the OpenMPIRBuilder for "omp parallel"

This revision was automatically updated to reflect the committed changes. Closed by commit rG10fedd94b432: [OpenMP] Use the OpenMPIRBuilder for `omp parallel` (authored by jdoerfert). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D70290/new/ https://

[PATCH] D71948: [OpenMP] Use the OpenMPIRBuilder for `cancel` directives

This revision was automatically updated to reflect the committed changes. jdoerfert marked an inline comment as done. Closed by commit rG000c6a5038bc: [OpenMP] Use the OpenMPIRBuilder for `omp cancel` (authored by jdoerfert). Changed prior to commit: https://reviews.llvm.org/D71948?vs=235448&id

[PATCH] D72304: Summary: Add OpenMP Directives (master and critical) to OMPBuilder, and use them in clang.

jdoerfert added a comment. Some initial comments. Can we split it in two patches (master/critical)? Comment at: llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h:250 + llvm::ArrayType *KmpCriticalNameTy; + llvm::PointerType *KmpCriticalNamePtrTy; + I

[PATCH] D72304: Summary: Add OpenMP Directives (master and critical) to OMPBuilder, and use them in clang.

jdoerfert added inline comments. Comment at: llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h:250 + llvm::ArrayType *KmpCriticalNameTy; + llvm::PointerType *KmpCriticalNamePtrTy; + fghanim wrote: > jdoerfert wrote: > > If there is no good reason again

[PATCH] D72304: Summary: Add OpenMP Directives (master and critical) to OMPBuilder, and use them in clang.

jdoerfert added inline comments. Comment at: llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp:845 + default: + assert(false && "unknown OMP directive"); + While I hope this code goes away, I should mention that we don't write `assert(false && "...")

[PATCH] D72304: Summary: Add OpenMP Directives (master and critical) to OMPBuilder, and use them in clang.

jdoerfert added a comment. I added quite a bunch of inline comments again :( and some high level stuff below. --- Commits need a message that explains the change. Also the "Summary" should not be in the commit title. --- This adds support for clang to build the directives via the OMPIRBuilder

[PATCH] D72304: Summary: Add OpenMP Directives (master and critical) to OMPBuilder, and use them in clang.

jdoerfert added a comment. In D72304#1815793 , @fghanim wrote: > > Commits need a message that explains the change. > > Also the "Summary" should not be in the commit title. > > Is this something for future commits or do you want me to change these? > If

[PATCH] D72304: [OpenMP]{OMPIRBuilder] Add Directives (master and critical) to OMPBuilder.

jdoerfert accepted this revision. jdoerfert added a comment. This revision is now accepted and ready to land. LGTM with some minor things you should address before the merge. Comment at: llvm/include/llvm/Frontend/OpenMP/OMPConstants.h:90 +// extern ArrayType *KmpCriticalNameTy

[PATCH] D76211: OpenMP Metadirective with user defined condition

jdoerfert added a comment. Some comments but I haven't gone through all of it. The tests are still missing. Comment at: clang/lib/Sema/SemaOpenMP.cpp:6334 + + for (auto i = Clauses.rbegin(); i < Clauses.rend(); i++) { +OMPWhenClause *WhenClause = dyn_cast(*i);

[PATCH] D83004: [UpdateCCTestChecks] Include generated functions if asked

jdoerfert added a comment. In D83004#2274573 , @greened wrote: > In D83004#2168427 , @jdoerfert wrote: > >> Due to D82995 I realized we should have a >> test of this in `llvm/test/

[PATCH] D85876: [OpenMP][FIX] Do not drop a '$' while demangling declare variant names

This revision was automatically updated to reflect the committed changes. Closed by commit rG05fd04eda4b2: [OpenMP][FIX] Do not drop a '$' while demangling declare variant names (authored by jdoerfert). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D8

[PATCH] D85875: [OpenMP][FIX] Do not crash trying to print a missing (demangled) user condition

This revision was automatically updated to reflect the committed changes. Closed by commit rG6a02932becae: [OpenMP][FIX] Do not crash trying to print a missing (demangled) user condition (authored by jdoerfert). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.ll

[PATCH] D85777: [OpenMP] Support std::complex math functions in target regions

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG56069b5c71ca: [OpenMP] Support `std::complex` math functions in target regions (authored by jdoerfert). Repository: rG LLVM Github Monorepo CHANG

[PATCH] D85735: [OpenMP] Context selector extensions for template functions

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. jdoerfert marked 2 inline comments as done. Closed by commit rG5c1084e8840b: [OpenMP] Context selector extensions for template functions (authored by jdoerfert). Repos

[PATCH] D85879: [OpenMP] Overload `std::isnan` and friends multiple times for the GPU

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG97652202d1e6: [OpenMP] Overload `std::isnan` and friends multiple times for the GPU (authored by jdoerfert). Changed prior to commit: https://revi

[PATCH] D85877: [OpenMP] Support nested OpenMP context selectors (declare variant)

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG5c63ae156e96: [OpenMP] Support nested OpenMP context selectors (declare variant) (authored by jdoerfert). Repository: rG LLVM Github Monorepo CHA

<    2   3   4   5   6   7   8   9   10   11   >