[PATCH] D159118: [libc] Implement the 'clock()' function on the GPU

jdoerfert added inline comments. Comment at: libc/src/time/gpu/clock.cpp:61-62 +return clock_t(ticks / (GPU_CLOCKS_PER_SEC / CLOCKS_PER_SEC)); + else +return clock_t(ticks * (CLOCKS_PER_SEC / GPU_CLOCKS_PER_SEC)); +} Repository: rG LLVM Github

[PATCH] D153924: [OpenMP] Allow exceptions in target regions when offloading to GPUs

jdoerfert accepted this revision. jdoerfert added a comment. This revision is now accepted and ready to land. LG, see below. Comment at: clang/lib/Sema/SemaExprCXX.cpp:869 bool IsThrownVarInScope) { - // Don't report an error if 'throw' is used

[PATCH] D158383: [OpenMP] Add NVIDIA annotations for static grid thread limit

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGf3958ce0086a: [OpenMP] Add NVIDIA annotations for static grid thread limit (authored by jdoerfert). Herald added a project: clang. Herald added a

[PATCH] D158381: [OpenMP] Properly set static thread limit (w/o analysis)

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGc5488c8dcc8c: [OpenMP] Properly set static thread limit (w/o analysis) (authored by jdoerfert). Herald added projects: clang, OpenMP. Herald added

[PATCH] D153924: [OpenMP] Allow exceptions in target regions when offloading to GPUs

jdoerfert added a comment. I think this is basically done, one question though. Comment at: clang/test/OpenMP/amdgpu_exceptions.cpp:11 +// RUN: %clang_cc1 -fopenmp -triple amdgcn-amd-amdhsa -fopenmp-is-target-device -fexceptions %s -emit-llvm -S -verify=with

[PATCH] D157994: [OpenMP] Migrate dispatch related utility functions from Clang codegen to OMPIRBuilder

jdoerfert accepted this revision. jdoerfert added a comment. This revision is now accepted and ready to land. LG, but do not remove the Attribute from our set, add it to the tests. The functions are nounwind. Comment at: llvm/include/llvm/Frontend/OpenMP/OMPKinds.def:532

[PATCH] D156901: [OpenMP] Change OpenMP default version in documentation and help text for -fopenmp-version

jdoerfert added inline comments. Comment at: flang/test/Driver/driver-help.f90:55 ! HELP-NEXT: -fopenmp-version= -! HELP-NEXT:Set OpenMP version (e.g. 45 for OpenMP 4.5, 50 for OpenMP 5.0). Default value is 50 for Clang and 11 for Flang +! HELP-NEXT:

[PATCH] D156816: [Clang] Make generic aliases to OpenCL address spaces

jdoerfert added a comment. In D156816#4551417 , @jhuber6 wrote: > In D156816#4551409 , @Anastasia > wrote: > >> Why not to just use target address space and define it to some macro with >> desirable spelling?

[PATCH] D156641: [Clang][Frontend] Change help text for --offload-host-device

jdoerfert added a comment. In D156641#4547718 , @AntonRydahl wrote: > The tests that failed pass locally on my machine. If the tests are clearly not related to your change, assume they are broken before or flaky. Repository: rG LLVM Github

[PATCH] D116910: [OpenMP][3/3] Introduce the KernelEnvironment into Clang tests

jdoerfert closed this revision. jdoerfert added a comment. Herald added subscribers: jplehr, sunshaoce, mattd. Herald added a project: All. Subsumed by D142569 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D155213: [HIP] Add `-fno-offload-uniform-block`

jdoerfert added a comment. This should be named -foffload*, it should not use HIP in the description, and it should apply to OpenMP as well. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D155213/new/ https://reviews.llvm.org/D155213 ___

[PATCH] D155794: [OpenMP][OpenMPIRBuilder] Migrate setPropertyExecutionMode() from Clang to OpenMPIRBuilder.

jdoerfert accepted this revision. jdoerfert added a comment. This revision is now accepted and ready to land. LG, address the nits below please. Comment at: clang/lib/CodeGen/CodeGenModule.h:1017 +return LLVMCompilerUsed; + } + Unused now.

