[PATCH] D94973: [clang][OpenMP] Use OpenMPIRBuilder for workshare loops.

jdoerfert added a comment. This seems sensible. The clang parts look OK but I haven't thought everything through to the last detail. I left some questions below, I guess we can go down this road and slowly transition to the new scheme. Comment at: clang/lib/AST/Stmt.cpp:1275

[PATCH] D94745: [OpenMP][deviceRTLs] Build the deviceRTLs with OpenMP instead of target dependent language

jdoerfert added a comment. In D94745#2539405 , @protze.joachim wrote: > For me this patch breaks building llvm. Before this patch, I successfully > built llvm using a llvm/10 installation on the system. What is probably > special with our llvm installat

[PATCH] D95915: [clang][driver] Only warn once about invalid -stdlib value

jdoerfert added a comment. Drive by: I don't believe a static variable is a good idea. Also the name is not really informative. A member in `ToolChain` with a proper name would be nicer (IMHO). CHANGES SINCE LAST ACTION https://reviews.llvm.org/D95915/new/ https://reviews.llvm.org/D95915 _

[PATCH] D63845: [WIP] Create a clang attribute that lets users specify LLVM attributes

jdoerfert added a comment. Herald added a subscriber: dexonsmith. ping, could we come up with a design for this functionality? Comment at: llvm/lib/IR/Attributes.cpp:522 + .Default(Attribute::None); +} + can't we create this via tablegen? Repository: r

[PATCH] D91944: OpenMP 5.0 metadirective

jdoerfert added a comment. There are no test cases anymore, as far as I can tell. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D91944/new/ https://reviews.llvm.org/D91944 ___ cfe-commits mailing list cfe

[PATCH] D95971: [OpenMP][NVPTX] Take functions in `deviceRTLs` as `convergent`

jdoerfert added a comment. Please check in a clang FE test for the attribute as well, we have plenty test that use -fopenmp-is-device or add one as you see fit. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D95971/new/ https://reviews.llvm.org/D959

[PATCH] D95974: [CUDA, NVPTX] Allow targeting sm_86 GPUs.

jdoerfert accepted this revision. jdoerfert added a comment. This revision is now accepted and ready to land. One nit below. Seems reasonable in general. So we add ptx72 but it's not used with sm_86, interesting. Comment at: clang/lib/Driver/ToolChains/Cuda.cpp:103 Line

[PATCH] D91944: OpenMP 5.0 metadirective

jdoerfert accepted this revision. jdoerfert added a comment. Asssuming this passes the tests I'll merge it as is. There are unresolved review requests, e.g., custom implementation instead of balanced parenthesis trackers, the template support, etc. but we decided we can make faster progress in t

[PATCH] D95974: [CUDA, NVPTX] Allow targeting sm_86 GPUs.

jdoerfert added a comment. In D95974#2540352 , @tra wrote: >> So we add ptx72 but it's not used with sm_86, interesting. > > `ptx71` is the minimum/default requited PTX version for sm_86. If we compile > with CUDA-11.2, clang will set the '+ptx72' as we m

[PATCH] D95971: [OpenMP][NVPTX] Take functions in `deviceRTLs` as `convergent`

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

[PATCH] D95915: [clang][driver] Only warn once about invalid -stdlib value

jdoerfert added a comment. Test still missing. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D95915/new/ https://reviews.llvm.org/D95915 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cf

[PATCH] D96102: [clangd] Fix missing __syncthreads.

jdoerfert added a comment. I doubt this compiles: https://godbolt.org/z/c51v5d even with adjusted type, this fails because `__syncthreads` is a builtin (right now): https://godbolt.org/z/jP7e6h (and `__nvvm_bar_sync` requires an argument). CHANGES SINCE LAST ACTION https://reviews.llv

[PATCH] D96102: [clangd] Fix missing __syncthreads.

jdoerfert added a comment. In D96102#2545644 , @oToToT wrote: > OK, I've found what's wrong with my fix here. I should check for the clangd > compile mechanism with more detail to fix this problem. > > Sorry for disturbance. No worries. I'm not sure how

