[PATCH] D80735: [OpenMP][NFC] Reuse OMPIRBuilder `struct ident_t` handling in Clang

jdoerfert added a comment. In D80735#2073639 , @jhuber6 wrote: > The failure message is pretty much always this assertion as far as I can tell > > clang-11: > /home/jhuber/Documents/llvm-project/llvm/lib/IR/Instructions.cpp:439: void > llvm::CallInst::

[PATCH] D81192: [OPENMP]Fix PR45854: prevent code movement out of the critical region.

jdoerfert added a comment. I doubt this is the right fix. I'll take a closer look. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D81192/new/ https://reviews.llvm.org/D81192 ___ cfe-commits mailing list

[PATCH] D81192: [OPENMP]Fix PR45854: prevent code movement out of the critical region.

jdoerfert added a comment. In D81192#2074762 , @ABataev wrote: > In D81192#2074732 , @jdoerfert wrote: > > > I doubt this is the right fix. I'll take a closer look. > > > I believe, the issue is caused by the new at

[PATCH] D80917: [OpenMP][AMDGCN] Support OpenMP offloading for AMDGCN architecture - Part 2

jdoerfert added inline comments. Comment at: clang/include/clang/Basic/TargetInfo.h:1333 +return GridValues[gv]; + } + I somehow believe `!GridValues` is the wrong way around. You should also initialize it directly to null so this makes more sense. The ret

[PATCH] D80917: [OpenMP][AMDGCN] Support OpenMP offloading for AMDGCN architecture - Part 2

jdoerfert added a comment. I am fine with this. @arsenm feel free to accept. --- --- In D80917#2085138 , @saiislam wrote: > Ping. Afaik, pings should be reserved to ~1 week of inactivity. I know this can be frustrating, but we also need to consider no

[PATCH] D78232: [OPENMP50]Codegen for scan directive in simd loops.

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

[PATCH] D80730: [OPENMP50]Codegen for use_device_addr clauses.

jdoerfert accepted this revision. jdoerfert added a comment. This revision is now accepted and ready to land. One nit below, the others seem silent. LGTM. Comment at: clang/lib/Sema/SemaOpenMP.cpp:18508 + // Component = CreateBuiltinUnaryOp(ELoc, UO_Deref, Component).get()

[PATCH] D79675: [OpenMP][OMPBuilder] Adding Privatization Requirements to OMPIRBuilder

jdoerfert added subscribers: raghavendhra, jhuber6, anchu-rajendran, kiranktp, DavidTruby, ronlieb, kiranchandramohan. jdoerfert added a comment. In D79675#2058657 , @fghanim wrote: > I am moving on because we are not getting anywhere. However, There are

[PATCH] D81311: [RFC] LangRef: Define inmem parameter attribute