[PATCH] D154568: [Clang][OpenMP] GPU simd directive code generation

jdoerfert added a comment. In D154568#4528274 , @efwright wrote: > Dropping off a simple test case. If this looks about what you would expect > for the tests I have a couple more involved ones that I can repurpose and add > in. For more complex tests

[PATCH] D156184: [OpenMP] Add the `ompx_attribute` clause for target directives

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGef9ec4bbcca2: [OpenMP] Add the `ompx_attribute` clause for target directives (authored by jdoerfert). Herald added a project: clang. Herald added a

[PATCH] D148216: [UTC] Add fallback support for specific metadata, and check their defs

jdoerfert added a comment. In D148216#4486400 , @jdoerfert wrote: > I might pull this later today until a fix is available. It makes updates of > files with globals super cumbersome (you need to manually remove the > duplicates). FWIW, the duplication

[PATCH] D155794: [OpenMP][OpenMPIRBuilder] Migrate setPropertyExecutionMode() from Clang to OpenMPIRBuilder.

jdoerfert added a comment. Mostly nits. One problem I have is the vector. I do not believe Flang will use the same for their "llvm.used" tracking. I think it makes more sense to provide a callback or just return the global and let the caller handle it. Comment at:

[PATCH] D155606: [Clang] Only emit CUDA version warnings when creating the CUDA toolchain

jdoerfert added a comment. tests? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D155606/new/ https://reviews.llvm.org/D155606 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D155606: [Clang] Only emit CUDA version warnings when creating the CUDA toolchain

jdoerfert added a comment. I like it Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D155606/new/ https://reviews.llvm.org/D155606 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D152554: [OpenMP] Migrate device code privatisation from Clang CodeGen to OMPIRBuilder

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/D152554/new/ https://reviews.llvm.org/D152554

[PATCH] D148216: [UTC] Add fallback support for specific metadata, and check their defs

jdoerfert requested changes to this revision. jdoerfert added a comment. This revision now requires changes to proceed. I might pull this later today until a fix is available. It makes updates of files with globals super cumbersome (you need to manually remove the duplicates). Repository:

[PATCH] D148216: [UTC] Add fallback support for specific metadata, and check their defs

jdoerfert reopened this revision. jdoerfert added a comment. This revision is now accepted and ready to land. This is broken: https://github.com/llvm/llvm-project/issues/63746 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D148216/new/

[PATCH] D148216: [UTC] Add fallback support for specific metadata, and check their defs

jdoerfert closed this revision. jdoerfert added a comment. When you commit things, can you please add the Differential Revision into the commit message so it is closed here and we can easily find the review for a change. Committed as 8a3fdf7b908978625e9a7e57fbb443e4e6f98976

[PATCH] D153725: [clang] Make amdgpu-arch tool work on Windows

jdoerfert added inline comments. Comment at: clang/tools/amdgpu-arch/AMDGPUArchByHIP.cpp:96 + return 0; +} Where is the call to this? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D153725/new/ https://reviews.llvm.org/D153725

[PATCH] D154568: [Clang][OpenMP] GPU simd directive code generation

jdoerfert added inline comments. Comment at: llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp:1532 + +} + This is in large parts copied from existing code. Can we extract those parts into helper functions instead of duplicating them? Repository: rG LLVM Github

[PATCH] D154568: [Clang][OpenMP] GPU simd directive code generation

jdoerfert added a comment. Do you have tests? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D154568/new/ https://reviews.llvm.org/D154568 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D154495: clang: Attach !fpmath metadata to __builtin_sqrt based on language flags

jdoerfert added a comment. FWIW, I assume we want this also for OpenMP offload. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D154495/new/ https://reviews.llvm.org/D154495 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D154672: [OPENMP52] Deprecation of 'depend' clause in ordered directive.

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/D154672/new/ https://reviews.llvm.org/D154672