[PATCH] D94735: CGDebugInfo CreatedLimitedType: Drop file/line for RecordType with invalid location

jdoerfert added a comment. In D94735#2546968 , @dblaikie wrote: > This patch is about a clang synthesized `struct`. > `clang/lib/CodeGen/CGOpenMPRuntime.cpp:3463` has a synthesized `union` > (`distinct !DICompositeType(tag: DW_TAG_union_type, name: "kmp_

[PATCH] D94973: [clang][OpenMP] Use OpenMPIRBuilder for workshare loops.

jdoerfert added a comment. I have a single last comment/request. @jdenny I'll take it you finish the review and accept as you see fit. Comment at: llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp:986 + if (BodyGenCB) +BodyGenCB(CL->getBodyIP(), CL->getIndVar()); --

[PATCH] D109635: [OpenMP] Support construct trait set for Clang

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

[PATCH] D109990: [OpenMP][Docs] Update clang support based on community discussion

jdoerfert created this revision. jdoerfert added reviewers: ABataev, grokos, JonChesterfield, gregrodgers, ronl, RaviNarayanaswamy, dreachem. Herald added subscribers: guansong, bollu, yaxunl. jdoerfert requested review of this revision. Herald added subscribers: cfe-commits, sstefan1. Herald adde

[PATCH] D109997: [OpenMP] Change debugging symbol to weak_odr linkage

jdoerfert added a comment. You checked that this works with the usage in the RT, right? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D109997/new/ https://reviews.llvm.org/D109997 ___ cfe-commits mailing

[PATCH] D109997: [OpenMP] Change debugging symbol to weak_odr linkage

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

[PATCH] D110029: [OpenMP][Offloading] Use bitset to indicate execution mode instead of value

jdoerfert added inline comments. Comment at: clang/lib/CodeGen/CGOpenMPRuntimeGPU.cpp:1118 + llvm::GlobalValue::WeakAnyLinkage, + llvm::ConstantInt::get(CGM.Int8Ty, Mode ? 0x2 : 0x1), + Twine(Name, "_exec_mode")); The magic constants need to go int

[PATCH] D110108: [OpenMP] Add clang option to change device RTL stack size

jdoerfert added a comment. The runtime code is missing, right? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D110108/new/ https://reviews.llvm.org/D110108 ___ cfe-commits mailing list cfe-commits@lists.ll

[PATCH] D110029: [OpenMP][Offloading] Use bitset to indicate execution mode instead of value

jdoerfert added a comment. Just more style things, I think the rest is fine. Comment at: llvm/include/llvm/Frontend/OpenMP/OMPConstants.h:134 + SPMD = 0x2, +}; + If you copy the LLVM_MARK_AS_BITMASK_ENUM stuff you can actually use the enum class as bitmask w/

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

jdoerfert added a comment. In D102107#3013233 , @ronlieb wrote: > seeing buildbot failures after this patch landed > https://lab.llvm.org/staging/#/builders/183/builds/1598 This looks like another AMDGPU issue. The code in question doesn't do anything

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

jdoerfert added a comment. In D102107#3013437 , @ronlieb wrote: > Please revert the patch so our buildbot can resume greeness, and we can look > into it with urgency today (me or Jon) > as it should be reproducible Sounds good. @ggeorgakoudis let's reve

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

jdoerfert added a comment. In D102107#3014759 , @JonChesterfield wrote: > In D102107#3014743 , @pdhaliwal > wrote: > >> I got this after changing __kmpc_impl_malloc to return 0xdeadbeef. So, this >> confirms th

[PATCH] D110029: [OpenMP][Offloading] Use bitset to indicate execution mode instead of value

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

[PATCH] D110257: [CFE][Codegen] Do not break the contiguity of static allocas.

jdoerfert added a comment. In D110257#3015641 , @hsmhsm wrote: > In D110257#3015553 , @arsenm wrote: > >> I do think it's cleaner/more canonical IR to cluster these at the top of the >> block, but I don't underst

[PATCH] D110257: [CFE][Codegen] Do not break the contiguity of static allocas.