jdoerfert added a comment. Do we allow `inmem` to be used for other purposes? I would assume the answer is yes, as we do not forbid it. I ask because front-ends might use this as a way to add type information to a pointer (especially after we strip them). Or am I confusing something here? > `by

[PATCH] D81311: [RFC] LangRef: Define inmem parameter attribute

jdoerfert added a comment. In D81311#2086326 , @rjmccall wrote: > In D81311#2086227 , @jdoerfert wrote: > > > Do we allow `inmem` to be used for other purposes? I would assume the > > answer is yes, as we do not fo

[PATCH] D81641: [SYCL] Implement thread-local storage restriction

jdoerfert added a comment. OpenMP has the same restriction (no surprise I guess). Thanks for the ping! I think we do not emit diagnosis right now: https://godbolt.org/z/srDkXZ I think we also should diagnose this the same way, though it might be beyond the scope of this patch: https://godbolt.or

[PATCH] D81311: [RFC] LangRef: Define inmem parameter attribute

jdoerfert added a comment. In D81311#2088075 , @rjmccall wrote: > In D81311#2087592 , @jdoerfert wrote: > > > In D81311#2086326 , @rjmccall > > wrote: > > > > > In D81311#20

[PATCH] D81311: [RFC] LangRef: Define inmem parameter attribute

jdoerfert added a comment. > But it does mess with the ABI; that's why it's being added. My worry is, that we do not clearly sate it does. I'm worried front-ends will use it to attach types to pointers once pointers do not have types anymore. I'll stop arguing on this one now, if you think this

[PATCH] D81736: [openmp] Base of tablegen generated OpenMP common declaration

jdoerfert added reviewers: fghanim, ABataev, jdenny, hfinkel, jhuber6, kiranchandramohan, kiranktp. jdoerfert added a comment. This is really cool :) In D81736#2090419 , @clementval wrote: > @jdoerfert Some tests in clang relies on the position of the sp

[PATCH] D81736: [openmp] Base of tablegen generated OpenMP common declaration

jdoerfert added a comment. Assuming this passes all the tests, it looks good to me. Let's wait a day or two for people to take a look though :) Comment at: llvm/test/TableGen/directive.td:34 +// CHECK-NEXT: } +// CHECK-NEXT: } clementval wrote: > jdoerfert wro

[PATCH] D81881: [OPENMP]Fix overflow during counting the number of iterations.

jdoerfert added a comment. So the idea is to do the trip count computation with defined overflow behavior, e.g., without `nsw/nuw` in IR, right? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D81881/new/ https://reviews.llvm.org/D81881 __

[PATCH] D81658: [OPENMP50]Codegen for scan directive in for simd regions.

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

[PATCH] D81478: [OPENMP50]Codegen for scan directives in parallel for regions.

jdoerfert added inline comments. Comment at: clang/lib/CodeGen/CGStmtOpenMP.cpp:3627 + emitScanBasedDirective(CGF, S, NumIteratorsGen, FirstGen, SecondGen); +} else { + OMPCancelStackRAII CancelRegion(CGF, OMPD_parallel_for, S.hasCancel()); This loo

[PATCH] D81881: [OPENMP]Fix overflow during counting the number of iterations.

jdoerfert accepted this revision. jdoerfert added a comment. This revision is now accepted and ready to land. In D81881#2095509 , @ABataev wrote: > In D81881#2094062 , @jdoerfert wrote: > > > So the idea is to do th

[PATCH] D81736: [openmp] Base of tablegen generated OpenMP common declaration

jdoerfert added a comment. In D81736#2095947 , @clementval wrote: > In D81736#2093926 , @jdoerfert wrote: > > > Assuming this passes all the tests, it looks good to me. Let's wait a day > > or two for people to tak

[PATCH] D81641: [SYCL][OpenMP] Implement thread-local storage restriction

jdoerfert accepted this revision. jdoerfert added a comment. In D81641#2096114 , @Fznamznon wrote: > In D81641#2088446 , @jdoerfert wrote: > > > OpenMP has the same restriction (no surprise I guess). Thanks for the

[PATCH] D81951: [OPENMP]Fix PR46347: several ordered directives in a single region.

jdoerfert accepted this revision. jdoerfert added a comment. This revision is now accepted and ready to land. LGTM, can you add the nested ordered + depend test case too, assuming we don't have it. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D8195

[PATCH] D81478: [OPENMP50]Codegen for scan directives in parallel for regions.

jdoerfert added inline comments. Comment at: clang/lib/CodeGen/CGStmtOpenMP.cpp:3627 + emitScanBasedDirective(CGF, S, NumIteratorsGen, FirstGen, SecondGen); +} else { + OMPCancelStackRAII CancelRegion(CGF, OMPD_parallel_for, S.hasCancel()); ABataev

[PATCH] D81478: [OPENMP50]Codegen for scan directives in parallel for regions.

jdoerfert added inline comments. Comment at: clang/lib/CodeGen/CGStmtOpenMP.cpp:3627 + emitScanBasedDirective(CGF, S, NumIteratorsGen, FirstGen, SecondGen); +} else { + OMPCancelStackRAII CancelRegion(CGF, OMPD_parallel_for, S.hasCancel()); ABataev

[PATCH] D79675: [OpenMP][OMPBuilder] Adding Privatization Requirements to OMPIRBuilder

jdoerfert accepted this revision. jdoerfert added a comment. This revision is now accepted and ready to land. LGTM. Thanks a lot! Sorry for the delay in reviewing. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D79675/new/ https://reviews.llvm.org/D7

[PATCH] D82019: [OPENMP]Fix PR46357: Do not allow types declarations in pragmas.

jdoerfert added reviewers: dreachem, kkwli0, trws. jdoerfert added a comment. Hm, does the standard say this is not allowed? I can see that we don't want it but I'm not 100% certain here. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D82019/new/ ht

[PATCH] D71739: [AssumeBundles] Use operand bundles to encode alignment assumptions

jdoerfert added a comment. I think this should go on, could you address the minor comments so we can merge it? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71739/new/ https://reviews.llvm.org/D71739

[PATCH] D80911: AArch64+ARM: make LLVM consider system registers volatile to prevent unsound optimizations.

jdoerfert added a comment. As an alternative, and necessary regardless, we need to teach AA that "no-`nosync`" operations can experience the effects of external things. As long as `llvm.read_register` is not marked `nosync` that should prevent hoisting. Should, because AA doesn't know this yet.

[PATCH] D78478: [UpdateTestChecks] Add UTC_ARGS support for update_{llc,cc}_test_checks.py

jdoerfert accepted this revision. jdoerfert added a comment. This revision is now accepted and ready to land. Nice cleanup again! I think eventually we need to teach the common logic to recognize top-level entities, e.g., functions, and with these changes it will be accessible w/o duplication i

[PATCH] D82659: Fix missing build dependency on omp_gen.

jdoerfert added a comment. Thank you both for figuring this out! *Much* appreciated! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D82659/new/ https://reviews.llvm.org/D82659 ___ cfe-commits mailing li

[PATCH] D78478: [UpdateTestChecks] Add UTC_ARGS support for update_{llc,cc}_test_checks.py

jdoerfert added a comment. In D78478#2128926 , @arichardson wrote: > In D78478#2128604 , @MaskRay wrote: > > > The idea look good to me, but I want some opinions on the name `UTC_ARGS` > > (I can't help associating

[PATCH] D80897: [OpenMP] Initial support for std::complex in target regions

jdoerfert added a comment. @JonChesterfield @hfinkel @tra ping I would really like to land this before the release branches off to allow people to use complex in target regions. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D80897/new/ https://rev

[PATCH] D83268: [OpenMP][NFC] Remove unused (always fixed) arguments

jdoerfert created this revision. jdoerfert added reviewers: jhuber6, fghanim, JonChesterfield, grokos, AndreyChurbanov, ye-luo, tianshilei1992, ggeorgakoudis. Herald added subscribers: llvm-commits, cfe-commits, sstefan1, guansong, bollu, yaxunl, jholewinski. Herald added projects: clang, OpenMP,

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

jdoerfert added inline comments. Comment at: llvm/utils/update_cc_test_checks.py:133 + parser.add_argument('--include-generated-funcs', action='store_true', + help='Output checks for functions not in source') parser.add_argument('tests', nargs='+') --

[PATCH] D83281: [OpenMP] Allow traits for the OpenMP context selector `isa`

jdoerfert created this revision. jdoerfert added reviewers: jhuber6, fghanim, JonChesterfield, grokos, ggeorgakoudis, ABataev. Herald added subscribers: llvm-commits, sstefan1, guansong, bollu, hiraditya, yaxunl. Herald added projects: clang, LLVM. NOTE: The changes are fairly mechanical overall

[PATCH] D83268: [OpenMP][NFC] Remove unused (always fixed) arguments

jdoerfert added a comment. In D83268#2135081 , @Hahnfeld wrote: > This is definitely not NFC and breaks API compatibility (but apparently > nobody cares anymore?). This is the device RTL. I am not aware we (want to) keep the API stable. If we are, I'm

[PATCH] D83268: [OpenMP][NFC] Remove unused (always fixed) arguments

jdoerfert marked an inline comment as done. jdoerfert added a comment. > I don't think gcc can be using this runtime library for nvptx. Yes, and: We are (going to) use clang specific intrinsics to avoid CUDA (soon). > Use of the new library with the previous version of the compiler. Except that

[PATCH] D83268: [OpenMP][NFC] Remove unused (always fixed) arguments

jdoerfert added a comment. > Especially taking into account, that libomp can be built separately. This is *not* affecting libomp in any way. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D83268/new/ https://reviews.llvm.org/D83268 __

[PATCH] D83268: [OpenMP][NFC] Remove unused (always fixed) arguments

jdoerfert added a comment. In D83268#2135989 , @ABataev wrote: > In D83268#2135954 , @JonChesterfield > wrote: > > > What can libomp be built by separately? Nvcc and gcc don't use this > > runtime. That leaves us

[PATCH] D83268: [OpenMP][NFC] Remove unused (always fixed) arguments

jdoerfert added a comment. In D83268#2135988 , @ABataev wrote: > In D83268#2135955 , @jdoerfert wrote: > > > > Especially taking into account, that libomp can be built separately. > > > > This is *not* affecting lib

[PATCH] D83268: [OpenMP][NFC] Remove unused (always fixed) arguments

jdoerfert added a comment. In D83268#2136031 , @ABataev wrote: > There is still compatibility between clang10 and clang11. Or they are > incompatible in LLVM IR level? That is the point. They might or might not be, right? There is no guarantee they are

[PATCH] D83268: [OpenMP][NFC] Remove unused (always fixed) arguments

jdoerfert added a comment. In D83268#2136029 , @ABataev wrote: > `llvm-project/openmp/libomptarget` Please use more words. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D83268/new/ https://reviews.llvm.org

[PATCH] D83268: [OpenMP][NFC] Remove unused (always fixed) arguments

jdoerfert added a comment. In D83268#2136055 , @ABataev wrote: > In D83268#2136054 , @jdoerfert wrote: > > > In D83268#2136029 , @ABataev wrote: > > > > > `llvm-project/openm

