[PATCH] D74941: [OpenMP] `omp begin/end declare variant` - part 1, parsing

jdoerfert marked 2 inline comments as done. jdoerfert added inline comments. Comment at: clang/lib/Parse/ParseOpenMP.cpp:1357-1359 + // Skip last tokens. + while (Tok.isNot(tok::annot_pragma_openmp_end)) +ConsumeAnyToken(); ABataev wrote: > Better to emit

[PATCH] D74941: [OpenMP] `omp begin/end declare variant` - part 1, parsing

jdoerfert added a comment. In D74941#1886403 , @kiranchandramohan wrote: > Will tests come in a later patch? I have tests, seems I forgot to add them. Will fix later today with the other things. Repository: rG LLVM Github Monorepo CHANGES SINCE

[PATCH] D74787: [IRBuilder] Always respect inserter/folder

jdoerfert added a comment. > There should be at most two iterations, one to perform folds and one to > verify the fixpoint. If there are more iterations, that's a bug in worklist > management FWIW, the Attributor uses basically the same scheme. It helps us to monitor iteration

[PATCH] D74941: [OpenMP] `omp begin/end declare variant` - part 1, parsing

jdoerfert updated this revision to Diff 246035. jdoerfert added a comment. Revert test check lines to the parser part only Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D74941/new/ https://reviews.llvm.org/D74941 Files:

[PATCH] D75001: [OpenMP][cmake] ignore warning on unknown CUDA version

jdoerfert accepted this revision. jdoerfert added a comment. This revision is now accepted and ready to land. I like this way better. I was hoping we could do it in our cmake only :) Give others a day or so to comment before your commit but I'm fine with this. CHANGES SINCE LAST ACTION

[PATCH] D74941: [OpenMP] `omp begin/end declare variant` - part 1, parsing

jdoerfert updated this revision to Diff 246032. jdoerfert marked an inline comment as done. jdoerfert added a comment. Herald added a subscriber: jfb. Add tests and address comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D74941/new/

[PATCH] D74941: [OpenMP] `omp begin/end declare variant` - part 1, parsing

jdoerfert marked an inline comment as done. jdoerfert added inline comments. Comment at: clang/test/OpenMP/begin-declare-variant_elided_range_withouth_end.c:5 +// TODO: Improve the error message +#pragma omp begin declare variant match(device={kind(cpu)}) // expected-error

[PATCH] D74935: [LangRef][AliasAnalysis] Clarify `noalias` affects only modified objects

jdoerfert added a comment. In D74935#1887960 , @nhaehnle wrote: > In D74935#1887245 , @efriedma wrote: > > > In D74935#1886020 , @nhaehnle > > wrote: > > > > > I find this

[PATCH] D74935: [LangRef][AliasAnalysis] Clarify `noalias` affects only modified objects

jdoerfert updated this revision to Diff 246088. jdoerfert added a comment. Update language to memory locations instead of objects Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D74935/new/ https://reviews.llvm.org/D74935 Files:

[PATCH] D74941: [OpenMP] `omp begin/end declare variant` - part 1, parsing

jdoerfert updated this revision to Diff 246094. jdoerfert added a comment. minor adjustment Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D74941/new/ https://reviews.llvm.org/D74941 Files: clang/include/clang/AST/OpenMPClause.h

[PATCH] D71830: [OpenMP][Part 2] Use reusable OpenMP context/traits handling

jdoerfert marked 14 inline comments as done. jdoerfert added inline comments. Comment at: clang/include/clang/Basic/Attr.td:185 +// An argument of type \p type with name \p name. +class GenericPointerArgument : Argument { + string Type = type; aaron.ballman

[PATCH] D71830: [OpenMP][Part 2] Use reusable OpenMP context/traits handling

jdoerfert added inline comments. Comment at: clang/lib/AST/OpenMPClause.cpp:1723 +llvm::APInt CondVal = +Selector.ScoreOrCondition->EvaluateKnownConstInt(ASTCtx); +if (CondVal.isNullValue()) aaron.ballman wrote: > jdoerfert wrote: > >

[PATCH] D71830: [OpenMP][Part 2] Use reusable OpenMP context/traits handling