jdoerfert added a subscriber: rnk. jdoerfert added a comment. In D110257#3015712 , @jdoerfert wrote: > In D110257#3015641 , @hsmhsm wrote: > >> In D110257#3015553 , @arsen

[PATCH] D110279: [OpenMP][Offloading] Change `bool IsSPMD` to `int8_t Mode` in `__kmpc_target_init` and `__kmpc_target_deinit`

jdoerfert accepted this revision. jdoerfert added a comment. This revision is now accepted and ready to land. This should not change anything, right? Please confirm running a non-trivial example, like miniqmc or so, and then this is good to go. Repository: rG LLVM Github Monorepo CHANGES SIN

[PATCH] D110257: [CFE][Codegen] Do not break the contiguity of static allocas.

jdoerfert added inline comments. Comment at: clang/lib/CodeGen/CGExpr.cpp:103 +if (!ArraySize) { + auto *EBB = AllocaInsertPt->getParent(); + auto Iter = AllocaInsertPt->getIterator(); arsenm wrote: > Why is there a special AllocaInsertPt iterator i

[PATCH] D110429: [OpenMP] Introduce a new worksharing RTL function for distribute

jdoerfert accepted this revision. jdoerfert added a comment. This revision is now accepted and ready to land. LG. We might be able to revert/simplify this eventually again but for now it makes it much simpler to distinguish the distribute and workshare (=for) via their call site. ===

[PATCH] D110429: [OpenMP] Introduce a new worksharing RTL function for distribute

jdoerfert added a comment. In D110429#3024886 , @ronlieb wrote: > looks like subsequent builders passed. we might have an issue where the > runtime was not fully built when the test ran. > please disregard , your patch is fine. two dependent patches and

[PATCH] D110257: [CFE][Codegen] Do not break the contiguity of static allocas.

jdoerfert added a comment. While I understand people are eager to receive feedback on their patches, it is not helpful to ping/remind the reviewers constantly. This does generate noise for them and can consequently also reduce their interest in a patch. The recommendation for time without review

[PATCH] D110257: [CFE][Codegen] Do not break the contiguity of static allocas.

jdoerfert added inline comments. Comment at: clang/lib/CodeGen/CGExpr.cpp:151 + // allocas. + Builder.CreateStore(Init, Var); } hsmhsm wrote: > jdoerfert wrote: > > I'm not even sure this is necessarily correct. > > > > How do we know the new store is not in

[PATCH] D110257: [CFE][Codegen] Do not break the contiguity of static allocas.

jdoerfert added inline comments. Comment at: clang/lib/CodeGen/CGExpr.cpp:151 + // allocas. + Builder.CreateStore(Init, Var); } hsmhsm wrote: > jdoerfert wrote: > > hsmhsm wrote: > > > jdoerfert wrote: > > > > I'm not even sure this is necessarily correct. >

[PATCH] D110655: [OpenMP] Apply OpenMP assumptions to applicable call sites

jdoerfert accepted this revision. jdoerfert added a comment. This revision is now accepted and ready to land. LG, once we can propagate assumptions we can remove this again. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D110655/new/ https://reviews.

[PATCH] D91054: [Clang][OpenMP] Frontend work for sections - D89671

jdoerfert added a comment. In D91054#2548266 , @AMDChirag wrote: > @fghanim @jdoerfert please review the code if/when possible. Let's resolve the issue described in the TODO before we go ahead with this. There is little point reviewing something that can

[PATCH] D88645: [Annotation] Allows annotation to carry some additional constant arguments.

jdoerfert reopened this revision. jdoerfert added a comment. This revision is now accepted and ready to land. The lang ref needs to reflect the new type https://llvm.org/docs/LangRef.html#llvm-var-annotation-intrinsic Please also review the auto-upgrade patch for this: https://reviews.llvm.org/D

[PATCH] D88645: [Annotation] Allows annotation to carry some additional constant arguments.

jdoerfert requested changes to this revision. jdoerfert added a comment. This revision now requires changes to proceed. status change, see last message Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D88645/new/ https://reviews.llvm.org/D88645 __

[PATCH] D96418: [clang] Refactor mustprogress handling, add it to all loops in c++11+.

jdoerfert added inline comments. Comment at: clang/lib/CodeGen/CodeGenFunction.h:523 return getLangOpts().C11 || getLangOpts().C17 || getLangOpts().C2x; } Can you modify the documentation to talk about what loops must make progress, this is the code be