[PATCH] D80897: [OpenMP] Initial support for std::complex in target regions

jdoerfert updated this revision to Diff 276053. jdoerfert marked an inline comment as done. jdoerfert added a comment. Addressed comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D80897/new/ https://reviews.llvm.org/D80897 Files: clang/lib/H

[PATCH] D83268: [OpenMP][NFC] Remove unused (always fixed) arguments

jdoerfert added a comment. In D83268#2136060 , @ABataev wrote: > Better to ask the users. Maybe, send an RFC to openmp-devs? Sure: http://lists.llvm.org/pipermail/openmp-dev/2020-July/003531.html Repository: rG LLVM Github Monorepo CHANGES SINCE LAS

[PATCH] D83349: [OpenMP][NFC] Remove unused and untested code from the device runtime

jdoerfert created this revision. jdoerfert added reviewers: hfinkel, jhuber6, fghanim, JonChesterfield, grokos, AndreyChurbanov, ye-luo, tianshilei1992, ggeorgakoudis, Hahnfeld, ABataev, hbae, ronlieb, gregrodgers. Herald added subscribers: cfe-commits, aaron.ballman, sstefan1, jfb, guansong, bo

[PATCH] D83349: [OpenMP][NFC] Remove unused and untested code from the device runtime

jdoerfert added a comment. Please let me know if something slipped through my "clever" grep logic and is actually used. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D83349/new/ https://reviews.llvm.org/D83349 ___

