[PATCH] D105648: [OpenMP] Support OpenMP 5.1 attributes

jdoerfert added inline comments. Comment at: clang/lib/Parse/ParseDeclCXX.cpp:2670-2676 + + // The next token may be an OpenMP pragma annotation token. That would + // normally be handled from ParseCXXClassMemberDeclarationWithPragmas, but in + // this case, it came from an *a

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

jdoerfert added a comment. In D102107#2832740 , @ABataev wrote: > In D102107#2832286 , @jdoerfert > wrote: > >> In D102107#2824581 , @ABataev >> wrote: >> >>> In D102107

[PATCH] D104904: [OpenMP][AMDGCN] Initial math headers support

jdoerfert added inline comments. Comment at: clang/test/Headers/Inputs/include/cstdlib:29 float fabs(float __x) { return __builtin_fabs(__x); } +#endif estewart08 wrote: > JonChesterfield wrote: > > jdoerfert wrote: > > > That seems to be fundamentally broken

[PATCH] D101976: [OpenMP] Unified entry point for SPMD & generic kernels in the device RTL

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. jdoerfert marked an inline comment as done. Closed by commit rG1d5711c3eeb6: [OpenMP] Unified entry point for SPMD & generic kernels in the device RTL (authored by jdoe

[PATCH] D77491: [Sema] Introduce BuiltinAttr, per-declaration builtin-ness

jdoerfert reopened this revision. jdoerfert added a comment. Herald added a subscriber: dexonsmith. > A pthread_create() related test is XFAIL-ed, as it relied on it being > recognized as a builtin based on its name. > The builtin declaration syntax is too restrictive and doesn't allow custom >

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

jdoerfert added a comment. In D102107#2867693 , @ABataev wrote: > In D102107#2867670 , @josemonsalve2 > wrote: > >> In D102107#2867417 , @ABataev >> wrote: >> >>> In D10

[PATCH] D105898: [OpenMP] Rework OpenMP remarks

jdoerfert accepted this revision. jdoerfert added a comment. This revision is now accepted and ready to land. typo in commit message. Some minor suggestions. LG Comment at: llvm/lib/Transforms/IPO/OpenMPOpt.cpp:1211 auto Remark = [&](OptimizationRemark OR) { -re

[PATCH] D105876: OMPIRBuilder for Interop directive

jdoerfert added a subscriber: mikerice. jdoerfert added a comment. There are 3 clang tidy warnings to address. Overall this is a good first step. @ABataev @mikerice is there any major problem with this? If not we should try to get it in asap and improve it in tree. Comment at

[PATCH] D105937: [OpenMP] Encode `omp [...] assume[...]` assumptions with `omp[x]` prefix

jdoerfert created this revision. jdoerfert added a reviewer: jhuber6. Herald added subscribers: guansong, bollu, yaxunl. jdoerfert requested review of this revision. Herald added subscribers: cfe-commits, sstefan1. Herald added a project: clang. Since these assumptions are coming from OpenMP it ma

[PATCH] D105937: [OpenMP] Encode `omp [...] assume[...]` assumptions with `omp[x]` prefix

jdoerfert updated this revision to Diff 358502. jdoerfert added a comment. Fix lifetime problem, update one more test Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D105937/new/ https://reviews.llvm.org/D105937 Files: clang/include/clang/Sema/Sema

[PATCH] D105939: [OpenMP] Add IDs to OpenMP remarks

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

[PATCH] D112504: [OpenMP] Wrap (v)printf in the new RT and use same handling for AMD

jdoerfert created this revision. jdoerfert added reviewers: JonChesterfield, tianshilei1992, jhuber6. Herald added subscribers: guansong, bollu, yaxunl. jdoerfert requested review of this revision. Herald added subscribers: cfe-commits, sstefan1. Herald added projects: clang, OpenMP. To support `p

[PATCH] D112504: [OpenMP] Wrap (v)printf in the new RT and use same handling for AMD

jdoerfert updated this revision to Diff 382171. jdoerfert added a comment. Actually use the new wrapper for OpenMP offload targeting AMD (and the new RT) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D112504/new/ https://reviews.llvm.org/D112504 Fi

[PATCH] D112504: [OpenMP] Wrap (v)printf in the new RT and use same handling for AMD

jdoerfert added inline comments. Comment at: clang/lib/CodeGen/CGGPUBuiltin.cpp:128 return RValue::get(Builder.CreateCall( VprintfFunc, {Args[0].getRValue(*this).getScalarVal(), BufferPtr})); } >>! In D112504#3086474, @JonChesterfield wrote: > That's a

[PATCH] D112227: [libomptarget] Build DeviceRTL for amdgpu

jdoerfert accepted this revision. jdoerfert added a comment. This revision is now accepted and ready to land. LG, I'll land my fix and with this we can switch over to the new RT. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D112227/new/ https://rev

[PATCH] D109770: [OpenMP] Declare variants for templates need to match # template args

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG6cf6fa6ef1c2: [OpenMP] Declare variants for templates need to match # template args (authored by jdoerfert). Repository: rG LLVM Github Monorepo

[PATCH] D113126: [OpenMP][NFCI] Embed the source location string size in the ident_t

jdoerfert updated this revision to Diff 384554. jdoerfert added a comment. Herald added a project: clang. Herald added a subscriber: cfe-commits. Include the clang changes Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D113126/new/ https://reviews.ll

[PATCH] D112680: [OpenMP] Lower printf to __llvm_omp_vprintf

jdoerfert accepted this revision. jdoerfert added a comment. This revision is now accepted and ready to land. LG, one nit. Comment at: clang/lib/CodeGen/CGGPUBuiltin.cpp:231 + VprintfFunc, {Args[0].getRValue(*this).getScalarVal(), BufferPtr, Size})); +} S

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

jdoerfert added inline comments. Comment at: clang/lib/CodeGen/CGOpenMPRuntimeVirtualGPU.cpp:54 + CGOpenMPRuntime::createOffloadEntry(ID, Addr, Size, Flags, Linkage); +} We should be able to get rid of this file (and the cuda/hip) version. Might be the right ti

[PATCH] D113421: [clang][openmp][NFC] Remove arch-specific CGOpenMPRuntimeGPU files

jdoerfert added a comment. In D113421#3116155 , @tianshilei1992 wrote: > I remember at some point we want to just emit function call to get those > information like thread id. Yes, no intrinsics please. Add `__kmpc_get_warp_size` and similar functions

[PATCH] D112504: [OpenMP] Wrap (v)printf in the new RT and use same handling for AMD

jdoerfert closed this revision. jdoerfert added a comment. Committed as part of D112680 . Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D112504/new/ https://reviews.llvm.org/D112504

[PATCH] D113421: [clang][openmp][NFC] Remove arch-specific CGOpenMPRuntimeGPU files

jdoerfert accepted this revision. jdoerfert added a comment. This revision is now accepted and ready to land. LG Comment at: clang/lib/CodeGen/CGOpenMPRuntimeGPU.cpp:23 #include "llvm/Frontend/OpenMP/OMPGridValues.h" +#include "llvm/IR/IntrinsicsAMDGPU.h" #include "llvm/IR/In

[PATCH] D111463: [OpenMP] Remove doing assumption propagation in the front end.

jdoerfert accepted this revision. jdoerfert added a comment. This revision is now accepted and ready to land. Herald added a subscriber: asavonic. I'm assuming this is simply a revert, LG in that case, once the others landed. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION htt

[PATCH] D113126: [OpenMP][NFCI] Embed the source location string size in the ident_t

jdoerfert updated this revision to Diff 386255. jdoerfert added a comment. Herald added a subscriber: asavonic. update tests Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D113126/new/ https://reviews.llvm.org/D113126 Files: clang/lib/CodeGen/CGOp

[PATCH] D113126: [OpenMP][NFCI] Embed the source location string size in the ident_t

jdoerfert added a comment. In D113126#3122659 , @tianshilei1992 wrote: > I'm not convinced. `std::strlen` can do the job. Can you explain more why > need it? Because the ident_t is on the device and you want to grab it from the host. Repository: rG

[PATCH] D113623: [OpenMP][FIX] Pass the num_threads value directly to parallel_51

jdoerfert created this revision. jdoerfert added reviewers: jhuber6, ggeorgakoudis, tianshilei1992, grokos, jdenny. Herald added subscribers: asavonic, guansong, bollu, yaxunl. jdoerfert requested review of this revision. Herald added subscribers: cfe-commits, sstefan1. Herald added projects: clan

[PATCH] D113140: [OpenMP][NFCI] Introduce the kernel environment for target regions

jdoerfert updated this revision to Diff 386343. jdoerfert added a comment. Herald added subscribers: cfe-commits, ormris, okura, kuter, kerbowa, nhaehnle. Herald added a reviewer: sstefan1. Herald added a reviewer: baziotis. Herald added a project: clang. Move more values into the kernel environme

[PATCH] D113140: [OpenMP][NFCI] Introduce the kernel environment for target regions

jdoerfert added a comment. In D113140#3123162 , @ye-luo wrote: >> the kernel environment which contains information passed by the compiler to >> a GPU kernel. > > DId you mean this environment is baked into kernel at compile time? So there > is no addit

[PATCH] D97573: [OpenMP] Handle non-function context before checking for diagnostic emission

jdoerfert added a comment. looks reasonable to me Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D97573/new/ https://reviews.llvm.org/D97573 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://

[PATCH] D97680: [OpenMP] Simplify GPU memory globalization

jdoerfert added inline comments. Comment at: openmp/libomptarget/deviceRTLs/common/src/data_sharing.cu:34 +EXTERN void *__kmpc_alloc_shared(size_t DataSize) { + return (void *)SafeMalloc(DataSize, "Alloc Shared"); } Add a TODO: 1) Use a small shared buffer 2) e

[PATCH] D97107: Replace func name with regex in update_cc_test_checks

jdoerfert accepted this revision. jdoerfert added a comment. This revision is now accepted and ready to land. Cool, almost able to generate offloading tests :) LG Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D97107/new/ https://reviews.llvm.org/D97

[PATCH] D97198: [OpenMP][Clang][NVPTX] Only build one bitcode library for each SM

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

[PATCH] D97068: Run non-filechecked commands in update_cc_test_checks.py

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

[PATCH] D98358: [OpenMP] Restore backwards compatibility for libomptarget

jdoerfert accepted this revision. jdoerfert added a comment. This revision is now accepted and ready to land. LG. We should have a test that references the entire interface so we realize if something breaks. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm

[PATCH] D91556: [OpenMPIRBuilder} Add capturing of parameters to pass to omp::parallel

jdoerfert added a subscriber: ggeorgakoudis. jdoerfert added a comment. In D91556#2617144 , @kiranchandramohan wrote: > Ping @llitchev. Would you have time to take this forward? I think @ggeorgakoudis is working on an alternative API solution, we might n

[PATCH] D94741: [Utils] Check for more global information in update_test_checks

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGcd1bd6e58700: [Utils] Check for more global information in update_test_checks (authored by jdoerfert). Herald added a project: clang. Herald added a

[PATCH] D95764: [OpenMP] Do not propagate match extensions to nested contexts

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGad9e98b8efa0: [OpenMP] Do not propagate match extensions to nested contexts (authored by jdoerfert). Herald added subscribers: llvm-commits, hiradity

[PATCH] D95765: [OpenMP] Introduce the `disable_selector_propagation` variant selector trait

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGb2642456abc6: [OpenMP] Introduce the `disable_selector_propagation` variant selector trait (authored by jdoerfert). Changed prior to commit: https

[PATCH] D94741: [Utils] Check for more global information in update_test_checks

jdoerfert added a comment. In D94741#2624475 , @nikic wrote: > It looks like this has broken handling of `getelementptr %T`, and generates > `getelementptr [[T]]` for it now, see e.g. the first change in > https://github.com/llvm/llvm-project/commit/7ee9

[PATCH] D98594: [clang/docs/LibASTMatchersTutorial.rst,clang/docs/HowToSetupToolingForLLVM.rst] ninja now uses `configure.py` rather than `bootstrap.py`

jdoerfert accepted this revision. jdoerfert added a comment. This revision is now accepted and ready to land. LG, use `[Docs]` as tag for the commit, it should not be verbose but helpful to determine if a commit is interesting. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D98594/new/ h

[PATCH] D96769: [OpenMP][AMDGPU] Skip backend and assemble phases for amdgcn

jdoerfert resigned from this revision. jdoerfert added a comment. I see the need for this to work, unsure if this is the right way, we need to make progress though. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D96769/new/ https://reviews.llvm.org/

[PATCH] D136198: [OpenMP] Make kernels have protected visibility

jdoerfert accepted this revision. jdoerfert added a comment. This revision is now accepted and ready to land. LG, but all of this code should move into the IR Builder... Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D136198/new/ https://reviews.llvm

[PATCH] D136103: OpenMP asynchronous memory copy support

jdoerfert added a comment. > 6. Why do you access args_ for some parts and not for others? That said, > where does the hidden helper need access to the dependences anyway? > > there's type cast for depend objects from 'omp_depend_t' to > 'kmp_depend_info_t*', and the array of casted depend obj

[PATCH] D134410: [clang][CodeGen] Add noundef metadata to load instructions (preliminary)

jdoerfert added a comment. > However cleanup looks scary. Msan reports maybe 20% of unique tests on our > code base. Many a of them share root cause, but still many unique root causes. On quick looks I see no false report While scary, this also is extremely encouraging to move forward with this

[PATCH] D136239: [testcase] [OpenMP] Fix the testcase error of check-all when DCLANG_DEFAULT_OPENMP_RUNTIME is not libomp

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

[PATCH] D136103: OpenMP asynchronous memory copy support

jdoerfert added inline comments. Comment at: openmp/libomptarget/src/api.cpp:264 + if (DepObj_Count > 0) { +DepObjs = new kmp_depend_info_t[DepObj_Count]; +for (int i = 0; i < DepObj_Count; i++) { Use an llvm::SmallVector Comment at: o

[PATCH] D136103: OpenMP asynchronous memory copy support

jdoerfert added a comment. Thanks for the latest updates. The duplication is still my main concern here. See below. Comment at: openmp/libomptarget/src/api.cpp:343 +static int __kmpc_target_memcpy_rect_async_helper(kmp_int32 Gtid, +

[PATCH] D136103: OpenMP asynchronous memory copy support

jdoerfert added inline comments. Comment at: openmp/libomptarget/src/api.cpp:405 + return Rc; +} + Now we have a first helper for the "async" part, we still should deduplicate the entry point code. Above, all but lines 385-387 are the same as in `omp_target_me

[PATCH] D136103: OpenMP asynchronous memory copy support

jdoerfert added a comment. In D136103#3876097 , @jz10 wrote: > Thanks Johannes and Shilei > > 1. '385-387 are the same as in omp_target_memcpy_async. Can we also not > duplicate those lines?' > > I put the common code (i.e.helper task creation) into anot

[PATCH] D136103: OpenMP asynchronous memory copy support

jdoerfert added a comment. Looks good from my end. Thanks for the updates, I hope they make sense in the end. Some minor style comments below. @tianshilei1992 Should accept once the header concerns are resolved. Comment at: openmp/libomptarget/src/api.cpp:256-257 +// Tas

[PATCH] D136715: formatting clang offloadbundler source file

jdoerfert accepted this revision. jdoerfert added a comment. I'm not sure why you fight with phabricator so much. I was going to edit your revision metadata but you for some reason disallowed that. I would generally suggest against that. --- The patch itself is fine, the commit subject line and

[PATCH] D136103: OpenMP asynchronous memory copy support

jdoerfert accepted this revision. jdoerfert added a comment. LG, thanks for all the changes. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D136103/new/ https://reviews.llvm.org/D136103 ___ cfe-commits mailing list cfe-commits@lists.llvm.org ht

[PATCH] D136872: [OpenMP][OpenMPIRBuilder] Migrate loadOffloadInfoMetadata from clang to OMPIRbuilder

jdoerfert added inline comments. Comment at: llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h:2167 +Module &M, llvm::OffloadEntriesInfoManager &OffloadEntriesInfoManager, +bool isDevice); + Why is this in llvm namespace? This should be qualified further,

[PATCH] D136872: [OpenMP][OpenMPIRBuilder] Migrate loadOffloadInfoMetadata from clang to OMPIRbuilder

jdoerfert added a comment. @jsjodin Can you take a look and accept once it looks ok. Comment at: llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp:4717 + if (!isDevice) +return; + TIFitis wrote: > jdoerfert wrote: > > Why do we call it if isDevice is false? Module

[PATCH] D136872: [OpenMP][OpenMPIRBuilder] Migrate loadOffloadInfoMetadata from clang to OMPIRbuilder

jdoerfert added inline comments. Comment at: llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h:1685 + + const std::string ompOffloadInfoName = "omp_offload.info"; + Drive by, add doxygen documentation to all methods + members please. Repository: rG LLVM Githu

[PATCH] D90275: [clang][IR] Add support for leaf attribute

jdoerfert added a comment. In D90275#3715563 , @xbolva00 wrote: > Was there any follow up? It seems that this hint (attribute) is currently not > used by LLVM passes. It is, also in our runtime, see https://reviews.llvm.org/D131628#3716252 Repository:

[PATCH] D131763: [OpenMP] Add lit test for metadirective device arch inspired from sollve

jdoerfert added a comment. This doesn't actually test much, only once case/compilation is covered. In the second function nothing specific to LLVM as impl is checked. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D131763/new/ https://reviews.llvm.o

[PATCH] D131763: [OpenMP] Add lit test for metadirective device arch inspired from sollve

jdoerfert added a comment. In D131763#3719140 , @saiislam wrote: > In D131763#3719132 , @jdoerfert > wrote: > >> This doesn't actually test much, only once case/compilation is covered. In >> the second function

[PATCH] D122255: Meta directive runtime support

jdoerfert added inline comments. Comment at: clang/lib/AST/OpenMPClause.cpp:1679 + OS << ": "; +} + abidmalikwaterloo wrote: > jdoerfert wrote: > > I'm assuming we already have a printer for trait selectors, no? Doesn't > > `OMPTraitInfo::print` do this already