[PATCH] D96419: [clang] Add -ffinite-loops & -fno-finite-loops options.

jdoerfert added inline comments. Comment at: clang/include/clang/Basic/CodeGenOptions.h:147 +No = 2, // No loop is assumed to be finite. + }; + Can we have different names? `FiniteLoopsKind::None` sounds more like what ``FiniteLoopsKind::No` implies. May

[PATCH] D96418: [clang] Refactor mustprogress handling, add it to all loops in c++11+.

jdoerfert added a comment. Seems reasonable to me and makes things simpler. @atmnpatel Any thoughts? Comment at: clang/lib/CodeGen/CodeGenFunction.h:523 return getLangOpts().C11 || getLangOpts().C17 || getLangOpts().C2x; } fhahn wrote: > jdoerfert wro

[PATCH] D96419: [clang] Add -ffinite-loops & -fno-finite-loops options.

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

[PATCH] D95928: [OpenMP] Delay more diagnostics of potentially non-emitted code

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

[PATCH] D95903: [OpenMP][NFC] Pre-commit test changes regarding PR48933

jdoerfert added a comment. ping and follow up patches. Fixes PR49158 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D95903/new/ https://reviews.llvm.org/D95903 ___ cfe-commits mailing list cfe-commits@list

[PATCH] D95912: [OpenMP] Attribute target diagnostics properly