[PATCH] D154591: [OpenMP][OMPIRBuilder] Rename IsEmbedded and IsTargetCodegen flags

jdoerfert added a subscriber: jhuber6. jdoerfert added a comment. Works for me. Tag @jhuber6 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D154591/new/ https://reviews.llvm.org/D154591 ___ cfe-commits

[PATCH] D154036: [libc] Add support for creating wrapper headers for offloading in clang

jdoerfert accepted this revision. jdoerfert added a comment. If it compiles, ship it. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D154036/new/ https://reviews.llvm.org/D154036 ___ cfe-commits mailing

[PATCH] D152054: [OpenMP] Codegen support for thread_limit on target directive

jdoerfert added inline comments. Comment at: openmp/runtime/test/target/target_thread_limit.cpp:28 +// OMP51: target: parallel +// OMP51: target: parallel + This doesn't check much. You need to verify a 5th or print the team size. Same for the rest

[PATCH] D154207: [AMDGPU] Rename predefined macro __AMDGCN_WAVEFRONT_SIZE

jdoerfert added a comment. I think this is a step in the right direction. @arsenm ? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D154207/new/ https://reviews.llvm.org/D154207 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D154133: [amdgpu] start documenting amdgpu support by clang

jdoerfert added a comment. I would recommend to introduce `__AMDGCN_WAVEFRONT_SIZE__` as an alias for `'__AMDGCN_WAVEFRONT_SIZE` and at some point to deprecate the latter. Otherwise, LGTM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D154133/new/ https://reviews.llvm.org/D154133

[PATCH] D152554: [OpenMP] Migrate deviice code privatization from Clang CodeGen to OMPIRBuilder

jdoerfert added a comment. typo in the title. Comment at: clang/lib/CodeGen/CGOpenMPRuntime.h:1444 +/// alloca address where the runtime returns the device pointers. +llvm::DenseMap CaptureDeviceAddrMap; }; If it is an alloca (for sure) use

[PATCH] D150860: [OpenMP] Change clang emitTargetDataCalls to use OMPIRBuilder

jdoerfert accepted this revision. jdoerfert added a comment. This revision is now accepted and ready to land. Let's move on. I described what this should look like, just for the record. Comment at: clang/lib/CodeGen/CGOpenMPRuntime.cpp:10399 +DeviceID =

[PATCH] D154133: [amdgpu] start documenting amdgpu support by clang

jdoerfert added inline comments. Comment at: clang/docs/AMDGPUSupport.rst:33 + * - ``__AMD__`` + - Indicates that the code is being compiled for an AMD GPU. + * - ``__AMDGPU__`` FWIW, this arguably confusing use of __AMD__, and duplication of __AMDGPU__,

[PATCH] D153924: Allowing exception handling in OpenMP target regions when offloading to AMDGCN or NVPTX targets

jdoerfert added inline comments. Comment at: clang/include/clang/Basic/DiagnosticCommonKinds.td:432 + "Target '%0' does not support exception handling." + " To allow code generation for '%0', 'catch' statement will be replaced by a no operation instruction.">; }

[PATCH] D144634: [Clang][OpenMP] Support for Code Generation of loop bind clause

jdoerfert added a comment. @ABataev wdyt? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D144634/new/ https://reviews.llvm.org/D144634 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D148216: Add support for annotations in UpdateTestChecks (NFC)

jdoerfert added a comment. could you put a little more information in the commit message please. "It won't do X when we do Y", could mean a lot of things. We don't do Y anymore, or we do X' now, with various choices for X'. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D150860: [OpenMP] Change clang emitTargetDataCalls to use OMPIRBuilder

jdoerfert added inline comments. Comment at: clang/test/OpenMP/target_data_codegen.cpp:355-356 // Region 00 +// CK2-DAG: [[DEV:%[^,]+]] = sext i32 [[DEVi32:%[^,]+]] to i64 +// CK2-DAG: [[DEVi32]] = load i32, ptr %{{[^,]+}}, // CK2: br i1 %{{[^,]+}}, label %[[IFTHEN:[^,]+]],