[PATCH] D130020: [OpenMP] Deprecate the old driver for OpenMP offloading

jdoerfert accepted this revision. jdoerfert added a comment. This revision is now accepted and ready to land. LG, as discussed over many weeks in our calls. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D130020/new/ https://reviews.llvm.org/D130020

[PATCH] D82087: AMDGPU/clang: Add builtins for llvm.amdgcn.ballot

jdoerfert added a comment. Herald added a subscriber: kosarev. Herald added a project: All. Can we land this? I'd like to use the new intrinsics as I don't understand the old ones. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D82087/new/ https://reviews.llvm.org/D82087 ___

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

jdoerfert added a comment. In D102107#3812582 , @dhruvachak wrote: > In D102107#3812554 , @dhruvachak > wrote: > >> Updated llvm tests. The following 3 tests still fail: >> >> LLVM :: Transforms/OpenMP/spmdiza

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

jdoerfert added a comment. I'm unlikely to get to it in the next 2 weeks (IWOMP and OpenMP F2F). What I would do is to take the new IR, the old IR, run instnamer on the new one. Then splice in the new parts into the old IR removing what was there wrt. parallel_51. Repository: rG LLVM Github

[PATCH] D134660: [LinkerWrapper] Fix optimized debugging builds for NVPTX LTO