jdoerfert added a comment. friendly ping, also D95928 . These were supposed to be backported into 12 :( Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D95912/new/ https://reviews.llvm.org/D95912

[PATCH] D95912: [OpenMP] Attribute target diagnostics properly

jdoerfert added a comment. Thanks for taking a look! Comment at: clang/lib/Sema/Sema.cpp:1798 + FunctionDecl *FD = + isa(C) ? cast(C) : dyn_cast(D); auto CheckType = [&](QualType Ty) { Fznamznon wrote: > So, we assume that lexical context is a function

[PATCH] D95912: [OpenMP] Attribute target diagnostics properly

jdoerfert updated this revision to Diff 323799. jdoerfert marked an inline comment as done. jdoerfert added a comment. Rename variable as requested Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D95912/new/ https://reviews.llvm.org/D95912 Files: c

[PATCH] D95928: [OpenMP] Delay more diagnostics of potentially non-emitted code

jdoerfert marked 2 inline comments as done. jdoerfert added inline comments. Comment at: clang/lib/Sema/SemaDecl.cpp:18345 if (LangOpts.OpenMPIsDevice) { +// In OpenMP device mode we will not emit host only functions, or functions +// we don't need due to their linkage

[PATCH] D95903: [OpenMP][NFC] Pre-commit test changes regarding PR48933

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG3b2f19d0bc28: [OpenMP][NFC] Pre-commit test changes regarding PR48933 (authored by jdoerfert). Repository: rG LLVM Github Monorepo CHANGES SINCE

[PATCH] D95912: [OpenMP] Attribute target diagnostics properly

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGf9286b434b76: [OpenMP] Attribute target diagnostics properly (authored by jdoerfert). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTI

[PATCH] D95928: [OpenMP] Delay more diagnostics of potentially non-emitted code

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 rG1dd66e6111a8: [OpenMP] Delay more diagnostics of potentially non-emitted code (authored by jdoerfert). R

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

jdoerfert added a comment. Could you somewhere explain why emit-llvm-bc is not enough? This simply reverts its introduction without saying why. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D96769/new/ https://reviews.llvm.org/D96769 _

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

jdoerfert added a comment. In D96769#2565751 , @pdhaliwal wrote: > emit-llvm-bc does not correctly solve the problem. It works because [input, > compile, assemble, backend] actions collapse to a single action by driver. > This single command handles emit

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

jdoerfert added a comment. What you are telling me here is that -save-temps and -emit-llvm(-bc) can't work together right now. If that is so, why not fix it there instead of introducing a AMDGPU solution? There are other reasons you might want to combine those to options after all. Repository

[PATCH] D96877: [libomptarget] Try a fallback devicertl if the preferred one is missing

jdoerfert added a comment. Can we wrap this up and backport it, last known issue we should fix for 12. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D96877/new/ https://reviews.llvm.org/D96877 ___ cfe-com

[PATCH] D97003: [Clang][OpenMP] Require CUDA 9+ for OpenMP offloading on NVPTX target

jdoerfert added a comment. Could you include the simplifications this allows in the openmp subfolder? If it's too much put it in a child revision. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D97003/new/ https://reviews.llvm.org/D97003 __

[PATCH] D97003: [Clang][OpenMP] Require CUDA 9+ for OpenMP offloading on NVPTX target

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

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

jdoerfert added a comment. Herald added a subscriber: frasercrmck. Looking back at this I'm not so sure if it was beneficial to divert from the generic behavior and print each function for each prefix. Having a function only in one run line but not the other should work perfectly fine as long as

[PATCH] D97273: OpenMP: Fix object clobbering issue when using save-temps

jdoerfert accepted this revision. jdoerfert added a comment. This revision is now accepted and ready to land. LGTM, assuming it doesn't break support the reasoning makes sense. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D97273/new/ https://review

[PATCH] D103642: [OPENMP]Fix PR49790: Constexpr values not handled in `omp declare mapper` clause.

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

[PATCH] D103646: [OPENMP]Fix PR50129: omp cancel parallel not working as expected.

jdoerfert added a comment. Do we have to update `OpenMPIRBuilder::createCancel` too? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D103646/new/ https://reviews.llvm.org/D103646 ___ cfe-commits mailing lis

[PATCH] D103646: [OPENMP]Fix PR50129: omp cancel parallel not working as expected.

jdoerfert accepted this revision. jdoerfert added a comment. This revision is now accepted and ready to land. LG, one nit below. Comment at: llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h:538 +omp::Directive CanceledDirective, +

[PATCH] D93525: [clang-offload-bundler] Add unbundling of archives containing bundled object files into device specific archives

jdoerfert added a comment. Thanks for splitting this. I quickly went over it only. Comment at: clang/docs/ClangOffloadBundler.rst:137 + + myarch--myOS- not needed here. Comment at: clang/tools/clang-offload-bundler/ClangOffloadBundler.cpp:

[PATCH] D103958: [WIP] Support MustControl conditional control attribute

jdoerfert added a comment. WIP here is fine, would help to include a test that shows/explains what problem is actually solved though. I don't understand form this patch alone. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D103958/new/ https://revie

[PATCH] D103958: [WIP] Support MustControl conditional control attribute

jdoerfert added a comment. In D103958#2808767 , @nickdesaulniers wrote: > The first talk from https://www.youtube.com/watch?v=FFjV9f_Ub9o > (https://github.com/ClangBuiltLinux/plumbers-2020-slides/blob/master/LPC_2020_--_Dependency_ordering.pdf) > migh

[PATCH] D103958: [WIP] Support MustControl conditional control attribute

jdoerfert added a comment. > Please bear with me, I'm updating examples and documentation. I didn't think > anybody would look at this while [WIP]. :-) People try to help so you have early design feedback ;) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llv

[PATCH] D103958: [WIP] Support MustControl conditional control attribute

jdoerfert added subscribers: nhaehnle, rjmccall, arsenm. jdoerfert added a comment. This starts to sound an awful lot like `convergent` to me, basically: Do not change the control conditions of this call. Still unsure, maybe you can add a LangRef draft so we know what you try to do, or a nice ex

[PATCH] D103995: [OpenMP] Add type to firstprivate symbol for const firstprivate values

jdoerfert added inline comments. Comment at: clang/lib/CodeGen/CGOpenMPRuntime.cpp:10598 +CGM.getCXXABI().getMangleContext().mangleTypeName(VD->getType(), OS); +OS << "_" << VD->getName() << "_l" << Line; VarName = OS.str(); What if the code is not C

[PATCH] D104240: [OPENMP][C++20]Add support for CXXRewrittenBinaryOperator in ranged for loops.

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

[PATCH] D104258: [OPENMP]Fix PR50699: capture locals in combine directrives for aligned clause.

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

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

jdoerfert added a comment. (answered now, I forgot to submit this yesterday -.- ) In D102107#2819869 , @lebedev.ri wrote: > (This is not offload-specific, right?) It is not. It applies to all parallel regions. We still use the variadic call on the CPU

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

jdoerfert added a comment. In D102107#2821976 , @ABataev wrote: > We used this kind of codegen initially but later found out that it causes a > large overhead when gathering pointers into a record. What about hybrid > scheme where the first args are pas

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

jdoerfert updated this revision to Diff 352776. jdoerfert added a comment. Fix tests Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D101976/new/ https://reviews.llvm.org/D101976 Files: clang/lib/CodeGen/CGOpenMPRuntimeGPU.cpp clang/lib/CodeGen/C

[PATCH] D104677: [OpenMP][AMDGCN] Apply fix for isnan, isinf and isfinite for amdgcn.

jdoerfert accepted this revision. jdoerfert added a comment. This revision is now accepted and ready to land. LGTM, glad AMD GPUs are catching up on the math header support now. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D104677/new/ https://revi

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

jdoerfert added a comment. In D102107#2824581 , @ABataev wrote: > In D102107#2823706 , @jdoerfert > wrote: > >> In D102107#2821976 , @ABataev >> wrote: >> >>> We used th

[PATCH] D104714: [UpdateCCTestChecks] Support --check-globals

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

[PATCH] D104854: Introduce intrinsic llvm.isnan

jdoerfert added a comment. In D104854#2838495 , @sepavloff wrote: > In D104854#2838468 , @thopre wrote: > >> Are you planning to do this for the other FP test builtin (isinf, isfinite, >> isinf_sign, isnormal)? >

[PATCH] D93525: [clang-offload-bundler] Add unbundling of archives containing bundled object files into device specific archives

jdoerfert accepted this revision. jdoerfert added a comment. Looks reasonable to me. We can always refine it as we go. Comment at: clang/tools/clang-offload-bundler/ClangOffloadBundler.cpp:137 +/// * GPUArch (Optional) - Processor name, like gfx906 or sm_30 +/// In presence of

[PATCH] D102361: [OpenMP] Add Module metadata for OpenMP compilation

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

[PATCH] D102361: [OpenMP] Add Module metadata for OpenMP compilation

jdoerfert added inline comments. Comment at: clang/lib/CodeGen/CodeGenModule.cpp:709 +getModule().addModuleFlag(llvm::Module::Max, "openmp-device", + LangOpts.OpenMP); + Why do we need `OpenMPRuntime` in the conditional? Reposit

[PATCH] D107668: [OpenMP]Fix PR50336: Remove temporary files in the offload bundler tool

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

[PATCH] D107668: [OpenMP]Fix PR50336: Remove temporary files in the offload bundler tool

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

[PATCH] D107971: [openmp] Annotate tmp variables with omp_thread_mem_alloc

jdoerfert accepted this revision. jdoerfert added a comment. This revision is now accepted and ready to land. Assuming this causes us to generate an `alloc as(5)` for `__tmp`, LG Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D107971/new/ https://rev

[PATCH] D107971: [openmp] Annotate tmp variables with omp_thread_mem_alloc

jdoerfert added a comment. In D107971#2941739 , @JonChesterfield wrote: > Failed a CI job that builds an openmp test in an environment without omp.h, > will revert. > > Thoughts on fixing? Putting the omp allocator definition in this header is > likely

[PATCH] D108246: [clang-offload-wrapper] Disabled ELF offload notes embedding by default.

jdoerfert requested changes to this revision. jdoerfert added a comment. This revision now requires changes to proceed. I would prefer to revert 93d08acaacec951dbb302f77eeae51974985b6b2 now and then fix the CUDA toolchain rath

[PATCH] D108246: [clang-offload-wrapper] Disabled ELF offload notes embedding by default.

jdoerfert accepted this revision. jdoerfert added a comment. This revision is now accepted and ready to land. As discussed, LG, we will look into the NVIDIA GPU problem now to get rid of this again. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D108246/new/ https://reviews.llvm.org/D108

[PATCH] D106960: [OffloadArch] Library to query properties of current offload archicture

jdoerfert added inline comments. Comment at: llvm/lib/OffloadArch/OffloadArch.cpp:280 + return results; +} the _aot_ names are not great. Comment at: llvm/lib/OffloadArch/amdgpu/hsa-subset.h:40 +// DEALINGS WITH THE SOFTWARE. +// +