[PATCH] D153123: Fix diag for read-only target features

jdoerfert added inline comments. Comment at: clang/test/OpenMP/driver-openmp-amdgpu.c:8 +// RUN: --offload-device-only -o - 2>&1 | FileCheck --check-prefix=CHECK %s +// CHECK-NOT: warning: feature flag {{.*}} is ignored since the feature is read only yaxunl

[PATCH] D150860: [OpenMP] Change clang emitTargetDataCalls to use OMPIRBuilder

jdoerfert added a comment. This looks pretty good. The device vs if clause thing should be fixed while we are here. Comment at: clang/lib/CodeGen/CGOpenMPRuntime.cpp:10399 +DeviceID = CGF.Builder.getInt64(OMP_DEVICEID_UNDEF); + } Move this behind the

[PATCH] D146557: [MLIR][OpenMP] Refactoring createTargetData in OMPIRBuilder

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/D146557/new/ https://reviews.llvm.org/D146557

[PATCH] D146557: [MLIR][OpenMP] Refactoring createTargetData in OMPIRBuilder

jdoerfert added a comment. I think this is now style wise pretty good, I still found some potential problems below. Comment at: llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h:1487 + // possible, or else at the end of the function. + void emitBlock(BasicBlock *BB, Function

[PATCH] D152226: [FunctionAttrs] Propagate some func/arg/ret attributes from caller to callsite (WIP)

jdoerfert added a comment. FWIW, I wish the time that went into this would have been to improve and enable the (lightweight) Attributor pass. Anyway, I'm still doing that, and the latest numbers didn't look that bad. I will start to upstream stuff that I found. At this point it might be good to

[PATCH] D146557: [MLIR][OpenMP] Refactoring createTargetData in OMPIRBuilder

jdoerfert added inline comments. Comment at: llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h:1487 + // possible, or else at the end of the function. + void emitBlock(BasicBlock *BB, Function *CurFn, bool IsFinished = false); + This does not mention the

[PATCH] D146557: [MLIR][OpenMP] Refactoring createTargetData in OMPIRBuilder

jdoerfert added inline comments. Comment at: llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h:1365 + // possible, or else at the end of the function. + void EmitBlock(BasicBlock *BB, Function *CurFn, bool IsFinished = false); + why are these capitalized?

[PATCH] D149872: [OpenMP][OMPIRBuilder] Migrate emitOffloadingArrays and EmitNonContiguousDescriptor from Clang

jdoerfert accepted this revision. jdoerfert added a comment. This revision is now accepted and ready to land. If this passes all our tests, it looks fine. A few nits below, try to address if possible. Comment at: llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp:4427 +

[PATCH] D149162: [Clang][OpenMP][IRBuilder] Move registerTargetGlobalVariable & getAddrOfDeclareTargetVar into the OMPIRBuilder

jdoerfert accepted this revision. jdoerfert added a comment. I browsed it, looks fine. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D149162/new/ https://reviews.llvm.org/D149162 ___ cfe-commits mailing

[PATCH] D150608: [clang] Convert several OpenMP tests to opaque pointers

jdoerfert accepted this revision. jdoerfert added a comment. This revision is now accepted and ready to land. LG, assuming these are stable now. We should move them to the update scripts though... Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D150608: [clang] Convert several OpenMP tests to opaque pointers

jdoerfert added a comment. Did you manually update these tests? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D150608/new/ https://reviews.llvm.org/D150608 ___ cfe-commits mailing list

[PATCH] D147321: [RFC][Flang][OMPIRBuilder] Add nounwind attribute to the LLVM IR

jdoerfert added inline comments. Comment at: llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp:5063 + } +} + Style `F` Comment at: llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp:5063 + } +} + jdoerfert wrote: > Style `F` Why does this