[PATCH] D82906: [flang][openmp] Use common Directive and Clause enum from llvm/Frontend

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

[PATCH] D83176: [OpenMPIRBuilder][Fix] Move llvm::omp::types to OpenMPIRBuilder.

jdoerfert accepted this revision. jdoerfert added a comment. This revision is now accepted and ready to land. LGTM. Comment at: llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp:1125 +#include "llvm/Frontend/OpenMP/OMPKinds.def" +} While we are here, remove `uninitiali

[PATCH] D71739: [AssumeBundles] Use operand bundles to encode alignment assumptions

jdoerfert accepted this revision. jdoerfert added a comment. LGTM. @lebedev.ri ? Comment at: llvm/lib/Transforms/Scalar/AlignmentFromAssumptions.cpp:227 } - - if (!AAPtr) -return false; - - // Sign extend the offset to 64 bits (so that it is like all of the other - //

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

jdoerfert added inline comments. Comment at: llvm/utils/update_cc_test_checks.py:133 + parser.add_argument('--include-generated-funcs', action='store_true', + help='Output checks for functions not in source') parser.add_argument('tests', nargs='+') --

[PATCH] D80897: [OpenMP] Initial support for std::complex in target regions

This revision was automatically updated to reflect the committed changes. Closed by commit rGd999cbc98832: [OpenMP] Initial support for std::complex in target regions (authored by jdoerfert). Changed prior to commit: https://reviews.llvm.org/D80897?vs=276053&id=276584#toc Repository: rG LLVM

