[PATCH] D61643: [PragmaHandler][NFC] Expose `#pragma` location

jdenny marked an inline comment as done. jdenny added inline comments. Comment at: clang/docs/ClangPlugins.rst:58 ExamplePragmaHandler() : PragmaHandler("example_pragma") { } -void HandlePragma(Preprocessor &PP, PragmaIntroducerKind Introducer, +void HandlePragma(Pre

[PATCH] D61643: [PragmaHandler][NFC] Expose `#pragma` location

jdenny marked an inline comment as done. jdenny added inline comments. Comment at: clang/docs/ClangPlugins.rst:58 ExamplePragmaHandler() : PragmaHandler("example_pragma") { } -void HandlePragma(Preprocessor &PP, PragmaIntroducerKind Introducer, +void HandlePragma(Pre

[PATCH] D61643: [PragmaHandler] Expose `#pragma` location

This revision was automatically updated to reflect the committed changes. Closed by commit rC361335: [PragmaHandler] Expose `#pragma` location (authored by jdenny, committed by ). Changed prior to commit: https://reviews.llvm.org/D61643?vs=198489&id=200608#toc Repository: rC Clang CHANGES S

[PATCH] D61643: [PragmaHandler] Expose `#pragma` location

jdenny added a comment. In D61643#1510815 , @Meinersbur wrote: > +1 > > Such a solution also came up in https://bugs.llvm.org/show_bug.cgi?id=41514#c1 Cool. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61

[PATCH] D61509: [OpenMP] Set pragma start loc to `#pragma` loc

jdenny added a subscriber: Meinersbur. jdenny added a comment. Now that D61643 is pushed, we're back to this patch. My recollection is there are two remaining issues: 1. Should we store both the `#pragma` location and the `omp` location in the AST, or is it fi

[PATCH] D61509: [OpenMP] Set pragma start loc to `#pragma` loc

jdenny added a comment. In D61509#1512294 , @ABataev wrote: > In D61509#1512293 , @Meinersbur > wrote: > > > 1. Is there a diagnostic that would point to the `omp` token? As much as I > > like complete info (such

[PATCH] D61509: [OpenMP] Set pragma start loc to `#pragma` loc

jdenny added a comment. In D61509#1512321 , @Meinersbur wrote: > In D61509#1512311 , @jdenny wrote: > > > 2. I too think it likely makes sense to adjust them all eventually. But do > > people think it's important

[PATCH] D61509: [OpenMP] Set pragma start loc to `#pragma` loc

jdenny added a comment. In D61509#1512488 , @aaron.ballman wrote: > In D61509#1512090 , @jdenny wrote: > > > Now that D61643 is pushed, we're back to > > this patch. My recollect

[PATCH] D61509: [OpenMP] Set pragma start loc to `#pragma` loc

jdenny added a comment. In D61509#1512570 , @aaron.ballman wrote: > These changes LGTM! Thanks, and thanks to everyone who chimed in. > I'd say give it a few days in case other reviewers would like to chime in. Sure. Maybe this weekend or so. CHANG

[PATCH] D61509: [OpenMP] Set pragma start loc to `#pragma` loc

jdenny added a comment. In D61509#1512601 , @Meinersbur wrote: > > I would think different people would want to review different pragmas, so > > separate patches would be better, but I'm happy to be corrected as I > > haven't explored who owns what here.

[PATCH] D61467: [Rewrite] Extend to further accept CharSourceRange

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

[PATCH] D61509: [OpenMP] Set pragma start loc to `#pragma` loc

This revision was automatically updated to reflect the committed changes. Closed by commit rL361867: [OpenMP] Set pragma start loc to `#pragma` loc (authored by jdenny, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Changed prior to commit: https://review

[PATCH] D55861: [OpenMP] Fix data sharing analysis in nested clause

jdenny created this revision. jdenny added a reviewer: ABataev. Herald added a subscriber: guansong. Without this patch, clang doesn't complain that X needs explicit data sharing attributes in the following: #pragma omp target teams default(none) { #pragma omp parallel num_threads(X)

[PATCH] D55861: [OpenMP] Fix data sharing analysis in nested clause

This revision was automatically updated to reflect the committed changes. Closed by commit rC349635: [OpenMP] Fix data sharing analysis in nested clause (authored by jdenny, committed by ). Changed prior to commit: https://reviews.llvm.org/D55861?vs=178803&id=178890#toc Repository: rC Clang

[PATCH] D56113: [OpenMP] Replace predetermined shared for const variable

jdenny added a comment. In D56113#1341940 , @ABataev wrote: > The patch does not seem quite correct. I committed another fix for this > problem. Thanks for the fix, r350127, which does seem to address the use cases I care about right now. However, you

[PATCH] D56113: [OpenMP] Replace predetermined shared for const variable

jdenny added a comment. Hi Alexey, > In D56113#1342076 , @jdenny wrote: > >> In D56113#1341940 , @ABataev wrote: >> >> > The patch does not seem quite correct. I committed another fix for this >> > problem. >> >>

[PATCH] D56113: [OpenMP] Replace predetermined shared for const variable

jdenny marked 2 inline comments as done. jdenny added a comment. In D56113#1342879 , @ABataev wrote: > But you will need another serie of patches for `reduction` and `linear` > clauses to update their error messages. Those already had their own checks f

[PATCH] D56113: [OpenMP] Replace predetermined shared for const variable

jdenny added a comment. In D56113#1345047 , @ABataev wrote: > In D56113#1344210 , @jdenny wrote: > > > In D56113#1342879 , @ABataev wrote: > > > > > But you will need another

[PATCH] D56113: [OpenMP] Replace predetermined shared for const variable

jdenny added a comment. In D56113#1345238 , @ABataev wrote: > >>> By the way, is there any value to keeping the predetermined shared for > >>> const if -openmp-version=3.1 or earlier? > >> > >> Yes, you can check for the value of `LangOpts.OpenMP`. For O

[PATCH] D56113: [OpenMP] Replace predetermined shared for const variable

jdenny added a comment. In D56113#1345337 , @ABataev wrote: > > In D56113#1345238 , @ABataev wrote: > > > >> In D56113#1345232 , @jdenny wrote: > >> > >> > In D56113#1345047

[PATCH] D56113: [OpenMP] Replace predetermined shared for const variable

jdenny added a comment. In D56113#1345238 , @ABataev wrote: > >>> By the way, is there any value to keeping the predetermined shared for > >>> const if -openmp-version=3.1 or earlier? > >> > >> Yes, you can check for the value of `LangOpts.OpenMP`. For O

[PATCH] D56113: [OpenMP] Replace predetermined shared for const variable

jdenny added a comment. In D56113#1345535 , @ABataev wrote: > In D56113#1345529 , @jdenny wrote: > > > In D56113#1345238 , @ABataev wrote: > > > > > >>> By the way, is there

[PATCH] D56113: [OpenMP] Replace predetermined shared for const variable

jdenny added a comment. In D56113#1345578 , @ABataev wrote: > >>> By the way, LangOpts.OpenMP currently defaults to 0. Should it? > >> > >> Yes, it means it is disabled by default. > > > > Ah, it becomes 1 when we have `-fopenmp` but not `-fopenmp-versi

[PATCH] D56113: [OpenMP] Replace predetermined shared for const variable

jdenny added a comment. In D56113#1345529 , @jdenny wrote: > In D56113#1345238 , @ABataev wrote: > > > >>> By the way, is there any value to keeping the predetermined shared for > > >>> const if -openmp-version=3.1

[PATCH] D56299: [OpenMP] Refactor const restriction for linear

jdenny created this revision. jdenny added a reviewer: ABataev. Herald added a subscriber: guansong. As discussed in D56113 , this patch refactors the implementation of the const restriction for linear to reuse a function introduced by D56113

[PATCH] D56113: [OpenMP] Replace predetermined shared for const variable

jdenny marked an inline comment as done. jdenny added inline comments. Comment at: clang/lib/Sema/SemaOpenMP.cpp:9736 +static bool rejectConstNotMutableType(Sema &SemaRef, ValueDecl *D, + OpenMPClauseKind CKind, ABataev wrot

[PATCH] D56299: [OpenMP] Refactor const restriction for linear

jdenny updated this revision to Diff 180283. jdenny added a comment. Update for changes to earlier patches in series. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56299/new/ https://reviews.llvm.org/D56299 Files: clang/lib/Sema/SemaOpenMP.cpp clang/test/OpenMP/distribute_parallel_

[PATCH] D56299: [OpenMP] Refactor const restriction for linear

jdenny updated this revision to Diff 180301. jdenny added a comment. Update for changes to earlier patches in series. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56299/new/ https://reviews.llvm.org/D56299 Files: clang/lib/Sema/SemaOpenMP.cpp clang/test/Ope

[PATCH] D56113: [OpenMP] Replace predetermined shared for const variable

jdenny added a comment. Thanks for the review! Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56113/new/ https://reviews.llvm.org/D56113 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cg

[PATCH] D56299: [OpenMP] Refactor const restriction for linear

This revision was automatically updated to reflect the committed changes. Closed by commit rC350441: [OpenMP] Refactor const restriction for linear (authored by jdenny, committed by ). Changed prior to commit: https://reviews.llvm.org/D56299?vs=180301&id=180320#toc Repository: rC Clang CHAN

[PATCH] D45093: [AST] Fix -ast-print for _Bool when have diagnostics

jdenny updated this revision to Diff 142662. jdenny added a comment. Rebased onto a more recent master. https://reviews.llvm.org/D45093 Files: include/clang/AST/ASTContext.h include/clang/Sema/Sema.h lib/Parse/ParseDecl.cpp lib/Sema/Sema.cpp lib/Sema/SemaChecking.cpp lib/Sema/SemaCo

[PATCH] D45456: [Attr] Print enum attributes at correct position

jdenny updated this revision to Diff 142672. jdenny edited the summary of this revision. jdenny added a comment. Rebased onto a more recent master. Added example to summary. https://reviews.llvm.org/D45456 Files: lib/AST/DeclPrinter.cpp test/Sema/ast-print.c Index: test/Sema/ast-print.c

[PATCH] D45463: [AST] Print correct tag decl for tag specifier

jdenny updated this revision to Diff 142673. jdenny added a comment. Rebased onto a more recent master. https://reviews.llvm.org/D45463 Files: include/clang-c/Index.h include/clang/AST/PrettyPrinter.h lib/AST/DeclPrinter.cpp lib/AST/TypePrinter.cpp test/Misc/ast-print-enum-decl.c te

[PATCH] D45465: [AST] Fix printing tag decl groups in decl contexts

jdenny updated this revision to Diff 142687. jdenny added a comment. Rebased onto a more recent master. https://reviews.llvm.org/D45465 Files: lib/AST/DeclPrinter.cpp test/Misc/ast-print-enum-decl.c test/Misc/ast-print-record-decl.c test/Sema/ast-print.c test/SemaCXX/ast-print.cpp In

[PATCH] D45093: [AST] Fix -ast-print for _Bool when have diagnostics

jdenny updated this revision to Diff 143667. jdenny added a comment. Rebased. https://reviews.llvm.org/D45093 Files: include/clang/AST/ASTContext.h include/clang/Sema/Sema.h lib/Parse/ParseDecl.cpp lib/Sema/Sema.cpp lib/Sema/SemaChecking.cpp lib/Sema/SemaCodeComplete.cpp lib/Sema/

[PATCH] D45456: [Attr] Print enum attributes at correct position

jdenny added a comment. Ping. In case it's not clear: this patch doesn't depend on other patches, but others depend on it. https://reviews.llvm.org/D45456 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailma

[PATCH] D45456: [Attr] Print enum attributes at correct position

jdenny added a comment. Thanks. https://reviews.llvm.org/D45456 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D45456: [Attr] Print enum attributes at correct position

This revision was automatically updated to reflect the committed changes. Closed by commit rL330722: [Attr] Print enum attributes at correct position (authored by jdenny, committed by ). Herald added a subscriber: llvm-commits. Changed prior to commit: https://reviews.llvm.org/D45456?vs=142672&

[PATCH] D45456: [Attr] Print enum attributes at correct position

This revision was automatically updated to reflect the committed changes. Closed by commit rC330722: [Attr] Print enum attributes at correct position (authored by jdenny, committed by ). Repository: rL LLVM https://reviews.llvm.org/D45456 Files: lib/AST/DeclPrinter.cpp test/Sema/ast-print

[PATCH] D46370: [OPENMP] Fix test typos: CHECK-DAG-N -> CHECK-N-DAG

jdenny created this revision. jdenny added a reviewer: ABataev. Herald added a subscriber: guansong. https://reviews.llvm.org/D46370 Files: test/OpenMP/target_teams_distribute_firstprivate_codegen.cpp test/OpenMP/target_teams_distribute_parallel_for_firstprivate_codegen.cpp test/OpenMP/targ

[PATCH] D46370: [OPENMP] Fix test typos: CHECK-DAG-N -> CHECK-N-DAG

This revision was automatically updated to reflect the committed changes. Closed by commit rC331469: [OPENMP] Fix test typos: CHECK-DAG-N -> CHECK-N-DAG (authored by jdenny, committed by ). Changed prior to commit: https://reviews.llvm.org/D46370?vs=144949&id=145047#toc Repository: rC Clang

[PATCH] D45093: [AST] Fix -ast-print for _Bool when have diagnostics

jdenny updated this revision to Diff 145078. jdenny edited the summary of this revision. jdenny added a comment. Rebased. Added example to summary. Ping. https://reviews.llvm.org/D45093 Files: include/clang/AST/ASTContext.h include/clang/Sema/Sema.h lib/Parse/ParseDecl.cpp lib/Sema/Se

[PATCH] D45463: [AST] Print correct tag decl for tag specifier

jdenny updated this revision to Diff 145092. jdenny removed a reviewer: jbcoe. jdenny added a comment. Rebased. Ping. https://reviews.llvm.org/D45463 Files: include/clang-c/Index.h include/clang/AST/PrettyPrinter.h lib/AST/DeclPrinter.cpp lib/AST/TypePrinter.cpp test/Misc/ast-print-e

[PATCH] D45463: [AST] Print correct tag decl for tag specifier

jdenny added a comment. In https://reviews.llvm.org/D45463#1087174, @rsmith wrote: > > TagSpecifierAs expands sizeof(PrintingPolicy) from 8 to 16 bytes. My > > concern is the comments on PrintingPolicy about how PrintingPolicy is > > intended to be small. My guess it that 16 bytes is still smal

[PATCH] D45463: [AST] Print correct tag decl for tag specifier

jdenny added inline comments. Comment at: lib/AST/DeclPrinter.cpp:180-181 if (isFirst) { - if(TD) -SubPolicy.IncludeTagDefinition = true; + if (TD) +SubPolicy.TagSpecifierAs = TD; SubPolicy.SuppressSpecifiers = false; rsmith

[PATCH] D45093: [AST] Fix -ast-print for _Bool when have diagnostics

jdenny added a comment. In https://reviews.llvm.org/D45093#1087652, @aaron.ballman wrote: > This approach generally looks good to me, but I'd like @rsmith's opinion on > whether we should be trying to make -ast-print have good source fidelity or > not. I was under the impression we wanted -ast-

[PATCH] D45093: [AST] Fix -ast-print for _Bool when have diagnostics

jdenny added a comment. In https://reviews.llvm.org/D45093#1090292, @rsmith wrote: > If you want to force a particular printing policy to be used for > `-ast-print`, I think it would be better to change the `print` call in > `lib/Frontend/ASTConsumers.cpp` to pass your desired printing policy,

[PATCH] D48735: [OPENMP] Fix incomplete type check for array reductions

jdenny created this revision. jdenny added a reviewer: ABataev. Herald added a subscriber: guansong. A reduction for an incomplete array type used to produce an assert fail during codegen. Now it produces a diagnostic. Repository: rC Clang https://reviews.llvm.org/D48735 Files: clang/lib/

[PATCH] D48735: [OPENMP] Fix incomplete type check for array reductions

This revision was automatically updated to reflect the committed changes. Closed by commit rC335911: [OPENMP] Fix incomplete type check for array reductions (authored by jdenny, committed by ). Changed prior to commit: https://reviews.llvm.org/D48735?vs=153366&id=153374#toc Repository: rC Cl

[PATCH] D46919: [libclang] Deprecate CXPrintingPolicy_IncludeTagDefinition

jdenny updated this revision to Diff 153930. jdenny added a comment. Ping. Rebased. https://reviews.llvm.org/D46919 Files: include/clang-c/Index.h include/clang/AST/PrettyPrinter.h lib/AST/DeclPrinter.cpp lib/AST/TypePrinter.cpp tools/c-index-test/c-index-test.c Index: tools/c-index

[PATCH] D83922: [OpenMP] Fix map clause for unused var: don't ignore it

jdenny created this revision. jdenny added reviewers: ABataev, jdoerfert, hfinkel, Meinersbur, kkwli0, grokos, sfantao, gtbercea, Hahnfeld. Herald added subscribers: cfe-commits, sstefan1, guansong, yaxunl. Herald added a project: clang. For example, without this patch: $ cat test.c int main

[PATCH] D83061: [OpenMP] Implement TR8 `present` map type modifier in Clang (1/2)

jdenny updated this revision to Diff 278343. jdenny edited the summary of this revision. jdenny set the repository for this revision to rG LLVM Github Monorepo. jdenny added a comment. Rebased, and extracted D83922 as discussed. Repository: rG LLVM Github Mono

[PATCH] D83922: [OpenMP] Fix map clause for unused var: don't ignore it

jdenny updated this revision to Diff 278546. jdenny marked 7 inline comments as done. jdenny edited the summary of this revision. jdenny added a comment. Use `CanonicalDeclPtr`, as suggested. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D83922/new/ https://reviews.llvm.org/D83922 Files

[PATCH] D83922: [OpenMP] Fix map clause for unused var: don't ignore it

jdenny marked an inline comment as done. jdenny added inline comments. Comment at: clang/lib/CodeGen/CGOpenMPRuntime.cpp:7948-7949 + MapFlagsArrayTy &Types, + const llvm::DenseSet &SkipVarSet = + llvm::DenseSet

[PATCH] D83922: [OpenMP] Fix map clause for unused var: don't ignore it

jdenny added a comment. In D83922#2155749 , @ABataev wrote: > I would add checks for mapping of `declare target to/link` vars and checked > if they work in runtime. There are existing codegen tests for that, and they don't seem to be affected by this p

[PATCH] D83922: [OpenMP] Fix map clause for unused var: don't ignore it

jdenny added a comment. In D83922#2156567 , @ABataev wrote: > In D83922#2156510 , @jdenny wrote: > > > In D83922#2155749 , @ABataev wrote: > > > > > I would add checks for ma

[PATCH] D83061: [OpenMP] Implement TR8 `present` map type modifier in Clang (1/2)

jdenny marked an inline comment as done. jdenny added inline comments. Comment at: clang/lib/CodeGen/CGOpenMPRuntime.cpp:7043-7044 OMP_MAP_CLOSE = 0x400, +/// Produce a runtime error if the data is not already allocated. +OMP_MAP_PRESENT = 0x800, /// The 16 MSBs

[PATCH] D83061: [OpenMP] Implement TR8 `present` map type modifier in Clang (1/2)

jdenny updated this revision to Diff 278658. jdenny added a comment. Rebased onto latest D83922 . Implemented rejection of `present` if not OpenMP >= 5.1. Cleaned up tests some. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D83061/new/ https://reviews.l

[PATCH] D83922: [OpenMP] Fix map clause for unused var: don't ignore it

jdenny marked an inline comment as done. jdenny added inline comments. Comment at: clang/lib/CodeGen/CGOpenMPRuntime.cpp:9470-9471 + MappedVarSet.insert(CI->getCapturedVar()->getCanonicalDecl()); +else + MappedVarSet.insert(nullptr); if (CurBase

[PATCH] D83061: [OpenMP] Implement TR8 `present` map type modifier in Clang (1/2)

jdenny added inline comments. Comment at: clang/include/clang/Basic/DiagnosticParseKinds.td:1252-1253 + "incorrect map type modifier, expected 'always', 'close', 'mapper', or 'present'">; +def err_omp_map_type_modifier_wrong_version : Error< + "map type modifier '%0' requires

[PATCH] D83922: [OpenMP] Fix map clause for unused var: don't ignore it

jdenny marked 3 inline comments as done. jdenny added a comment. Thanks for the review. Will try to push soon. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D83922/new/ https://reviews.llvm.org/D83922 ___ cfe-commits mailing list cfe-commit

[PATCH] D83061: [OpenMP] Implement TR8 `present` map type modifier in Clang (1/2)

jdenny marked 4 inline comments as done. jdenny added inline comments. Comment at: clang/lib/Parse/ParseOpenMP.cpp:3067-3068 + unsigned Type = getOpenMPSimpleClauseType(OMPC_map, PP.getSpelling(Tok)); + if (Type < OMPC_MAP_MODIFIER_unknown) +return OMPC_MAP_MODIFIER_unknown

[PATCH] D83061: [OpenMP] Implement TR8 `present` map type modifier in Clang (1/2)

jdenny marked 2 inline comments as done. jdenny added inline comments. Comment at: clang/lib/CodeGen/CGOpenMPRuntime.cpp:7934-7940 +// If any element has the present modifier, then make sure the runtime +// doesn't attempt to allocate the struct. +if (CurTypes.end() !

[PATCH] D83922: [OpenMP] Fix map clause for unused var: don't ignore it

jdenny added a comment. In D83922#2162365 , @ABataev wrote: > Looks like it breaks unified_shared_memory/close_enter_exit.c test. Could you > check this, please? It doesn't fail for me, and I didn't receive a buildbot notification. Do you have a link?

[PATCH] D83061: [OpenMP] Implement TR8 `present` map type modifier in Clang (1/2)

jdenny marked 2 inline comments as done. jdenny added inline comments. Comment at: clang/lib/Parse/ParseOpenMP.cpp:3067-3068 + unsigned Type = getOpenMPSimpleClauseType(OMPC_map, PP.getSpelling(Tok)); + if (Type < OMPC_MAP_MODIFIER_unknown) +return OMPC_MAP_MODIFIER_unknown

[PATCH] D83061: [OpenMP] Implement TR8 `present` map type modifier in Clang (1/2)

jdenny marked an inline comment as done. jdenny added inline comments. Comment at: clang/lib/Parse/ParseOpenMP.cpp:3067-3068 + unsigned Type = getOpenMPSimpleClauseType(OMPC_map, PP.getSpelling(Tok)); + if (Type < OMPC_MAP_MODIFIER_unknown) +return OMPC_MAP_MODIFIER_unknown

[PATCH] D83061: [OpenMP] Implement TR8 `present` map type modifier in Clang (1/2)

jdenny added inline comments. Comment at: clang/lib/Parse/ParseOpenMP.cpp:3067-3068 + unsigned Type = getOpenMPSimpleClauseType(OMPC_map, PP.getSpelling(Tok)); + if (Type < OMPC_MAP_MODIFIER_unknown) +return OMPC_MAP_MODIFIER_unknown; + return static_cast(Type); --

[PATCH] D83061: [OpenMP] Implement TR8 `present` map type modifier in Clang (1/2)

jdenny added a comment. In D83061#2165063 , @ABataev wrote: > LG. Thanks for the review. As discussed in the review summary, please consider the following. A present map type modifier behavior that this patch does not attempt to implement is TR8 sec.

[PATCH] D83061: [OpenMP] Implement TR8 `present` map type modifier in Clang (1/2)

jdenny added a comment. In D83061#2165093 , @ABataev wrote: > In D83061#2165089 , @jdenny wrote: > > > In D83061#2165063 , @ABataev wrote: > > > > > LG. > > > > > > Thanks fo

[PATCH] D83061: [OpenMP] Implement TR8 `present` map type modifier in Clang (1/2)

jdenny added a comment. Thanks. It probably won't be soon. At the very least, I need to find the answer to question 3 from the review summary. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D83061/new/ https://reviews.llvm.org/D83061 ___ c

[PATCH] D84422: [OpenMP] Fix `present` for exit from `omp target data`

jdenny created this revision. jdenny added reviewers: grokos, ABataev, jdoerfert. Herald added subscribers: llvm-commits, openmp-commits, cfe-commits, sstefan1, guansong, yaxunl. Herald added projects: clang, OpenMP, LLVM. Without this patch, the following example fails but shouldn't according to

[PATCH] D84422: [OpenMP] Fix `present` for exit from `omp target data`

jdenny added a comment. I don't know if the OpenMP committee has any documented rationale for this behavior. I can say that the OpenACC committee is considering the same semantics. However, the issues to consider are not identical. For example, OpenACC has a separate structured reference cou

[PATCH] D84422: [OpenMP] Fix `present` for exit from `omp target data`

jdenny added a comment. In D84422#2170702 , @RaviNarayanaswamy wrote: > In D84422#2170667 , @grokos wrote: > > > So is the test case that motivated this patch illegal OpenMP code? > > > > #pragma omp target enter

[PATCH] D84422: [OpenMP] Fix `present` for exit from `omp target data`

jdenny added a comment. In D84422#2172898 , @jdenny wrote: > Has anyone clarified the motivation for this behavior? I meant, is there any insight into why the spec specifies this behavior? In D84422#2172926 , @gr

[PATCH] D84422: [OpenMP] Fix `present` for exit from `omp target data`

jdenny marked an inline comment as done. jdenny added a comment. In D84422#2173449 , @RaviNarayanaswamy wrote: > In D84422#2173372 , @jdenny wrote: > > > In D84422#2172898 ,

[PATCH] D84422: [OpenMP] Fix `present` for exit from `omp target data`

jdenny added a comment. In D84422#2176802 , @grokos wrote: > In D84422#2173500 , @jdenny wrote: > > > I've added a comment to the runtime code that performs the check. As you > > can see, the check is performed re

[PATCH] D84422: [OpenMP] Fix `present` for exit from `omp target data`

jdenny updated this revision to Diff 281067. jdenny edited the summary of this revision. jdenny added a comment. Rewrite patch as discussed: instead of generating different runtime calls for the end of an `omp target data` vs. the beginning of an `omp target exit data` so that the runtime can de

[PATCH] D84710: [OpenMP][NFC] Consolidate `to` and `from` clause modifiers

jdenny created this revision. jdenny added reviewers: ABataev, grokos, jdoerfert, RaviNarayanaswamy. Herald added subscribers: cfe-commits, sstefan1, guansong, yaxunl. Herald added a project: clang. `to` and `from` clauses take the same modifiers, which are called "motion modifiers" in TR8, so imp

[PATCH] D84711: [OpenMP] Implement TR8 `present` motion modifier in Clang (1/2)

jdenny created this revision. jdenny added reviewers: ABataev, grokos, jdoerfert, RaviNarayanaswamy. Herald added subscribers: cfe-commits, sstefan1, guansong, yaxunl. Herald added a project: clang. This patch implements Clang front end support for the OpenMP TR8 `present` motion modifier for `om

[PATCH] D84422: [OpenMP] Fix `present` for exit from `omp target data`

jdenny updated this revision to Diff 281330. jdenny added a comment. Replaced `SeparateBeginEnd` parameter with new `TargetDataInfo` field as requested. Rebased. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D84422/new/ https://reviews.llvm.org/D84422 Files: clang/lib/CodeGen/CGOpen

[PATCH] D84710: [OpenMP][NFC] Consolidate `to` and `from` clause modifiers

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

[PATCH] D84711: [OpenMP] Implement TR8 `present` motion modifier in Clang (1/2)

jdenny added inline comments. Comment at: clang/lib/Parse/ParseOpenMP.cpp:3462 + // OpenMP 5.1 accepts an optional ',' even if the next character is ':'. + // TODO: Is that intentional? + if (Tok.is(tok::comma)) ABataev wrote: > `FIXME`. This is a

[PATCH] D84711: [OpenMP] Implement TR8 `present` motion modifier in Clang (1/2)

jdenny updated this revision to Diff 281339. jdenny marked 4 inline comments as done. jdenny added a comment. Adjusted logic for rejecting `present` as requested. Rebased. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D84711/new/ https://reviews.llvm.org/D84711 Files: clang/include/c

[PATCH] D84711: [OpenMP] Implement TR8 `present` motion modifier in Clang (1/2)

jdenny added inline comments. Comment at: clang/lib/Basic/OpenMPKinds.cpp:73-74 .Default(OMPC_MOTION_MODIFIER_unknown); +if (OpenMPVersion < 51 && Type != OMPC_MOTION_MODIFIER_mapper) + return OMPC_MOTION_MODIFIER_unknown; +return Type; ABat

[PATCH] D84711: [OpenMP] Implement TR8 `present` motion modifier in Clang (1/2)

jdenny added a comment. Thanks! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D84711/new/ https://reviews.llvm.org/D84711 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D84422: [OpenMP] Fix `present` for exit from `omp target data`

jdenny marked an inline comment as done. jdenny added inline comments. Comment at: clang/lib/CodeGen/CGOpenMPRuntime.cpp:8843 llvm::Value *&MapTypesArrayArg, llvm::Value *&MappersArrayArg, -CGOpenMPRuntime::TargetDataInfo &Info) { +CGOpenMPRuntime::TargetDataInfo &In

[PATCH] D84710: [OpenMP][NFC] Consolidate `to` and `from` clause modifiers

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGa3d1f88fa7da: [OpenMP][NFC] Consolidate `to` and `from` clause modifiers (authored by jdenny). Repository: rG LLVM Github Monorepo CHANGES SINCE

[PATCH] D84711: [OpenMP] Implement TR8 `present` motion modifier in Clang (1/2)

This revision was automatically updated to reflect the committed changes. Closed by commit rG3c3faae49704: [OpenMP] Implement TR8 `present` motion modifier in Clang (1/2) (authored by jdenny). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D84711/new/

[PATCH] D84711: [OpenMP] Implement TR8 `present` motion modifier in Clang (1/2)

jdenny added inline comments. Comment at: clang/lib/CodeGen/CGOpenMPRuntime.cpp:8055 for (const auto L : C->component_lists()) { -InfoGen(std::get<0>(L), std::get<1>(L), OMPC_MAP_to, llvm::None, +SmallVector MapModifiers; +translateMotionModifiers(C

[PATCH] D84711: [OpenMP] Implement TR8 `present` motion modifier in Clang (1/2)

jdenny added inline comments. Comment at: clang/lib/CodeGen/CGOpenMPRuntime.cpp:8055 for (const auto L : C->component_lists()) { -InfoGen(std::get<0>(L), std::get<1>(L), OMPC_MAP_to, llvm::None, +SmallVector MapModifiers; +translateMotionModifiers(C

[PATCH] D84422: [OpenMP] Fix `present` for exit from `omp target data`

jdenny marked an inline comment as done. jdenny added a comment. Thanks for the review. As discussed during the 7/29 call, I'll wait to push until we're sure about what the OpenMP committee intended here. I'm pursuing this and will report back when I have more information. CHANGES SINCE LAST

[PATCH] D80944: Add begin source location for the attributed statement created from PragmaLoopHint decorated loop

jdenny added a comment. This looks like a continuation of the work started in D61643 (which makes the loc of `#pragma` available to pragma handlers) and D61509 (which changes the start loc for OpenMP pragmas from `omp` to `#pra

[PATCH] D80944: Add begin source location for the attributed statement created from PragmaLoopHint decorated loop

jdenny accepted this revision. jdenny added a comment. This revision is now accepted and ready to land. LGTM except for the nit I put in a comment. However, I have little experience with these particular pragmas. Before pushing, please give others a few more days in case they want to review.

[PATCH] D80944: Add begin source location for the attributed statement created from PragmaLoopHint decorated loop

jdenny marked an inline comment as done. jdenny added inline comments. Comment at: clang/test/AST/sourceranges.cpp:116 + void f() { +// CHECK: AttributedStmt {{.*}} +DO_PRAGMA (unroll(2)) ychen wrote: > jdenny wrote: > > Is there any reason not to check

[PATCH] D81422: Change filecheck default to dump input on failure

jdenny added a comment. Also, please grep for `FILECHECK_DUMP_INPUT_ON_FAILURE`. There are a few more occurrences to remove from the repo. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D81422/new/ https://reviews.llvm.org/D81422 ___

[PATCH] D81422: Change filecheck default to dump input on failure

jdenny added inline comments. Comment at: llvm/utils/FileCheck/FileCheck.cpp:117 - "FILECHECK_DUMP_INPUT_ON_FAILURE environment variable.\n" - "This option is deprecated in favor of -dump-input=fail.\n")); Please mention in the patch sum

[PATCH] D81422: Change filecheck default to dump input on failure

jdenny added a comment. In D81422#2080861 , @mehdi_amini wrote: > In D81422#2080758 , @probinson wrote: > > > I don't remember the exact reasoning but I believe it had something to do > > with bot logs? @jdenny or

[PATCH] D81422: Change filecheck default to dump input on failure

jdenny added a comment. By the way, `-vv` combined with `-dump-input=fail` provides a lot of additional information that can be helpful when debugging. Bots could pass that via `FILECHECK_OPTS`, or it could become the default when `-dump-input` is not `never`. In the latter case, I suggest a

[PATCH] D81422: Change filecheck default to dump input on failure

jdenny added inline comments. Comment at: llvm/test/FileCheck/dump-input-enable.txt:77 ;-- -; Check no -dump-input, which defaults to never. ;-- The previous section

[PATCH] D81422: Change filecheck default to dump input on failure

jdenny accepted this revision. jdenny added a comment. LGTM. Thanks! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D81422/new/ https://reviews.llvm.org/D81422 ___ cfe-commits mailing list cfe-commits@l

<    1   2   3   4   5   >