[PATCH] D149666: [OpenMP][OMPIRBuilder] Migrate MapCombinedInfoTy from Clang to OpenMPIRBuilder

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/D149666/new/ https://reviews.llvm.org/D149666

[PATCH] D149666: [OpenMP][OMPIRBuilder] Migrate MapCombinedInfoTy from Clang to OpenMPIRBuilder

jdoerfert added inline comments. Comment at: clang/lib/CodeGen/CGOpenMPRuntime.cpp:6848 + class MapCombinedInfoTy : public llvm::OpenMPIRBuilder::MapCombinedInfoTy { + public: MapExprsArrayTy Exprs; TIFitis wrote: > jdoerfert wrote: > > Not sure why you

[PATCH] D148370: [Clang][Flang][OpenMP] Add loadOffloadInfoMetadata and createOffloadEntriesAndInfoMetadata into OMPIRBuilder's finalize and initialize

jdoerfert added inline comments. Comment at: llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp:561 + + llvm::OpenMPIRBuilder::EmitMetadataErrorReportFunctionTy & = + [](llvm::OpenMPIRBuilder::EmitMetadataErrorKind kind, no llvm:: style: ErrorReportFn similarly

[PATCH] D149666: [OpenMP][OMPIRBuilder] Migrate MapCombinedInfoTy from Clang to OpenMPIRBuilder

jdoerfert added inline comments. Comment at: clang/lib/CodeGen/CGOpenMPRuntime.cpp:6848 + class MapCombinedInfoTy : public llvm::OpenMPIRBuilder::MapCombinedInfoTy { + public: MapExprsArrayTy Exprs; Not sure why you made it a class with public, but I

[PATCH] D148370: [Clang][Flang][OpenMP] Add loadOffloadInfoMetadata and createOffloadEntriesAndInfoMetadata into OMPIRBuilder's finalize and initialize

jdoerfert added inline comments. Comment at: clang/lib/CodeGen/CGOpenMPRuntime.cpp:1063 + // Initialize Types used in OpenMPIRBuilder from OMPKinds.def as well as load + // offload metadata for device from an OpenMP host IR file. +

[PATCH] D148849: [OpenMP-OPT] Remove limit for heap to stack conversions of __kmpc_alloc_shared allocations

jdoerfert added a comment. Make a test for the attributor/openmp-opt, also don't use O2 in tests, the IR only test is sufficient. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D148849/new/

[PATCH] D148805: [Clang][OpenMP] Avoid emitting a __kmpc_alloc_shared for implicit casts which do not have their address taken

jdoerfert added a comment. I don't understand the concerns. A array to ptr decay, by itself, does not capture the pointer such that multiple threads can access it. You need to check the uses of the decay and decide based on them. Thus, I think this is fine. Repository: rG LLVM Github

[PATCH] D147572: [Clang][OpenMP] Fix failure with team-wide allocated variable

jdoerfert added inline comments. Comment at: clang/test/OpenMP/target_team_variable_codegen.cpp:33 +//. +// CHECK-NVIDIA: @local_a = internal addrspace(3) global [10 x i32] zeroinitializer, align 4 +//. doru1004 wrote: > jhuber6 wrote: > > Shouldn't the Nvidia

[PATCH] D136103: OpenMP asynchronous memory copy support

jdoerfert accepted this revision. jdoerfert added a comment. LG Comment at: openmp/libomptarget/src/api.cpp:386 + // Need to check this first to not return OFFLOAD_FAIL instead + if (!(Dst || Src)) { +DP("Call to omp_target_memcpy_rect returns max supported dimensions

[PATCH] D146973: [Clang] Implicitly include LLVM libc headers for the GPU

jdoerfert added a comment. @sivachandra Host and device should agree on all ABI related properties of types or things turn badly, fast. That has to be a given for things to work, and from there, users expect types to be copyable, and usable on both sides. @jhuber6 There are things we might

[PATCH] D146973: [Clang] Implicitly include LLVM libc headers for the GPU

jdoerfert added a comment. I said this before, many times: We don't want to have different host and device libraries that are incompatible. Effectively, what we really want, is the host environment just work on the GPU, that includes extensions in the host headers, macros, taking the address of

[PATCH] D146975: [NVPTX] Add __CUDA_ARCH__ macro to standalone NVPTX compilations

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/D146975/new/ https://reviews.llvm.org/D146975

[PATCH] D145591: [clang][HIP][OpenMP] Add warning if mixed HIP / OpenMP offloading

jdoerfert resigned from this revision. jdoerfert added inline comments. Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:8635 + "HIP does not support OpenMP target directives; directive has been ignored">, + InGroup; + yaxunl wrote: > mhalk wrote:

[PATCH] D140722: [OpenMP] Prefix outlined and reduction func names with original func's name

jdoerfert accepted this revision. jdoerfert added a comment. This revision is now accepted and ready to land. Assuming this passes all tests, incl. runtime tests in openmp/libomptarget/tests, LGTM. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D140722/new/

[PATCH] D146557: [MLIR][OpenMP] Refactoring how map clause is processed

jdoerfert added inline comments. Comment at: llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp:4261 + bool Temp; + return Val->getPointerDereferenceableBytes(M.getDataLayout(), Temp, Temp); } TIFitis wrote: > `getPointerDereferenceableBytes` doesn't fully support

[PATCH] D146557: [MLIR][OpenMP] Added OMPIRBuilder support for use_device_ptr clause

jdoerfert added a comment. Conceptually, I am very confused but mostly because this patch conflates things. I would suggest to split this part: > This also refactors how the map clause is processed and separates the mlir > and IRBuilder parts of the code similar to Clang. it that is a better

[PATCH] D146552: [Clang][OpenMP] Enable device-mapped constexpr class members to not be optimized out

jdoerfert accepted this revision. jdoerfert added a comment. This revision is now accepted and ready to land. This is as good as it gets until we rip out $ref. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D146552/new/ https://reviews.llvm.org/D146552

[PATCH] D146552: [Clang][OpenMP] Enable device-mapped constexpr class members to not be optimized out

jdoerfert added inline comments. Comment at: clang/lib/CodeGen/CGOpenMPRuntime.cpp:10384 // Do not create a "ref-variable" if the original is not also available // on the host. if (!OffloadEntriesInfoManager.hasDeviceGlobalVarEntryInfo(VarName))

[PATCH] D140722: [OpenMP] Prefix outlined and reduction func names with original func's name

jdoerfert added a comment. Generally, I think this is fine. I left comments below. Comment at: clang/lib/CodeGen/CGOpenMPRuntime.cpp:1261 +std::string CGOpenMPRuntime::getOutlinedHelperName(StringRef Name) const { + std::string Suffix = getName({"omp_outlined", ""}); +

[PATCH] D146371: [Clang][OpenMP]Solved the the always truth condition in Arm64

jdoerfert added a comment. Can you please upload a patch that does not reformat the entire file? Also, add a commit message explaining what this does. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D146371/new/ https://reviews.llvm.org/D146371

[PATCH] D140722: [OpenMP] Prefix outlined and reduction func names with original func's name

jdoerfert added a comment. Herald added subscribers: jplehr, sunshaoce. can we update some tests to see the results? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D140722/new/ https://reviews.llvm.org/D140722

[PATCH] D143704: [flang] Feature list plugin

jdoerfert accepted this revision. jdoerfert added a comment. This revision is now accepted and ready to land. LG, as it doesn't affect core features. Comment at: flang/examples/FeatureList/FeatureList.cpp:40-43 +const auto [it, ins] = frequencies.insert({name, 1}); +if

[PATCH] D145591: [clang][HIP][OpenMP] Add warning if mixed HIP / OpenMP offloading

jdoerfert added inline comments. Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:8634 +def warn_hip_omp_target_directives : Warning< + "HIP does not support OpenMP target directives; directive has been ignored">, + InGroup; yaxunl wrote: >

[PATCH] D145591: [clang][HIP][OpenMP] Add warning if mixed HIP / OpenMP offloading

jdoerfert added inline comments. Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:8634 +def warn_hip_omp_target_directives : Warning< + "HIP does not support OpenMP target directives; directive has been ignored">, + InGroup; yaxunl wrote: >

[PATCH] D145820: [Clang][OpenMP] Insert alloca for kernel args at function entry block instead of the launch point.

jdoerfert accepted this revision. jdoerfert added a comment. This revision is now accepted and ready to land. LG, make sure all tests pass, this one seems to fail according to build kite: OpenMP/target_map_codegen_hold.cpp Comment at:

[PATCH] D145591: [clang][HIP][OpenMP] Add warning if mixed HIP / OpenMP offloading

jdoerfert added a comment. I would emit an error. A warning only if we can ensure the code does something sensible. Right now, I doubt that is the case, similarly I doubt we actually ignore things. Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:8634 +def

[PATCH] D145820: [Clang][OpenMP] Insert alloca for kernel args at function entry block instead of the launch point.

jdoerfert added a comment. Check the other functions around, they take an AllocaIP, which is on the user side not necessarily the entry block. We need to follow that scheme here too. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D145820/new/

[PATCH] D145591: [clang][HIP][OpenMP] Add warning if mixed HIP / OpenMP offloading

jdoerfert added a comment. I would emit an error. A warning only if we can ensure the code does something sensible. Right now, I doubt that is the case, similarly I doubt we actually ignore things. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D145591: [clang][HIP][OpenMP] Add warning if mixed HIP / OpenMP offloading

jdoerfert added inline comments. Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:8634 +def warn_hip_omp_target_directives : Warning< + "HIP does not support OpenMP target directives; directive has been ignored">, + InGroup; I doubt the ignored

[PATCH] D145514: [OPENMP]Fix PR59947: "Partially-triangular" loop collapse crashes.

jdoerfert accepted this revision. jdoerfert added a comment. This revision is now accepted and ready to land. This seems to pass our other tests, LG. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D145514/new/ https://reviews.llvm.org/D145514

[PATCH] D145514: [OPENMP]Fix PR59947: "Partially-triangular" loop collapse crashes.

jdoerfert added inline comments. Comment at: openmp/runtime/test/worksharing/for/omp_for_non_rectangular.c:33 + collapsed(N); + return test(N); +} should we test the count? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D144864: [Flang][Driver][MLIR] Add -fopenmp-is-device to Flang and link to an omp.is_device attribute

jdoerfert added a comment. Tests? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D144864/new/ https://reviews.llvm.org/D144864 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D144884: [clang-format] Only add pragma continuation indentation for 'omp' clauses

jdoerfert added a comment. I'm assuming they have tests? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D144884/new/ https://reviews.llvm.org/D144884 ___ cfe-commits mailing list

[PATCH] D143704: [Flang] Part one of Feature List action

jdoerfert added a comment. can we have a test? Comment at: flang/examples/FeatureList/FeatureList.cpp:605 + template bool Pre(const std::variant &) { return true; } + template void Post(const std::variant &) {} +}; Can you record the features we missed so

[PATCH] D144634: [Clang][OpenMP] Support for Code Generation of loop bind clause

jdoerfert added a comment. We need tests. Comment at: clang/lib/CodeGen/CGStmtOpenMP.cpp:7790 + for (const auto *C : S.getClausesOfKind()) { +OpenMPBindClauseKind bindParam = C->getBindKind(); +switch (bindParam) { style, also below. Repository:

[PATCH] D143306: [Driver] Default to -fno-openmp-implicit-rpath

jdoerfert added a comment. In D143306#4145603 , @tstellar wrote: > In D143306#4145432 , @MaskRay wrote: > >> This is the point. Specifying a driver option to use >> libc++/libc++abi/libunwind doesn't magically

[PATCH] D143306: [Driver] Default to -fno-openmp-implicit-rpath

jdoerfert added a comment. In D143306#4145465 , @MaskRay wrote: > In D143306#4144541 , @jdoerfert > wrote: > >> I'm worried this makes use of LLVM on HPC machines even harder. That said, >> I'm open to

[PATCH] D142569: [OpenMP] Introduce kernel environment

jdoerfert added inline comments. Comment at: openmp/libomptarget/plugins-nextgen/common/PluginInterface/PluginInterface.cpp:583 + if (!KernelEnvOrError) +return KernelEnvOrError.takeError(); + tianshilei1992 wrote: > jdoerfert wrote: > > We used to default

[PATCH] D144544: [OpenMP] Improve LIT tests on composite target constructs

jdoerfert accepted this revision. jdoerfert added a comment. This revision is now accepted and ready to land. I guess this is fine. We should try to run V tests, this is really just checking if we generate (some) code. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D143306: [Driver] Default to -fno-openmp-implicit-rpath

jdoerfert added a comment. I'm worried this makes use of LLVM on HPC machines even harder. That said, I'm open to suggestions and I am not well versed in all the ways we can make it work. Our problem is that there are N `libomptarget.so` files and N `libomptarget.nvptx.so` files on a system,

[PATCH] D144320: [Clang][OpenMP] Update tests using update_cc_test_checks.py

jdoerfert added a comment. LG, assuming the bots are happy Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D144320/new/ https://reviews.llvm.org/D144320 ___ cfe-commits mailing list

[PATCH] D142569: [OpenMP] Introduce kernel environment

jdoerfert accepted this revision. jdoerfert added a comment. This revision is now accepted and ready to land. LG Comment at: llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp:3917 + + Twine DynamicEnvironmenttName = KernelName + "_dynamic_environment"; + Constant

[PATCH] D143704: [Flang] Part one of Feature List action

jdoerfert added a comment. > However, it should be considered harmful to make the TODO macro always > associate with some syntactic source artifact. It would also be unwise to > redefine TODO from a halt semantics to a "just keep going and see what > happens" semantics. @schweitz Why? What's

[PATCH] D142569: [OpenMP] Introduce kernel environment

jdoerfert added inline comments. Comment at: llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp:3888 + + Function *Kernel = Builder.GetInsertBlock()->getParent(); Function *Fn = getOrCreateRuntimeFunctionPtr( That is not the kernel, at least not when clang emits a

[PATCH] D143301: Emit warning for unsupported gfortran flags

jdoerfert added a comment. In D143301#4126712 , @awarzynski wrote: >> I think the -W stuff can go in, it has tests and is reasonable. > > I'd like for us to rely on a flag from Options.td for this instead. Something > similar to clang_ignored_f_Group

[PATCH] D143301: Emit warning for unsupported gfortran flags

jdoerfert added a comment. Split this into two patches/reviews. I think the -W stuff can go in, it has tests and is reasonable. The other stuff needs tests too. Comment at: flang/lib/Frontend/CompilerInvocation.cpp:607 if (args.hasArg(clang::driver::options::OPT_W_Joined))

[PATCH] D143549: [clang][AIX] Remove test for the default OpenMP runtime

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/D143549/new/ https://reviews.llvm.org/D143549

[PATCH] D142914: [MLIR][OpenMP] Added OMPIRBuilder support for Target Data directives.

jdoerfert added a comment. Please avoid the MLIR style in any other subproject. I know some of the OpenMP Builder stuff already uses `llvm::` and MLIR style names, but that is in itself bad, not something we should extend. Comment at:

[PATCH] D142650: [OpenMP] Run an extra 'OpenMPOpt' pass in LTO-mode

jdoerfert accepted this revision. jdoerfert added a comment. This revision is now accepted and ready to land. LG, one nit. Comment at: llvm/lib/Passes/PassBuilderPipelines.cpp:1720 + MPM.addPass(OpenMPOptPass(ThinOrFullLTOPhase::FullLTOPostLink)); + // If we didn't decide

  1   2   3   4   5   6   7   8   9   10   >