[PATCH] D83591: [OpenMP][CUDA] Fix std::complex in GPU regions

jdoerfert created this revision. jdoerfert added reviewers: tra, hfinkel, JonChesterfield. Herald added subscribers: sstefan1, guansong, bollu, yaxunl. Herald added a project: clang. The old way worked to some degree for C++-mode but in C mode we actually tried to introduce variants of macros (e.g

[PATCH] D83492: [OpenMP] Use common interface to access GPU Grid Values

jdoerfert added inline comments. Comment at: clang/lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp:654 + unsigned LaneIDBits = + CGF.getTarget().getGridValue(llvm::omp::GV_Warp_Size_Log2); return Bld.CreateAShr(getNVPTXThreadID(CGF), LaneIDBits, "nvptx_warp_id"); W

[PATCH] D83591: [OpenMP][CUDA] Fix std::complex in GPU regions

jdoerfert added a comment. In D83591#2145411 , @tra wrote: > In D83591#2145378 , @JonChesterfield > wrote: > > > Fine by me. Let's get nvptx working properly in tree now and work out how > > to wire up amdgcn subs

[PATCH] D78155: [OpenMP] Use __OPENMP_NVPTX__ instead of _OPENMP in wrapper headers

This revision was automatically updated to reflect the committed changes. Closed by commit rG7f1e6fcff942: [OpenMP] Use __OPENMP_NVPTX__ instead of _OPENMP in wrapper headers (authored by jdoerfert). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D7815

[PATCH] D83591: [OpenMP][CUDA] Fix std::complex in GPU regions

jdoerfert updated this revision to Diff 277173. jdoerfert added a comment. Keep the std:: functions in non-OpenMP mode Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D83591/new/ https://reviews.llvm.org/D83591 Files: clang/lib/Headers/__clang_cuda

[PATCH] D83478: [OPENMP]Fix compiler crash for target data directive without actual target codegen.

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

[PATCH] D83349: [OpenMP][NFC] Remove unused and untested code from the device runtime

This revision was automatically updated to reflect the committed changes. Closed by commit rGcd0ea03e6f15: [OpenMP][NFC] Remove unused and untested code from the device runtime (authored by jdoerfert). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D83

[PATCH] D83591: [OpenMP][CUDA] Fix std::complex in GPU regions

jdoerfert added a comment. Thx for the reviews! FWIW, OpenMP should be able to use the C/C++ standard functions/macros for this eventually. Getting the overloads right if you don't have type system support is tricky though and I need more time... On a separate note, we should bundle the resour

[PATCH] D83591: [OpenMP][CUDA] Fix std::complex in GPU regions

This revision was automatically updated to reflect the committed changes. Closed by commit rGb5667d00e044: [OpenMP][CUDA] Fix std::complex in GPU regions (authored by jdoerfert). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D83591/new/ https://revie

[PATCH] D83268: [OpenMP][NFC] Remove unused (always fixed) arguments

This revision was automatically updated to reflect the committed changes. Closed by commit rGc98699582a63: [OpenMP][NFC] Remove unused (always fixed) arguments (authored by jdoerfert). Changed prior to commit: https://reviews.llvm.org/D83268?vs=275871&id=277218#toc Repository: rG LLVM Github

[PATCH] D83707: [OpenMP][NFC] Emit remarks during GPU state machine optimization

jdoerfert created this revision. jdoerfert added reviewers: jhuber6, fghanim, JonChesterfield, grokos, AndreyChurbanov, ye-luo, tianshilei1992, ggeorgakoudis. Herald added subscribers: llvm-commits, cfe-commits, sstefan1, guansong, bollu, hiraditya, yaxunl. Herald added projects: clang, LLVM. Si

[PATCH] D83707: [OpenMP][NFC] Emit remarks during GPU state machine optimization

jdoerfert updated this revision to Diff 277524. jdoerfert added a comment. simplify test Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D83707/new/ https://reviews.llvm.org/D83707 Files: clang/test/OpenMP/remarks_parallel_in_target_state_machine.c

[PATCH] D82470: [OpenMP][IRBuilder] Support allocas in nested parallel regions

jdoerfert updated this revision to Diff 277634. jdoerfert added a comment. Rebase Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D82470/new/ https://reviews.llvm.org/D82470 Files: clang/lib/CodeGen/CGStmtOpenMP.cpp llvm/include/llvm/Frontend/Ope

[PATCH] D82470: [OpenMP][IRBuilder] Support allocas in nested parallel regions

jdoerfert added a comment. Used an extra argument now. Please let me know if this is OK. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D82470/new/ https://reviews.llvm.org/D82470 ___ cfe-commits mailing

[PATCH] D82822: [OpenMP][FIX] Consistently use OpenMPIRBuilder if requested

jdoerfert updated this revision to Diff 277635. jdoerfert added a comment. Rebase Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D82822/new/ https://reviews.llvm.org/D82822 Files: clang/lib/CodeGen/CGOpenMPRuntime.cpp clang/test/OpenMP/cancel_co

[PATCH] D82470: [OpenMP][IRBuilder] Support allocas in nested parallel regions

jdoerfert marked an inline comment as done. jdoerfert added a comment. I'll address the nits. Unsure if that is all. I also don't get the one comment. Comment at: llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp:433 - Builder.SetInsertPoint(OuterFn->getEntryBlock().getFirstNonPHI()

[PATCH] D83707: [OpenMP][NFC] Emit remarks during GPU state machine optimization

jdoerfert updated this revision to Diff 277889. jdoerfert added a comment. Improve wording Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D83707/new/ https://reviews.llvm.org/D83707 Files: clang/test/OpenMP/remarks_parallel_in_target_state_machine

[PATCH] D83707: [OpenMP][NFC] Emit remarks during GPU state machine optimization

jdoerfert updated this revision to Diff 277951. jdoerfert added a comment. Improve wording (again) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D83707/new/ https://reviews.llvm.org/D83707 Files: clang/test/OpenMP/remarks_parallel_in_target_state

[PATCH] D83707: [OpenMP][NFC] Emit remarks during GPU state machine optimization

jdoerfert updated this revision to Diff 277962. jdoerfert added a comment. Add SPMD tests Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D83707/new/ https://reviews.llvm.org/D83707 Files: clang/test/OpenMP/remarks_parallel_in_target_state_machine.

[PATCH] D83832: [OpenMP] Provide a flag to disable safety checks for GPU optimizations

jdoerfert created this revision. jdoerfert added reviewers: jhuber6, fghanim, JonChesterfield, grokos, AndreyChurbanov, ye-luo, tianshilei1992, ggeorgakoudis. Herald added subscribers: llvm-commits, cfe-commits, sstefan1, guansong, bollu, hiraditya, yaxunl. Herald added projects: clang, LLVM. Th

[PATCH] D82470: [OpenMP][IRBuilder] Support allocas in nested parallel regions

jdoerfert marked an inline comment as done. jdoerfert added inline comments. Comment at: llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp:433 - Builder.SetInsertPoint(OuterFn->getEntryBlock().getFirstNonPHI()); - AllocaInst *TIDAddr = Builder.CreateAlloca(Int32, nullptr, "tid.addr")

[PATCH] D83832: [OpenMP] Provide a flag to disable safety checks for GPU optimizations

jdoerfert added a comment. In D83832#2152027 , @JonChesterfield wrote: > I think there's an unfortunate interaction with link time optimisation here. > If there are external regions, but their code is combined with llvm-link > before codegen, then a use

[PATCH] D83707: [OpenMP][NFC] Emit remarks during GPU state machine optimization

This revision was automatically updated to reflect the committed changes. Closed by commit rGfec1f2109f33: [OpenMP] Emit remarks during GPU state machine optimization (authored by jdoerfert). Changed prior to commit: https://reviews.llvm.org/D83707?vs=277962&id=278059#toc Repository: rG LLVM

[PATCH] D82722: [OpenMP][IRBuilder] Support nested parallel regions

This revision was automatically updated to reflect the committed changes. Closed by commit rG7af287d0d921: [OpenMP][IRBuilder] Support nested parallel regions (authored by jdoerfert). Changed prior to commit: https://reviews.llvm.org/D82722?vs=273959&id=278061#toc Repository: rG LLVM Github

[PATCH] D83281: [OpenMP] Allow traits for the OpenMP context selector `isa`

jdoerfert updated this revision to Diff 278175. jdoerfert added a comment. Add warning and negative test Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D83281/new/ https://reviews.llvm.org/D83281 Files: clang/include/clang/AST/OpenMPClause.h cla

[PATCH] D83696: [OPENMP]Fix PR46688: cast the type of the allocated variable to the initial one.

jdoerfert accepted this revision. jdoerfert added a comment. This revision is now accepted and ready to land. LGTM. We might need to pick this for 11. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D83696/new/ https://reviews.llvm.org/D83696 _

[PATCH] D83334: [OPENMP]Fix PR46593: Reduction initializer missing construnctor call.

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

[PATCH] D82026: [OPENMP50]Allow nonmonotonic modifier for all schedule kinds.

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

[PATCH] D81478: [OPENMP50]Codegen for scan directives in parallel for regions.

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

[PATCH] D82019: [OPENMP]Fix PR46357: Do not allow types declarations in pragmas.

jdoerfert accepted this revision. jdoerfert added a comment. This revision is now accepted and ready to land. I agree it is not a good idea. Let's hope no one will complain :) LGTM. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D82019/new/ https://r

[PATCH] D79249: [NOT FOR REVIEW] Experimental support for zero-or-trap behavior foruninitialized variables.

jdoerfert added a comment. FWIW, I tried to solve something similar the other day. My solution sketch looked like this: https://godbolt.org/z/bRQPjd The idea would be that we teach DSE (and others) to remove the `llvm.undef.init` intrinsic if the location is overwritten. In the example above onl

[PATCH] D81311: [RFC] LangRef: Define byref parameter attribute

jdoerfert added a comment. LGTM, @rjmccall wdyt? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D81311/new/ https://reviews.llvm.org/D81311 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinf

[PATCH] D81736: [openmp] Base of tablegen generated OpenMP common declaration

jdoerfert accepted this revision. jdoerfert added a comment. Happy. LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D81736/new/ https://reviews.llvm.org/D81736 ___ cfe-commits mailing list cfe-commit

[PATCH] D81678: Introduce frozen attribute at call sites for stricter poison analysis

jdoerfert requested changes to this revision. jdoerfert added a comment. This revision now requires changes to proceed. First, apologies for being late, I didn't properly monitor the list recently. --- This diff is impossible to review and later to understand. I would ask you to split it, at le

[PATCH] D82115: [OPENMP50]Codegen for scan directives in parallel for simd regions.

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

[PATCH] D81678: Introduce frozen attribute at call sites for stricter poison analysis

jdoerfert added inline comments. Comment at: clang/include/clang/Driver/CC1Options.td:507 +def disable_frozen_args : Flag<["-"], "disable-frozen-args">, + HelpText<"Disable emitting frozen attribute in LLVM IR">; def load : Separate<["-"], "load">, MetaVarName<"">,

[PATCH] D81678: Introduce frozen attribute at call sites for stricter poison analysis

jdoerfert added a comment. In D81678#2109059 , @nlopes wrote: > I'm a bit concerned with this patch as it increases the amount of UB that > LLVM exploits without any study of the impact. > For example, right now it's ok do this with clang (not with const

[PATCH] D82317: [Clang/Test]: Update tests where `frozen` attribute is necessary

jdoerfert added a comment. > This very large diff affects more than 1,000 files. Use the Changeset List to > browse changes. This we will "look" at once the rest is settled ;) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D82317/new/ https://revie

[PATCH] D81736: [openmp] Base of tablegen generated OpenMP common declaration

jdoerfert added inline comments. Comment at: llvm/include/llvm/Frontend/OpenMP/CMakeLists.txt:2 +set(LLVM_TARGET_DEFINITIONS OMP.td) +tablegen(LLVM OMP.h.inc --gen-directive-decls) +add_public_tablegen_target(omp_gen) thakis wrote: > All other tblgen outputs are

[PATCH] D82224: [OpenMP][NFC] Remove hard-coded line numbers from test

jdoerfert accepted this revision. jdoerfert added a comment. This revision is now accepted and ready to land. LGTM. Thx! Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D82224/new/ https://reviews.llvm.org/D82224 ___ cf

[PATCH] D71739: [AssumeBundles] Use operand bundles to encode alignment assumptions

jdoerfert accepted this revision. jdoerfert added a comment. This revision is now accepted and ready to land. Two nits and one question about removed check lines. If they can be re-added, LGTM. Otherwise we need to look into that. Comment at: clang/lib/CodeGen/CodeGenFunction.

[PATCH] D81678: Introduce frozen attribute at call sites for stricter poison analysis

jdoerfert added inline comments. Comment at: clang/lib/CodeGen/CGCall.cpp:4095 +} + } call->setCallingConv(getRuntimeCC()); eugenis wrote: > guiand wrote: > > jdoerfert wrote: > > > Why would we do this? Function attributes are valid at the call site, no