jdoerfert accepted this revision. jdoerfert added a comment. This revision is now accepted and ready to land. Better than before. Thx Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D134660/new/ https://reviews.llvm.org/D134660 __

[PATCH] D133578: [OpenMP][OMPIRBuilder] Add generation of SIMD align assumptions to OMPIRBuilder

jdoerfert added inline comments. Comment at: llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp:2976 + const int DefaultAlignment = 16; + domada wrote: > jdoerfert wrote: > > This doesn't work. Use the data layout for any default values please. > I have used pointer AB

[PATCH] D134046: [OpenMP][OMPIRBuilder] Add support for order(concurrent) to OMPIRBuilder for SIMD directive

jdoerfert added a comment. I think the rest looks good. Comment at: llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h:213 + enum class OMPOrderClauseValue { NotSpecified, Concurrent }; + I think this belongs into `llvm/include/llvm/Frontend/OpenMP/OMPConstant

[PATCH] D135076: [Clang] Make offloading flags accept '-' and '--'

jdoerfert added a comment. In D135076#3830914 , @MaskRay wrote: > Single-dash long options starting with `-o` conflict with the short option > `-o` so I am unsure I like this direction. > >> However, if a user uses a single dash it will instead name a fi

[PATCH] D132855: [OpenMP] Extend the lit test for uses_allocators in target region

jdoerfert added inline comments. Comment at: clang/test/OpenMP/target_map_codegen_10.cpp:19 +// RUN: %clang_cc1 -no-opaque-pointers -DCK11 -fopenmp -fopenmp-targets=i386-pc-linux-gnu -x c++ -std=c++11 -triple i386-unknown-unknown -emit-pch -o %t %s +// RUN: %clang_cc1 -no-opaqu

[PATCH] D133578: [OpenMP][OMPIRBuilder] Add generation of SIMD align assumptions to OMPIRBuilder

jdoerfert added inline comments. Comment at: llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp:2976 + const int DefaultAlignment = 16; + domada wrote: > jdoerfert wrote: > > domada wrote: > > > jdoerfert wrote: > > > > This doesn't work. Use the data layout for any de

[PATCH] D134046: [OpenMP][OMPIRBuilder] Add support for order(concurrent) to OMPIRBuilder for SIMD directive

jdoerfert accepted this revision. jdoerfert added a comment. This revision is now accepted and ready to land. LG CHANGES SINCE LAST ACTION https://reviews.llvm.org/D134046/new/ https://reviews.llvm.org/D134046 ___ cfe-commits mailing list cfe-commi

[PATCH] D135199: [OpenMP][C++] Allow #pragma omp simd in constexpr functions

jdoerfert created this revision. jdoerfert added reviewers: aaron.ballman, faisalv. Herald added subscribers: guansong, bollu, yaxunl. Herald added a project: All. jdoerfert requested review of this revision. Herald added a subscriber: sstefan1. Herald added a project: clang. There is no reason (o

[PATCH] D135155: [AMDGPU] Annotate the intrinsics to be default and nocallback

jdoerfert updated this revision to Diff 465256. jdoerfert added a comment. Herald added a project: clang. Herald added a subscriber: cfe-commits. Update tests Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D135155/new/ https://reviews.llvm.org/D13515

[PATCH] D135155: [AMDGPU] Annotate the intrinsics to be default and nocallback

jdoerfert updated this revision to Diff 465262. jdoerfert marked 6 inline comments as done. jdoerfert added a comment. Address comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D135155/new/ https://reviews.llvm.org/D135155 Files: clang/test/

[PATCH] D135155: [AMDGPU] Annotate the intrinsics to be default and nocallback

jdoerfert updated this revision to Diff 465350. jdoerfert added a comment. Last changes Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D135155/new/ https://reviews.llvm.org/D135155 Files: clang/test/CodeGenOpenCL/builtins-amdgcn.cl llvm/include/

[PATCH] D133578: [OpenMP][OMPIRBuilder] Add generation of SIMD align assumptions to OMPIRBuilder

jdoerfert added inline comments. Comment at: clang/lib/CodeGen/CGStmtOpenMP.cpp:2640 if (UseOMPIRBuilder) { +llvm::DenseMap AlignedVars; // Emit the associated statement and get its loop representation. Where is this map populated? =

[PATCH] D135199: [OpenMP][C++] Allow #pragma omp simd in constexpr functions

jdoerfert added inline comments. Comment at: clang/lib/AST/ExprConstant.cpp:8274-8279 // Only if a local variable was declared in the function currently being // evaluated, do we expect to be able to find its value in the current // frame. (Otherwise it was likely

[PATCH] D132855: [OpenMP] Extend the lit test for uses_allocators in target region

jdoerfert accepted this revision. jdoerfert added a comment. This revision is now accepted and ready to land. LG Comment at: clang/test/OpenMP/target_map_codegen_10.cpp:19 +// RUN: %clang_cc1 -no-opaque-pointers -DCK11 -fopenmp -fopenmp-targets=i386-pc-linux-gnu -x c++ -std=c+

[PATCH] D135374: [OpenMP][AMDGPU] Add 'uniform-work-group' attribute to OpenMP kernels

jdoerfert accepted this revision. jdoerfert added a comment. This revision is now accepted and ready to land. LG, add a TODO to move the code. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D135374/new/ https://reviews.llvm.org/D135374 _

[PATCH] D135199: [OpenMP][C++] Allow #pragma omp simd in constexpr functions

jdoerfert added inline comments. Comment at: clang/lib/AST/ExprConstant.cpp:8274-8279 // Only if a local variable was declared in the function currently being // evaluated, do we expect to be able to find its value in the current // frame. (Otherwise it was likely

[PATCH] D135614: [OpenMP][CUDA][AMDGPU] Accept case insensitive subarchitecture names

jdoerfert added a comment. First, as noted, I asked @jhuber6 to update this. In D135614#3847775 , @tra wrote: > Is that really something we need/want to do? I've never seen anyone > complaining about this particular issue. I have, and not only once. He

[PATCH] D133578: [OpenMP][OMPIRBuilder] Add generation of SIMD align assumptions to OMPIRBuilder

jdoerfert added a comment. This now looks good. The test is the only things I don't understand. Comment at: clang/test/OpenMP/irbuilder_simd_aligned.cpp:69 +// CHECK: omp_loop.preheader: +// CHECK-COUNT-3: call void @llvm.assume(i1 true) [ "align" +// CHECK-NEXT:br la

[PATCH] D133578: [OpenMP][OMPIRBuilder] Add generation of SIMD align assumptions to OMPIRBuilder

jdoerfert accepted this revision. jdoerfert added a comment. This revision is now accepted and ready to land. LG, thanks for working on this. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D133578/new/ https://reviews.llvm.org/D133578 ___ cfe-c

[PATCH] D136111: [OpenMP] Make device functions have hidden visibility

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

[PATCH] D136103: OpenMP asynchronous memory copy support

jdoerfert added a comment. First set of comments. Comment at: clang/docs/ReleaseNotes.rst:253 + `Issue 47177 `_ + `Issue 47179 `_ Unrelated, probably bad d

[PATCH] D140292: [OpenMP] Migrate OpenMPOffloadMappingFlags from Clang CodeGen to OMPConstants

jdoerfert added inline comments. Comment at: llvm/include/llvm/Frontend/OpenMP/OMPConstants.h:193 - OMP_TGT_EXEC_MODE_GENERIC | OMP_TGT_EXEC_MODE_SPMD, - LLVM_MARK_AS_BITMASK_ENUM(/* LargestValue */ OMP_TGT_EXEC_MODE_GENERIC_SPMD) }; TIFitis wrote: > I am

[PATCH] D140336: [OpenMP][NFC] Cleanup clause boilerplate (unfinished)

This revision was automatically updated to reflect the committed changes. Closed by commit rG66890ce0b3d3: [OpenMP][NFC] Cleanup clause boilerplate (authored by jdoerfert). Herald added a project: clang. Herald added a subscriber: cfe-commits. Changed prior to commit: https://reviews.llvm.org/D

[PATCH] D140292: [OpenMP] Migrate OpenMPOffloadMappingFlags from Clang CodeGen to OMPConstants

jdoerfert added inline comments. Comment at: llvm/include/llvm/Frontend/OpenMP/OMPConstants.h:193 - OMP_TGT_EXEC_MODE_GENERIC | OMP_TGT_EXEC_MODE_SPMD, - LLVM_MARK_AS_BITMASK_ENUM(/* LargestValue */ OMP_TGT_EXEC_MODE_GENERIC_SPMD) }; TIFitis wrote: > jdoer

[PATCH] D140292: [OpenMP] Migrate OpenMPOffloadMappingFlags from Clang CodeGen to OMPConstants

jdoerfert added a comment. Can you please check why there are so many reformatted lines are part of this patch? Is the clang-format version up to date? Maybe it changed but we should only format the parts touched by the patch, hence `git-clang-format HEAD~` rather than format everything. Repo

[PATCH] D140292: [OpenMP] Migrate OpenMPOffloadMappingFlags from Clang CodeGen to OMPConstants

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

[PATCH] D141158: [OpenMP] Introduce '-f[no]-openmp-target-jit' flag to control JIT for offloading

jdoerfert added inline comments. Comment at: clang/lib/Driver/Driver.cpp:730 + ? LTOK_Full + : OffloadLTOMode; } Should we overwrite unconditionally or warn/error if the combination is nonsensical? Repository:

[PATCH] D141158: [OpenMP] Introduce '-f[no]-openmp-target-jit' flag to control JIT for offloading

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

[PATCH] D14119: [libcxxabi] Correctly align fallback heap

jdoerfert added a comment. > jdoerfert added a commit: rG10410534696e > : > [CallGraph][FIX] Ensure generic intrinsics are represented in the CG. My bad, I copied the review ID and dropped a 0. Supposed to be https://reviews.

[PATCH] D141105: [OpenMP] Add support for '--offload-arch=native' to OpenMP offloading

jdoerfert accepted this revision. jdoerfert added a comment. This revision is now accepted and ready to land. Looks reasonable to me. See comments though Comment at: clang/include/clang/Driver/Driver.h:488 + Action::OffloadKind Kind, const ToolChain *TC, +

[PATCH] D136103: OpenMP asynchronous memory copy support

jdoerfert added a comment. Any reason this hasn't been pushed yet? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D136103/new/ https://reviews.llvm.org/D136103 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-b

[PATCH] D137180: [LinkerWrapper] report on missing libraries

jdoerfert added inline comments. Comment at: clang/tools/clang-linker-wrapper/ClangLinkerWrapper.cpp:1278 + if (identify_magic((*BufferOrErr)->getBuffer()) != file_magic::archive) +continue; + So if the library is found but not an archive we silently

[PATCH] D137180: [LinkerWrapper] report on missing libraries

jdoerfert added inline comments. Comment at: clang/tools/clang-linker-wrapper/ClangLinkerWrapper.cpp:1278 + if (identify_magic((*BufferOrErr)->getBuffer()) != file_magic::archive) +continue; + jhuber6 wrote: > jdoerfert wrote: > > So if the library i

[PATCH] D137180: [LinkerWrapper] report on missing libraries

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

[PATCH] D137720: Migrate getOrCreateInternalVariable from Clang to OMPIRBuilder.

jdoerfert accepted this revision. jdoerfert added a comment. This revision is now accepted and ready to land. LG, minor nits. Nice cleanup. Comment at: llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h:1729 + const llvm::StringRef &Na

[PATCH] D137851: [OPENMP]Initial support for at clause

jdoerfert added a comment. Drive by: Please add commit messages to non trivial changes to explaining what's going on. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D137851/new/ https://reviews.llvm.org/D137851 _

[PATCH] D142491: [OpenMP] Do not link the bitcode OpenMP runtime when targeting AMDGPU.

jdoerfert accepted this revision. jdoerfert added a comment. This revision is now accepted and ready to land. LG, not linking in something seems fine if it works and the direction is what we always wanted. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.o

<    6   7   8   9   10   11   12   13   14   15   >