[PATCH] D108317: [libomptarget][devicertl] Replace lanemask with uint64 at interface

jdoerfert accepted this revision. jdoerfert added a comment. This revision is now accepted and ready to land. LG. We could replace `LaneMaskTy` in the new dev rtl but that is not necessary. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D108317/new/

[PATCH] D108380: [openmp][nfc] Refactor GridValues

jdoerfert added inline comments. Comment at: llvm/include/llvm/Frontend/OpenMP/OMPGridValues.h:102 +return R; + } }; It should be in the device rtl then, no? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D1083

[PATCH] D108380: [openmp][nfc] Refactor GridValues

jdoerfert added inline comments. Comment at: llvm/include/llvm/Frontend/OpenMP/OMPGridValues.h:102 +return R; + } }; JonChesterfield wrote: > jdoerfert wrote: > > It should be in the device rtl then, no? > This header is currently used from clang and the (a

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

jdoerfert added a comment. Really looking forward to this! Thanks a lot! I left some comments. Comment at: clang/lib/Headers/__clang_hip_math.h:35 +#ifdef __OPENMP_AMDGCN__ +#define __RETURN_TYPE int +#else JonChesterfield wrote: > I'd expect openmp to match t

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

jdoerfert updated this revision to Diff 355391. jdoerfert added a comment. Herald added a subscriber: ormris. Adjust AAExecutionDomain properly to account for the new target agnostic kernel init, this makes D104911 obsolete. Repository: rG LLVM Github Monore

[PATCH] D105143: [OPENMP]Fix PR50929: Ignored initializer clause in user-defined reduction.

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

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

jdoerfert added a comment. OpenMP side looks reasonable. Comment at: clang/lib/Headers/__clang_hip_cmath.h:96 +__DEVICE__ __CONSTEXPR__ bool isnan(float __x) { return ::__isnanf(__x); } +__DEVICE__ __CONSTEXPR__ bool isnan(double __x) { return ::__isnan(__x); } --

[PATCH] D105191: [Clang][OpenMP] Add support for Static Device Libraries

jdoerfert added a comment. Not a thorough review but comments to address. Comment at: clang/lib/Driver/ToolChains/CommonArgs.cpp:1662 + std::string archname = ArchName.str(); + std::string gpuname = GPUArch.str(); + Coding convention. Comme

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

jdoerfert added inline comments. Comment at: clang/lib/Headers/__clang_hip_cmath.h:96 +__DEVICE__ __CONSTEXPR__ bool isnan(float __x) { return ::__isnanf(__x); } +__DEVICE__ __CONSTEXPR__ bool isnan(double __x) { return ::__isnan(__x); } pdhaliwal wrote: > jdoe

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

jdoerfert updated this revision to Diff 355958. jdoerfert added a comment. Rebase, only look for __kmpc_target_init in AAExecutionDomain as that will isolate the initial thread. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D101976/new/ https://rev

[PATCH] D105221: [openmp][nfc] Simplify macros guarding math complex headers

jdoerfert requested changes to this revision. jdoerfert added a comment. We need a macro for OPENMP and one for OPENMP_OFFLOAD, we can use a single one for the latter and avoid _NVPTX, _AMDGCN, ... but we need both as described by @fodinabor. Repository: rG LLVM Github Monorepo CHANGES SINC

[PATCH] D105322: [NFC][OpenMP][CUDA] Add test for using `-x cuda -fopenmp`

jdoerfert accepted this revision. jdoerfert added a comment. This revision is now accepted and ready to land. Wow, thanks for the CUDA stubs! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D105322/new/ https://reviews.llvm.org/D105322 __

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