[PATCH] D82470: [OpenMP][IRBuilder] Support nested parallel regions

jdoerfert created this revision. jdoerfert added reviewers: anchu-rajendran, kiranktp, fghanim. Herald added subscribers: llvm-commits, cfe-commits, sstefan1, guansong, bollu, hiraditya, yaxunl. Herald added projects: clang, LLVM. During code generation we might change/add basic blocks so keeping

[PATCH] D82324: [OPENMP]Dynamic globalization for parallel target regions.

jdoerfert added a comment. I do not understand why we need the flag. As far as I can tell, it has to be on to support the standard described behavior, right? Why should we (allow to) turn it off? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D82324

[PATCH] D79677: [Clang][OpenMP][OMPBuilder] (1/4) Privatize `parallel` for `OMPBuilder`

jdoerfert added a reviewer: kiranchandramohan. jdoerfert added a comment. @kiranchandramohan You can add yourself and others ;) Can we test this? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D79677/new/ https://reviews.llvm.org/D79677 _

[PATCH] D82324: [OPENMP]Dynamic globalization for parallel target regions.

jdoerfert added a comment. Let me rephrase. Does the user needs to request the fast path or the user needs to request the slow but correct path? Only the former is acceptable IMHO. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D82324/new/ https://r

[PATCH] D82324: [OPENMP]Dynamic globalization for parallel target regions.

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

[PATCH] D80897: [OpenMP] Initial support for std::complex in target regions

jdoerfert added a comment. @tra After chatting with @hfinkel I know now why we don't see the calls in the libc++ case. libc++ implements std::complex without `_Complex` types, stdlib++ does. If the user uses `_Complex` directly we need these functions for sure as the standard defines them (I t

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