jdoerfert marked 11 inline comments as done. jdoerfert added inline comments. Comment at: clang/utils/TableGen/ClangAttrEmitter.cpp:367 } else { -llvm_unreachable("Unknown SimpleArgument type!"); +OS << "if (SA->get" << getUpperName() << "())\n "; +

[PATCH] D74513: [OpenMP][NFCI] Use the libFrontend DefaultKind in Clang

This revision was automatically updated to reflect the committed changes. Closed by commit rG577c9b02ab57: [OpenMP][NFCI] Use the libFrontend DefaultKind in Clang (authored by atmnpatel, committed by jdoerfert). Changed prior to commit: https://reviews.llvm.org/D74513?vs=244334=244815#toc

[PATCH] D74562: [OpenMP][OMPIRBuilder] Introducing the `OMPBuilderCBHelpers` helper class

jdoerfert added a comment. This is great, thanks a lot! I only have two comments where I am not sure I understand the code/change. Comment at: clang/lib/CodeGen/CGStmtOpenMP.cpp:3134 // TODO: Replace with a generic helper function for emitting body auto BodyGenCB

[PATCH] D74571: [OpenMP][CUDA] Add CUDA 10.2 support

jdoerfert added a comment. In D74571#1877000 , @kkwli0 wrote: > > I am not sure I understand. Do we need to modify stuff outside of > > `/openmp`? I was hoping it is our CMake that can be adjusted to make this > > work as described earlier. TBH, he

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

This revision was automatically updated to reflect the committed changes. Closed by commit rG7438059a9032: [OpenMP][OMPIRBuilder] Add Directives (master and critical) to OMPBuilder. (authored by fghanim, committed by jdoerfert). Changed prior to commit:

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

jdoerfert added a comment. I did run clang-format on the OMPIRBuilder files and replaced tabs with spaces. I think you need to teach your editor to use spaces in the future. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D72304/new/

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

jdoerfert added a comment. In D72304#1877676 , @MaskRay wrote: > `test/CodeGen/opt-record-1.c` triggers an assertion failure. I hope the macro reordering in rGa0236de7a927 did the

[PATCH] D74372: [OpenMP][IRBuilder] Perform finalization (incl. outlining) late

This revision was automatically updated to reflect the committed changes. Closed by commit rG8a56d64d7620: [OpenMP][IRBuilder] Perform finalization (incl. outlining) late (authored by jdoerfert). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D74513: [OpenMP][NFCI] Use the libFrontend DefaultKind in Clang

jdoerfert added a comment. One question (below) otherwise this looks good. Comment at: llvm/include/llvm/Frontend/OpenMP/OMPKinds.def:358 +__OMP_DEFAULT_KIND(shared, 3) +__OMP_DEFAULT_KIND(unknown, 4) + Why 2, 3, 4? Do we really need to specify the value here?

[PATCH] D74372: [OpenMP][IRBuilder] Perform finalization (incl. outlining) late

jdoerfert added a comment. In D74372#1873623 , @plotfi wrote: > Could this be reverted for the time being so that bots can go green? There > appear to be an awful lot of commits piling up on top of this one. Should be fixed by now but you should revert

[PATCH] D74513: [OpenMP][NFCI] Use the libFrontend DefaultKind in Clang

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

[PATCH] D74372: [OpenMP][IRBuilder] Perform finalization (incl. outlining) late

jdoerfert updated this revision to Diff 244005. jdoerfert added a comment. Improve comment Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D74372/new/ https://reviews.llvm.org/D74372 Files: clang/lib/CodeGen/CodeGenFunction.cpp

[PATCH] D74372: [OpenMP][IRBuilder] Perform finalization (incl. outlining) late

jdoerfert updated this revision to Diff 244002. jdoerfert marked 6 inline comments as done. jdoerfert added a comment. Removed OMPIRBuilder workaround in extractor and addressed comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D74372/new/

[PATCH] D74372: [OpenMP][IRBuilder] Perform finalization (incl. outlining) late

jdoerfert added a comment. In D74372#1870620 , @vsk wrote: > Thanks :). IIRC `check-clang` is enough to exercise the relevant code path, > as we get an assert in CodeExtractor without the workaround. (side note: > please don't take my comments here as

[PATCH] D71830: [OpenMP][Part 2] Use reusable OpenMP context/traits handling

jdoerfert added a comment. @kiranchandramohan Do you have more input on this one? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71830/new/ https://reviews.llvm.org/D71830 ___ cfe-commits mailing list

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

jdoerfert added a comment. In D72304#1877855 , @fhahn wrote: > It looks like this patch (or the other related changes committed recently) > may cause ASan failures running `Transforms/OpenMP/gtid.ll ` and > `Transforms/OpenMP/parallel_deletion.ll` >

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

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

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

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

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

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

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

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

[PATCH] D72901: [OpenMP] [DOCS] Update OMP5.0 feature status table [NFC]

jdoerfert added inline comments. Comment at: clang/docs/OpenMPSupport.rst:194 +--+--+--+---+ -|

[PATCH] D72996: [Sema] Attempt to perform call-size-specific `__attribute__((alloc_align(param_idx)))` validation

jdoerfert added inline comments. Comment at: clang/lib/Sema/SemaChecking.cpp:4489 +// Alignment calculations can wrap around if it's greater than 2**29. +unsigned MaximumAlignment = 536870912; +if (I > MaximumAlignment) hfinkel wrote: >

[PATCH] D72996: [Sema] Attempt to perform call-size-specific `__attribute__((alloc_align(param_idx)))` validation

jdoerfert added inline comments. Comment at: clang/lib/Sema/SemaChecking.cpp:4489 +// Alignment calculations can wrap around if it's greater than 2**29. +unsigned MaximumAlignment = 536870912; +if (I > MaximumAlignment) hfinkel wrote: >

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

jdoerfert added a comment. Phab is not involved in the merge process, it just is a normal git push on top of the llmv-project. Do you have commit rights already? If not I can commit this for you in a few days. Comment at: llvm/lib/Frontend/OpenMP/OMPConstants.cpp:45 ///{ -

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

jdoerfert added a comment. The commit title has a stray `{` instead of `[` Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D72304/new/ https://reviews.llvm.org/D72304 ___ cfe-commits mailing list

[PATCH] D73128: [OPENMP][NVPTX]Add NVPTX specific definitions for new/delete operators.

jdoerfert accepted this revision. jdoerfert added a comment. This revision is now accepted and ready to land. Some minor comments but other than that, LGTM. Nit: Clang format the file Comment at: clang/lib/Headers/openmp_wrappers/new:8 +

[PATCH] D73128: [OPENMP][NVPTX]Add NVPTX specific definitions for new/delete operators.

jdoerfert added a comment. In D73128#1832033 , @ABataev wrote: > A note: this header file requires to include stdlib.h too to see the > declarations of malloc/free. Shall We include it here to fix it? otherwise > the error is reported that malloc and

[PATCH] D72996: [Sema] Attempt to perform call-size-specific `__attribute__((alloc_align(param_idx)))` validation

jdoerfert added inline comments. Comment at: clang/lib/Sema/SemaChecking.cpp:4489 +// Alignment calculations can wrap around if it's greater than 2**29. +unsigned MaximumAlignment = 536870912; +if (I > MaximumAlignment) erichkeane wrote:

[PATCH] D73005: [Codegen] If reasonable, materialize clang's `AssumeAlignedAttr` as llvm's Alignment Attribute on call-site function return value

jdoerfert added inline comments. Comment at: clang/test/CodeGen/assume-aligned-and-alloc-align-attributes.c:12 // CHECK-NEXT:[[MASKCOND:%.*]] = icmp eq i64 [[MASKEDPTR]], 0 // CHECK-NEXT:call void @llvm.assume(i1 [[MASKCOND]]) // CHECK-NEXT:ret i8* [[CALL]]

[PATCH] D72996: [Sema] Attempt to perform call-size-specific `__attribute__((alloc_align(param_idx)))` validation

jdoerfert added inline comments. Comment at: clang/lib/Sema/SemaChecking.cpp:4489 +// Alignment calculations can wrap around if it's greater than 2**29. +unsigned MaximumAlignment = 536870912; +if (I > MaximumAlignment) erichkeane wrote:

[PATCH] D72901: [OpenMP] [DOCS] Update OMP5.0 feature status table [NFC]

jdoerfert added a comment. @kkwli0 I propose you can merge the parts where discussion has reached a consensus while the other parts are resolved. I'm generally fine with this, we can always improve on it further. Comment at: clang/docs/OpenMPSupport.rst:194

[PATCH] D71830: [OpenMP] Reusable OpenMP context/traits handling

jdoerfert added a comment. In D71830#1794920 , @JonChesterfield wrote: > Big patch but looks like a net decrease in complexity. Please could you clang > format the areas phabricator is complaining about? I'll go over it. (FWIW, I do clang format my

[PATCH] D71884: [OpenMP] Fix formatting of OpenMP error message.

jdoerfert accepted this revision. jdoerfert added a comment. This revision is now accepted and ready to land. I would have just stripped the last chars of ` Out.str()` but if this passes all the tests I'm fine with it. LGTM. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D70289: [OpenMP][NFCI] Use the libFrontend ProcBindKind in Clang

This revision was automatically updated to reflect the committed changes. Closed by commit rG6c5d1f40ff8d: [OpenMP][NFCI] Use the libFrontend ProcBindKind in Clang (authored by jdoerfert). Herald added a subscriber: hiraditya. Herald added a project: LLVM. Changed prior to commit:

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

jdoerfert updated this revision to Diff 235390. jdoerfert added a comment. Herald added subscribers: llvm-commits, hiraditya. Herald added a project: LLVM. Rebase + add a test case Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D70290/new/

[PATCH] D70973: [OPENMP50]Treat context selectors as expressions, not just strings.

jdoerfert added a comment. In D70973#1793061 , @jdoerfert wrote: > I haven't forgotten about this! With the other two changes to the declare > variant going to happen now,as well, I figured we should look at this again > from a high-level. > > My plan

[PATCH] D70258: [OpenMP][IR-Builder] Introduce the finalization stack

This revision was automatically updated to reflect the committed changes. Closed by commit rGf9c3c5da19ab: [OpenMP][IR-Builder] Introduce the finalization stack (authored by jdoerfert). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D70258/new/

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

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

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

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

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

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

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

This revision was automatically updated to reflect the committed changes. jdoerfert marked an inline comment as done. Closed by commit rG000c6a5038bc: [OpenMP] Use the OpenMPIRBuilder for `omp cancel` (authored by jdoerfert). Changed prior to commit:

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

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

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

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

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

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

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

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

[PATCH] D75209: [OPENMP][NVPTX]Fix PR45003: add support for complex functions.

jdoerfert added a comment. I am unsure we need this with the proper math support. Sema patch for that is going for review today. I'll try this out soon. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75209/new/ https://reviews.llvm.org/D75209

[PATCH] D74935: [LangRef][AliasAnalysis] Clarify `noalias` affects only modified objects

jdoerfert added a comment. In D74935#1907100 , @jeroen.dobbelaere wrote: > Just to give an example: > > int foo(int* restrict *pA, int* restrict *pB) { > int tmp=**pB; > **pA=42; > return tmp - **pB; // **pA and **pB can refer to the same

[PATCH] D75210: [Attr] Allow ParsedAttr to be constructor for any Attribute

jdoerfert updated this revision to Diff 248845. jdoerfert added a comment. Pass parsed attribute kind explicitly Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75210/new/ https://reviews.llvm.org/D75210 Files:

[PATCH] D75788: [WIP][OpenMP] Reuse CUDA wrappers in `nvptx` target regions.

jdoerfert created this revision. jdoerfert added reviewers: kiranchandramohan, ABataev, RaviNarayanaswamy, gtbercea, grokos, sdmitriev, JonChesterfield, hfinkel, fghanim, aaron.ballman. Herald added subscribers: guansong, bollu, mgorny. Herald added a project: clang. This is a WIP patch to show

[PATCH] D75788: [WIP][OpenMP] Reuse CUDA wrappers in `nvptx` target regions.

jdoerfert marked an inline comment as done. jdoerfert added a comment. In D75788#1910743 , @JonChesterfield wrote: > That's less invasive than I feared. Nicely done. We need to run some more tests to make sure it works as expected but I hope we can

[PATCH] D75779: [OpenMP] `omp begin/end declare variant` - part 2, semantic analysis

jdoerfert planned changes to this revision. jdoerfert added a comment. I'll try to remove the extra namespace and I'll split the `function(is_definition)` matcher out. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75779/new/

[PATCH] D75827: Add new `attribute push` matcher `function(is_definition)`

jdoerfert created this revision. jdoerfert added reviewers: erichkeane, aaron.ballman. Herald added a subscriber: bollu. Herald added a project: clang. This can also be used as a subject for attributes. TODO: Tests are missing. Repository: rG LLVM Github Monorepo

[PATCH] D75779: [OpenMP] `omp begin/end declare variant` - part 2, semantic analysis

jdoerfert added a comment. There will be an update later today or tomorrow, I am making the actual target offloading CUDA interoperability "work" right now. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75779/new/ https://reviews.llvm.org/D75779

[PATCH] D75788: [WIP][OpenMP] Reuse CUDA wrappers in `nvptx` target regions.

jdoerfert updated this revision to Diff 248937. jdoerfert added a comment. Add complex test case Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75788/new/ https://reviews.llvm.org/D75788 Files: clang/lib/Driver/ToolChains/Clang.cpp

[PATCH] D75209: [OPENMP][NVPTX]Fix PR45003: add support for complex functions.

jdoerfert added a comment. In D75209#1910047 , @ABataev wrote: > In D75209#1909976 , @jdoerfert wrote: > > > I am unsure we need this with the proper math support. Sema patch for that > > is going for review

[PATCH] D75788: [WIP][OpenMP] Reuse CUDA wrappers in `nvptx` target regions.

jdoerfert updated this revision to Diff 250035. jdoerfert marked an inline comment as done. jdoerfert added a comment. Adjust to new scheme, tested locally with some math functions, seems to work Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D75210: [Attr] Allow ParsedAttr to be constructor for any Attribute

jdoerfert planned changes to this revision. jdoerfert added a comment. The reason for this patch is not there anymore. I'm fine with postponing this patch until there is a user again, thoughts? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D75788: [WIP][OpenMP] Reuse CUDA wrappers in `nvptx` target regions.

jdoerfert marked 3 inline comments as done. jdoerfert added inline comments. Comment at: clang/lib/AST/ASTContext.cpp:1621 case BuiltinType::LongDouble: -if (getLangOpts().OpenMP && getLangOpts().OpenMPIsDevice) return AuxTarget->getLongDoubleFormat();

[PATCH] D71830: [OpenMP][Part 2] Use reusable OpenMP context/traits handling

jdoerfert marked an inline comment as done. jdoerfert added inline comments. Comment at: clang/include/clang/AST/OpenMPClause.h:6682 + /// The outermost level of selector sets. + llvm::SmallVector Sets; + jdoerfert wrote: > rnk wrote: > > This is not a good

[PATCH] D76184: [OpenMP][NFC] Remove the need to include `OpenMPClause.h`

jdoerfert updated this revision to Diff 250396. jdoerfert added a comment. Finish changes Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D76184/new/ https://reviews.llvm.org/D76184 Files: clang/include/clang/AST/Attr.h

[PATCH] D75591: [OpenMP] Add firstprivate as a default data-sharing attribute to clang

jdoerfert added a comment. I think this is functionally correct even if we pass the pointer for now as it is properly "privatized" in the callee. I will go over this once more (after some sleep) but I think it is all good. Once @fghanim puts the new code path on phab we need to ensure it works

[PATCH] D76173: [OpenMP][NFC] Minimize memory usage and copying of `OMPTraitInfo`s

jdoerfert added a comment. In D76173#1922916 , @rnk wrote: > lgtm, thanks! > > I also noticed that Parser.h includes OpenMPClause.h just for this class. > OpenMPClause.h is pretty big. It's for this family of related methods: > > /// Parse a property

[PATCH] D75210: [Attr] Allow ParsedAttr to be constructor for any Attribute

jdoerfert abandoned this revision. jdoerfert added a comment. Code is good and usable but no users anymore. If anyone wants to pick this up, feel free! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75210/new/ https://reviews.llvm.org/D75210

[PATCH] D76184: [OpenMP][NFC] Remove the need to include `OpenMPClause.h`

jdoerfert created this revision. jdoerfert added a reviewer: rnk. Herald added subscribers: guansong, bollu. Herald added a project: clang. See rational here: https://reviews.llvm.org/D76173#1922916 Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D76184 Files:

[PATCH] D75010: [OpenMP] Adding InaccessibleMemOnly and InaccessibleMemOrArgMemOnly for runtime calls.

jdoerfert added a comment. I always thought there was a test for this, turns out I never committed it. Since I did now (1c9c23d60ea7656174ad3d76293c2a90dd25e24f ) you need to rebase and adjust it, sorry.

[PATCH] D74935: [LangRef][AliasAnalysis] Clarify `noalias` affects only modified objects

jdoerfert added a comment. In D74935#1915311 , @jeroen.dobbelaere wrote: > In D74935#1909909 , @jdoerfert wrote: > > > I would say that once we get modeling for indirect restrict we can adapt > > the lang ref

[PATCH] D75827: Add new `attribute push` matcher `function(is_definition)`

jdoerfert abandoned this revision. jdoerfert added a comment. Not needed anymore. Anyone should feel free to pick this up. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75827/new/ https://reviews.llvm.org/D75827

[PATCH] D75591: [OpenMP] Add firstprivate as a default data-sharing attribute to clang

jdoerfert added a subscriber: fghanim. jdoerfert added a comment. In D75591#1916433 , @atmnpatel wrote: > Modified the unit test for CodeGen of default(firstprivate) to more > accurately reflect the IR. > > From the perspective of the AST, the variables

[PATCH] D77683: [Docs] Make code review policy clearer about requested pre-commit reviews

jdoerfert added a comment. I did like @lattner 's solution so I use that now. Wdyt? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D77683/new/ https://reviews.llvm.org/D77683 ___ cfe-commits mailing

[PATCH] D77683: [Docs] Make code review policy clearer about requested pre-commit reviews

jdoerfert updated this revision to Diff 256108. jdoerfert added a comment. Use @lattern 's wording which is more specific and has less spelling errors. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D77683/new/ https://reviews.llvm.org/D77683

[PATCH] D77774: [OpenMP] Allow to go first in C++-mode in target regions

This revision was automatically updated to reflect the committed changes. Closed by commit rG17d83342235f: [OpenMP] Allow math.h to go first in C++-mode in target regions (authored by jdoerfert). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D70366: Add new 'flatten' LLVM attribute to fix clang's 'flatten' function attribute

jdoerfert added a subscriber: probinson. jdoerfert added a comment. Found the commit: dcbe35bad518 The way I see it we just have to teach the inliner about `optnone` so we can uncouple the two (`optnone` and `noinline`). @probinson WDTY? CHANGES SINCE LAST ACTION

[PATCH] D70366: Add new 'flatten' LLVM attribute to fix clang's 'flatten' function attribute

jdoerfert added a comment. In D70366#1972880 , @LevitatingLion wrote: > In D70366#1971137 , @dexonsmith > wrote: > > > In D70366#1970758 , @jdoerfert > > wrote: > > > > >

[PATCH] D75591: [OpenMP] Add firstprivate as a default data-sharing attribute to clang

jdoerfert accepted this revision. jdoerfert added a comment. Apologies for my delay. LGTM. Wait for @lebedev.ri though. Comment at: clang-tools-extra/docs/clang-tidy/checks/openmp-use-default-none.rst:56 + // ``parallel`` directive can have ``default`` clause, and said

[PATCH] D70366: Add new 'flatten' LLVM attribute to fix clang's 'flatten' function attribute

jdoerfert added a comment. In D70366#1970299 , @LevitatingLion wrote: > While adding tests to clang I realized the attribute is not working as > intended when using an optimization level of zero, because clang adds the > noinline attribute to all

[PATCH] D77731: [OPENMP]Fix capturing of global variables in OpenMP regions.

jdoerfert added a subscriber: fghanim. jdoerfert added inline comments. Comment at: clang/lib/Sema/SemaOpenMP.cpp:2117 +if (VD && !VD->hasLocalStorage() && DVarPrivate.CKind == OMPC_unknown && +(DSAStack->getDefaultDSA() != DSA_none || DVarTop.CKind == OMPC_shared))

[PATCH] D77731: [OPENMP]Fix capturing of global variables in OpenMP regions.

jdoerfert accepted this revision. jdoerfert added a subscriber: atmnpatel. jdoerfert added a comment. This revision is now accepted and ready to land. LGTM. Comment at: clang/lib/Sema/SemaOpenMP.cpp:2117 +if (VD && !VD->hasLocalStorage() && DVarPrivate.CKind ==

[PATCH] D70366: Add new 'flatten' LLVM attribute to fix clang's 'flatten' function attribute

jdoerfert added a comment. In D70366#1970526 , @dexonsmith wrote: > In D70366#1970299 , @LevitatingLion > wrote: > > > Maybe we can add an additional string attribute when adding the noinline > > attribute to

[PATCH] D74387: [SYCL] Defer __float128 type usage diagnostics

jdoerfert added a comment. In D74387#1969891 , @Fznamznon wrote: > In D74387#1967386 , @jdoerfert wrote: > > > In D74387#1967289 , @Fznamznon > > wrote: > > > > > In

[PATCH] D77751: [OPENMP50] Fix PR45469: Consider variable-category of defaultmap clause as optional.

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

[PATCH] D77683: [Docs] Make code review policy clearer about requested pre-commit reviews

jdoerfert added a comment. In D77683#1970826 , @mehdi_amini wrote: > I am still not sure what "if someone has asked for extra review of a specific > area" refers to? As said earlier >> If I understand this correctly, this is meant to cover

[PATCH] D77774: [OpenMP] Allow to go first in C++-mode in target regions

jdoerfert created this revision. jdoerfert added a reviewer: hfinkel. Herald added subscribers: guansong, bollu, yaxunl. Herald added a project: clang. If we are in C++ mode and include (not ) first, we still need to make sure is read first. The problem otherwise is that we haven't seen the

[PATCH] D77633: [Parser] Improve diagnostic and error recovery when C++ keywords are used as identifiers.

jdoerfert added a comment. This is cool, thx! One case I stumbled upon recently is: `void explicit() {}` Which results in these rather confusing messages: /tmp/test.cpp:1:14: warning: explicit(bool) is a C++20 extension [-Wc++20-extensions] void explicit() { } ^

[PATCH] D77984: Make IRBuilder automatically set alignment on load/store/alloca.

jdoerfert added a comment. I think this is fine. Two comments to consider below. Comment at: llvm/include/llvm/IR/IRBuilder.h:1600 +return CreateAlignedLoad(Ty, Ptr, DL.getABITypeAlign(Ty), isVolatile, Name); } Can't we just pawn of the alignment

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

jdoerfert created this revision. jdoerfert added reviewers: tra, hfinkel. Herald added subscribers: guansong, bollu, yaxunl. Herald added a project: clang. Due to recent changes we cannot use OpenMP in CUDA files anymore (PR45533) as the math handling of CUDA is different when _OPENMP is defined.

[PATCH] D77918: [OpenMP] Avoid crash in preparation for diagnose of unsupported type

jdoerfert marked an inline comment as done. jdoerfert added inline comments. Comment at: clang/lib/Sema/SemaOpenMP.cpp:1831 "OpenMP device compilation mode is expected."); + // TODO: Do not check outside of functions for now as the targetDiag below + //

[PATCH] D77683: [Docs] Make code review policy clearer about requested pre-commit reviews

jdoerfert added a comment. The problem I want to address is with code owners. TBH, I would like our overall description of that role to be (more) like @hfinkel described it. In case there is no controversy, that is not really different from @dblaikie vision (I think). If there is controversy,

[PATCH] D78190: Add Bfloat IR type

jdoerfert added inline comments. Comment at: llvm/include/llvm/IR/Constants.h:790 + static Constant *getFP(Type *ElementType, ArrayRef Elts); /// Return a ConstantVector with the specified constant in each element. Drive by: The documentation of these

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