[PATCH] D154130: [lit] Avoid os.path.realpath on Windows due to MAX_PATH limitations

jdenny added a comment. In D154130#4502036 , @MrTrillian wrote: > In D154130#4487292 , @jdenny wrote: > >> 3. Extend lit's own test suite to cover it. > > I submitted an update with your suggestions #1 and #2 but

[PATCH] D154130: [lit] Avoid os.path.realpath on Windows due to MAX_PATH limitations

jdenny added a comment. 3. Extend lit's own test suite to cover it. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D154130/new/ https://reviews.llvm.org/D154130 ___ cfe-commits mailing list cfe-commits@lis

[PATCH] D154130: [lit] Avoid os.path.realpath on Windows due to MAX_PATH limitations

jdenny added a comment. If the `%>` feature is going to remain (depending on, for example, the answer to @tahonermann's question about modulemap), please: 1. Add it to the lit documentation at https://llvm.org/docs/TestingGuide.html#substitutions and

[PATCH] D151350: [OpenMP] Extend omp teams to permit nested omp tile

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

[PATCH] D151356: [OpenMP] Fix transformed loop's var privacy

jdenny added a comment. Thanks for the review. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D151356/new/ https://reviews.llvm.org/D151356 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://l

[PATCH] D151356: [OpenMP] Fix transformed loop's var privacy

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG19841e4dcaab: [OpenMP] Fix transformed loop's var privacy (authored by jdenny). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION h

[PATCH] D151320: [clang] Add `// expected-maybe-no-diagnostics` comment to VerifyDiagnosticConsumer

jdenny accepted this revision. jdenny added inline comments. Comment at: clang/lib/Frontend/VerifyDiagnosticConsumer.cpp:468 +Status = VerifyDiagnosticConsumer::HasExpectedMaybeNoDiagnostics; + continue; +} else if (DToken.endswith(DType="-no-diagnostics")) { ---

[PATCH] D151320: [clang] Add `// expected-maybe-no-diagnostics` comment to VerifyDiagnosticConsumer

jdenny added inline comments. Comment at: clang/lib/Frontend/VerifyDiagnosticConsumer.cpp:468 +Status = VerifyDiagnosticConsumer::HasExpectedMaybeNoDiagnostics; + continue; +} else if (DToken.endswith(DType="-no-diagnostics")) { Endill wrote: > j

[PATCH] D151320: [clang] Add `// expected-maybe-no-diagnostics` comment to VerifyDiagnosticConsumer

jdenny accepted this revision. jdenny added a comment. This revision is now accepted and ready to land. Thanks for addressing all my concerns. LGTM except for a test issue I just commented on. Comment at: clang/lib/Frontend/VerifyDiagnosticConsumer.cpp:468 +Status = Ve

[PATCH] D151320: [clang] Add `// expected-maybe-no-diagnostics` comment to VerifyDiagnosticConsumer

jdenny added inline comments. Comment at: clang/include/clang/Frontend/VerifyDiagnosticConsumer.h:186-202 +/// Additionally, you can use: +/// +/// \code +/// // expected-maybe-no-diagnostics +/// \endcode +/// +/// to specify that a file with no "expected-*" comments should pa

[PATCH] D151320: [clang] Add `// expected-maybe-no-diagnostics` comment to VerifyDiagnosticConsumer

jdenny added a comment. Thanks for working on this. I've been wanting something like it too. In downstream work, I've used a hack that seems to accomplish the same thing but probably shouldn't: `expected-error 0 {{}}`. Anyway, please add documentation for the new directive here: https://clan

[PATCH] D151356: [OpenMP] Fix transformed loop's var privacy

jdenny created this revision. jdenny added reviewers: Meinersbur, ABataev. jdenny added a project: OpenMP. Herald added subscribers: sunshaoce, zzheng, guansong, yaxunl. Herald added a project: All. jdenny requested review of this revision. Herald added a reviewer: jdoerfert. Herald added subscribe

[PATCH] D151350: [OpenMP] Extend omp teams to permit nested omp tile

jdenny updated this revision to Diff 525250. jdenny added a comment. Added phab review number to status doc. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D151350/new/ https://reviews.llvm.org/D151350 Files: clang/docs/OpenMPSupport.rst clang/lib/Sema/SemaOpenMP.cpp openmp/docs/op

[PATCH] D151350: [OpenMP] Extend omp teams to permit nested omp tile

jdenny created this revision. jdenny added reviewers: Meinersbur, ABataev, jhuber6, jdoerfert. jdenny added projects: OpenMP, OpenACC. Herald added subscribers: sunshaoce, guansong, yaxunl. Herald added a reviewer: clementval. Herald added a project: All. jdenny requested review of this revision. H

[PATCH] D132513: [lit] Implement DEFINE and REDEFINE directives

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

[PATCH] D132513: [lit] Implement DEFINE and REDEFINE directives

jdenny added a comment. It was a small battle to get the tests past the windows bots, but they eventually succeeded: https://lab.llvm.org/buildbot/#/builders/216/builds/10125 I pushed the following additional patches while trying to fix the tests for windows: - f47a5df92d488bdff418e3b1d249f40

[PATCH] D132513: [lit] Implement DEFINE and REDEFINE directives

This revision was automatically updated to reflect the committed changes. Closed by commit rG28412d1800e3: [lit] Implement DEFINE and REDEFINE directives (authored by jdenny). Herald added a project: clang. Herald added a subscriber: cfe-commits. Repository: rG LLVM Github Monorepo CHANGES SIN

[PATCH] D126549: [Clang][OpenMP] Don't overload "extension" in status doc

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

[PATCH] D126549: [Clang][OpenMP] Don't overload "extension" in status doc

This revision was automatically updated to reflect the committed changes. Closed by commit rG0103d4da740c: [Clang][OpenMP] Don't overload "extension" in status doc (authored by jdenny). Changed prior to commit: https://reviews.llvm.org/D126549?vs=432567&id=440419#toc Repository: rG LLVM Gith

[PATCH] D126549: [Clang][OpenMP] Don't overload "extension" in status doc

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

[PATCH] D126619: [OpenMP][Clang] Fix atomic compare for signed vs. unsigned

jdenny added a comment. Thanks for the quick review. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D126619/new/ https://reviews.llvm.org/D126619 ___ cfe-commits mailing list cfe-commits@lists.llvm.org htt

[PATCH] D126619: [OpenMP][Clang] Fix atomic compare for signed vs. unsigned

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGd2e3cb737417: [OpenMP][Clang] Fix atomic compare for signed vs. unsigned (authored by jdenny). Repository: rG LLVM Github Monorepo CHANGES SINCE

[PATCH] D126619: [OpenMP][Clang] Fix atomic compare for signed vs. unsigned

jdenny created this revision. jdenny added reviewers: tianshilei1992, ABataev, jdoerfert. jdenny added projects: OpenMP, clang. Herald added subscribers: guansong, yaxunl. Herald added a project: All. jdenny requested review of this revision. Herald added subscribers: cfe-commits, sstefan1. Withou

[PATCH] D126547: [OpenACC][OpenMP] Document atomic-in-teams extension

jdenny added a comment. Thanks for the quick review. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D126547/new/ https://reviews.llvm.org/D126547 ___ cfe-commits mailing list cfe-commits@lists.llvm.org htt

[PATCH] D126547: [OpenACC][OpenMP] Document atomic-in-teams extension

This revision was automatically updated to reflect the committed changes. Closed by commit rG4a368136693b: [OpenACC][OpenMP] Document atomic-in-teams extension (authored by jdenny). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D126547/new/ https://r

[PATCH] D126549: [Clang][OpenMP] Don't overload "extension" in status doc

jdenny created this revision. jdenny added reviewers: ABataev, jdoerfert, RaviNarayanaswamy. jdenny added a project: OpenMP. Herald added subscribers: guansong, yaxunl. Herald added a project: All. jdenny requested review of this revision. Herald added a subscriber: sstefan1. Herald added a project

[PATCH] D126547: [OpenACC][OpenMP] Document atomic-in-teams extension

jdenny created this revision. jdenny added reviewers: ABataev, jdoerfert, clementval. jdenny added projects: OpenMP, OpenACC. Herald added subscribers: guansong, yaxunl. Herald added a project: All. jdenny requested review of this revision. Herald added subscribers: cfe-commits, sstefan1. Herald ad

[PATCH] D126323: [OpenMP] Extend omp teams to permit nested omp atomic

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG48ca3a5ebb15: [OpenMP] Extend omp teams to permit nested omp atomic (authored by jdenny). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST

[PATCH] D126323: [OpenMP] Extend omp teams to permit nested omp atomic

jdenny marked 2 inline comments as done. jdenny added a comment. Thanks. Will try to push tomorrow. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D126323/new/ https://reviews.llvm.org/D126323 ___ cfe-commits mailing list cfe-commits@lists.llv

[PATCH] D126323: [OpenMP] Extend omp teams to permit nested omp atomic

jdenny updated this revision to Diff 431804. jdenny added a comment. Fixed a bug in the new tests. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D126323/new/ https://reviews.llvm.org/D126323 Files: clang/lib/Sema/SemaOpenMP.cpp clang/test/OpenMP/nesting_of_regions.cpp openmp/libom

[PATCH] D126323: [OpenMP] Extend omp teams to permit nested omp atomic

jdenny added inline comments. Comment at: clang/lib/Sema/SemaOpenMP.cpp:4983 + CurrentRegion != OMPD_loop && + !(SemaRef.getLangOpts().OpenMPExtensions && +CurrentRegion == OMPD_atomic); ---

[PATCH] D126323: [OpenMP] Extend omp teams to permit nested omp atomic

jdenny added inline comments. Comment at: clang/lib/Sema/SemaOpenMP.cpp:4983 + CurrentRegion != OMPD_loop && + !(SemaRef.getLangOpts().OpenMPExtensions && +CurrentRegion == OMPD_atomic); ---

[PATCH] D126323: [OpenMP] Extend omp teams to permit nested omp atomic

jdenny created this revision. jdenny added reviewers: jdoerfert, ABataev. jdenny added a project: OpenMP. Herald added subscribers: guansong, yaxunl. Herald added a project: All. jdenny requested review of this revision. Herald added subscribers: cfe-commits, sstefan1. Herald added a project: clang

[PATCH] D109255: [NFC][OpenMP] Add fsyntax-only to driver tests

jdenny accepted this revision. jdenny added a comment. LGTM. Thanks for the fix. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D109255/new/ https://reviews.llvm.org/D109255 ___ cfe-commits mailing list c

[PATCH] D109255: [NFC][OpenMP] Add fsyntax-only to driver tests

jdenny added a comment. In D109255#2983063 , @jsji wrote: > In D109255#2983052 , @ABataev wrote: > >> In D109255#2983017 , @jdenny wrote: >> >>> This test is meant to chec

[PATCH] D109255: [NFC][OpenMP] Add fsyntax-only to driver tests

jdenny added a comment. In D109255#2983018 , @jsji wrote: > In D109255#2983017 , @jdenny wrote: > >> This test is meant to check that the driver processes these options >> correctly. -cc1 isn't the driver and of

[PATCH] D109255: [NFC][OpenMP] Add fsyntax-only to driver tests

jdenny added a comment. This test is meant to check that the driver processes these options correctly. -cc1 isn't the driver and often ignores either the positive or negative version of an option. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D109

[PATCH] D107503: [clang][Rewriter] patch to fix bug with ReplaceText erasing too many characters when text has been inserted before the replace location.

jdenny added inline comments. Comment at: clang/lib/Rewrite/Rewriter.cpp:136 if (OrigLength != NewStr.size()) AddReplaceDelta(OrigOffset, NewStr.size() - OrigLength); } I think this needs to be fixed too. That is, the removal of the inserts requires up

[PATCH] D107503: [clang][Rewriter] patch to fix bug with ReplaceText erasing too many characters when text has been inserted before the replace location.

jdenny added a comment. Thanks for working on this. I agree there's a bug. However, this patch doesn't fix the bug. It provides a workaround. That is, the current default `ReplaceText` behavior, as revealed in the first test added by this patch, doesn't make sense. `IncludeInsertsAtBeginOf

[PATCH] D106509: [OpenMP][OpenACC] Implement `ompx_hold` map type modifier extension in Clang (1/2)

jdenny added a comment. Thanks! I'll wait for the D106510 review before trying to land. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D106509/new/ https://reviews.llvm.org/D106509 ___ cfe-commits mailing li

[PATCH] D106509: [OpenMP][OpenACC] Implement `ompx_hold` map type modifier extension in Clang (1/2)

jdenny added inline comments. Comment at: clang/include/clang/Driver/Options.td:2383 +defm openmp_extensions: BoolFOption<"openmp-extensions", + LangOpts<"OpenMPExtensions">, DefaultFalse, + PosFlag jdenny wrote: > > ABataev wrote: > > > Why do you want to disable it by default

[PATCH] D106509: [OpenMP][OpenACC] Implement `ompx_hold` map type modifier extension in Clang (1/2)

jdenny added inline comments. Comment at: clang/include/clang/Driver/Options.td:2383 +defm openmp_extensions: BoolFOption<"openmp-extensions", + LangOpts<"OpenMPExtensions">, DefaultFalse, + PosFlag Why do you want to disable it by default? I thought that's what we agreed upon.

[PATCH] D106509: [OpenMP][OpenACC] Implement `ompx_hold` map type modifier extension in Clang (1/2)

jdenny marked 4 inline comments as done. jdenny added inline comments. Comment at: clang/lib/Basic/OpenMPKinds.cpp:64 + return OMPC_MAP_MODIFIER_unknown; +if (!LangOpts.OpenMPExtensions && Type == OMPC_MAP_MODIFIER_ompx_hold) return OMPC_MAP_MODIFIER_unknown;

[PATCH] D106509: [OpenMP][OpenACC] Implement `ompx_hold` map type modifier extension in Clang (1/2)

jdenny added inline comments. Comment at: clang/lib/Basic/OpenMPKinds.cpp:64 + return OMPC_MAP_MODIFIER_unknown; +if (!LangOpts.OpenMPExtensions && Type == OMPC_MAP_MODIFIER_ompx_hold) return OMPC_MAP_MODIFIER_unknown; ABataev wrote: > jdenny wrot

[PATCH] D106509: [OpenMP][OpenACC] Implement `ompx_hold` map type modifier extension in Clang (1/2)

jdenny added inline comments. Comment at: clang/lib/Basic/OpenMPKinds.cpp:64 + return OMPC_MAP_MODIFIER_unknown; +if (!LangOpts.OpenMPExtensions && Type == OMPC_MAP_MODIFIER_ompx_hold) return OMPC_MAP_MODIFIER_unknown; ABataev wrote: > I would ena

[PATCH] D106509: [OpenMP][OpenACC] Implement `hold` map type modifier extension in Clang (1/2)

jdenny added a comment. In D106509#2896956 , @jdoerfert wrote: >> That's fine for me, but don't the routines use llvm_omp_? > > That was before we "standardized" `ompx_` for OpenMP 5.2. Ah. Thanks. >> Should we also have that prefix in various enumerat

[PATCH] D106509: [OpenMP][OpenACC] Implement `hold` map type modifier extension in Clang (1/2)

jdenny added a comment. Thanks for the reviews. In D106509#2896351 , @ABataev wrote: > That's strange that we need to ignore `delete` modifier, It's not ignored in general: the standard (dynamic) OpenMP reference count is set to 0 (if it isn't already)

[PATCH] D104743: [UpdateCCTestChecks] Implement --global-hex-value-regex

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG5b0a948a81e6: [UpdateCCTestChecks] Implement --global-hex-value-regex (authored by jdenny). Repository: rG LLVM Github Monorepo CHANGES SINCE LAS

[PATCH] D104742: [UpdateCCTestChecks] Implement --global-value-regex

This revision was automatically updated to reflect the committed changes. Closed by commit rG2f5b2ea6cd85: [UpdateCCTestChecks] Implement --global-value-regex (authored by jdenny). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D104742/new/ https://re

[PATCH] D104743: [UpdateCCTestChecks] Implement --global-hex-value-regex

jdenny updated this revision to Diff 360131. jdenny added a comment. Applied @jdoerfert's suggestion: generalize to all integer values. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D104743/new/ https://reviews.llvm.org/D104743 Files: clang/test/utils/update_cc_test_checks/Inputs/glob

[PATCH] D104743: [UpdateCCTestChecks] Implement --global-hex-value-regex

jdenny added a comment. In D104743#2889228 , @jdoerfert wrote: > I don't understand what we do before, and how this work, Maybe part of the confusion is that `--global-hex-value-regex` does not change how the value is expected to appear in LLVM IR: deci

[PATCH] D104742: [UpdateCCTestChecks] Implement --global-value-regex

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

[PATCH] D105873: Fix utils/update_cc_test_checks/check-globals.test on stand-alone builds

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

[PATCH] D104742: [UpdateCCTestChecks] Implement --global-value-regex

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

[PATCH] D104743: [UpdateCCTestChecks] Implement --global-hex-value-regex

jdenny updated this revision to Diff 355582. jdenny added a comment. Fix the new test for the race discussed at D104714#2841465 . CHANGES SINCE LAST ACTION https://reviews.llvm.org/D104743/new/ https://reviews.llvm.org/D104743 Files: clang/test/ut

[PATCH] D104742: [UpdateCCTestChecks] Implement --global-value-regex

jdenny updated this revision to Diff 355579. jdenny added a comment. Fix the new test for the race discussed at D104714#2841465 . CHANGES SINCE LAST ACTION https://reviews.llvm.org/D104742/new/ https://reviews.llvm.org/D104742 Files: clang/test/ut

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

jdenny added a comment. In D104714#2841457 , @thakis wrote: > Another thing: https://reviews.llvm.org/harbormaster/unit/view/789318/ > > # command stderr: > > /var/lib/buildkite-agent/builds/llvm-project/clang/test/utils/update_cc_test_checks/check-g

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

jdenny added a comment. In D104714#2841304 , @thakis wrote: > I landed a fix attempt for my thing in > fda790fbfa0cba426d5e3787429a51e09ec64c6d > . Thanks! Repository: rG LLVM Gi

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

jdenny added a comment. In D104714#2841229 , @thakis wrote: > Oh I see this assumes that lit.site.cfg.py is below clang_obj_root. I guess > that's my problem, looking… Let me know if the patch isn't general enough, or if I can help in some other way.

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

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG9eaf0d120d32: [UpdateCCTestChecks] Support --check-globals (authored by jdenny). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

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

jdenny added a comment. In D104714#2835874 , @ggeorgakoudis wrote: > LGTM too! Thanks. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D104714/new/ https://reviews.llvm.org/D104714 ___ cfe-commits mai

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

jdenny updated this revision to Diff 353998. jdenny marked an inline comment as done. jdenny added a comment. Applied arichardson's suggestion: comment on exclusion of separator comments. Rebased. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D104714/new/ https://reviews.llvm.org/D10471

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

jdenny added a comment. In D104714#2834696 , @arichardson wrote: > Thanks for working on this! Thanks for the review. > We have some tests downstream that check globals and currently have to use > `// UTC_ARGS: --disable` to manually retain them. Doe

[PATCH] D104743: [UpdateCCTestChecks] Implement --global-hex-value-regex

jdenny created this revision. jdenny added reviewers: arichardson, ggeorgakoudis, jdoerfert, MaskRay, mtrofin, greened. jdenny requested review of this revision. Herald added a subscriber: sstefan1. Herald added projects: clang, LLVM. For example, in OpenMP offload codegen tests, global variables

[PATCH] D104742: [UpdateCCTestChecks] Implement --global-value-regex

jdenny created this revision. jdenny added reviewers: arichardson, ggeorgakoudis, jdoerfert, MaskRay, mtrofin, greened. jdenny requested review of this revision. Herald added a subscriber: sstefan1. Herald added projects: clang, LLVM. `--check-globals` activates checks for all global values, and

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

jdenny added a comment. In D104714#2833238 , @jdoerfert wrote: > LG, cool :) Thanks for the reviews. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D104714/new/ https://reviews.llvm.org/D104714 ___ cf

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

jdenny updated this revision to Diff 353765. jdenny added a comment. Fixed the reported test failure. (Sorry, I had accidentally put that fix in a later patch even though the associated test is in this patch.) CHANGES SINCE LAST ACTION https://reviews.llvm.org/D104714/new/ https://reviews.l

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

jdenny created this revision. jdenny added reviewers: arichardson, ggeorgakoudis, jdoerfert, MaskRay, mtrofin, greened. jdenny requested review of this revision. Herald added a subscriber: sstefan1. Herald added projects: clang, LLVM. This option is already supported by update_test_checks.py, but

[PATCH] D104566: [UpdateCCTestChecks] Fix --replace-value-regex across RUN lines

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG2bfe0536e514: [UpdateCCTestChecks] Fix --replace-value-regex across RUN lines (authored by jdenny). Repository: rG LLVM Github Monorepo CHANGES S

[PATCH] D104566: [UpdateCCTestChecks] Fix --replace-value-regex across RUN lines

jdenny created this revision. jdenny added reviewers: arichardson, ggeorgakoudis, jdoerfert, MaskRay, mtrofin, greened. jdenny requested review of this revision. Herald added a subscriber: sstefan1. Herald added projects: clang, LLVM. Without this patch, llvm/utils/update_cc_test_checks.py fails

[PATCH] D101564: [OpenMP] Fix second debug name from map clause

This revision was automatically updated to reflect the committed changes. Closed by commit rG82e99f50351d: [OpenMP] Fix second debug name from map clause (authored by jdenny). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D101564/new/ https://reviews

[PATCH] D101564: [OpenMP] Fix second debug name from map clause

jdenny created this revision. jdenny added reviewers: jhuber6, jdoerfert. Herald added subscribers: guansong, yaxunl. jdenny requested review of this revision. Herald added a subscriber: sstefan1. Herald added a project: clang. This patch fixes a bug from D89802 .

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

jdenny added a comment. In D94973#2591210 , @Meinersbur wrote: > In D94973#2590867 , @jdenny wrote: > >> One property of this patch that has bothered me is that OMPCanonicalLoop is >> not a loop. Instead, it's an

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

jdenny added a comment. One property of this patch that has bothered me is that OMPCanonicalLoop is not a loop. Instead, it's an AST node that is sandwiched between a directive and a loop to contain extra information about the loop. The TreeTransform issues we've been discussing highlight how

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

jdenny accepted this revision. jdenny added a comment. This revision is now accepted and ready to land. Other than the additional comments I've requested, LGTM. Thanks for the explanations and the changes. Comment at: clang/lib/Sema/TreeTransform.h:8321 +TreeTransform::Transf

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

jdenny added inline comments. Comment at: clang/lib/Sema/TreeTransform.h:8326-8331 +template +StmtResult +TreeTransform::TransformOMPCanonicalLoop(OMPCanonicalLoop *L) { + llvm_unreachable("OMPCanonicalLoop must be handled by the " + "OMPExecutableDirective th

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

jdenny added a comment. This patch has no effect if the OpenMP IR builder is not enabled, and it's disabled by default. Is that right? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D94973/new/ https://reviews.llvm.org/D94973 _

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

jdenny added inline comments. Comment at: clang/lib/Sema/SemaOpenMP.cpp:5355 + Expr *Cond, *Inc; + VarDecl *CounterDecl, *LVDecl; + if (auto *For = dyn_cast(AStmt)) { jdenny wrote: > `CounterDecl` is the declaration of the "loop iteration variable" based on >

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

jdenny added inline comments. Comment at: clang/lib/CodeGen/CodeGenFunction.h:292 + /// nest would extend. + SmallVector OMPLoopNestStack; + Unless I missed something, the only accesses to `OMPLoopNestStack` are `push_back`, `clear`, `back`, and `size`. It's

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

jdenny added inline comments. Comment at: clang/include/clang/AST/StmtOpenMP.h:166 +assert((isa(S) || isa(S)) && + "Canonical loop must be a for loop (range-based or otherwise)"); +SubStmts[LOOPY_STMT] = S; Meinersbur wrote: > jdenny wrote: > >

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

jdenny added a comment. In D94973#2547383 , @jdoerfert wrote: > I have a single last comment/request. @jdenny I'll take it you finish the > review and accept as you see fit. @jdoerfert, if you've already reviewed everything and want to accept so this ca

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

jdenny added a comment. @Meinersbur Sorry, I haven't gotten very far yet in reviewing this, but I'll try to work on this more soon. Comment at: clang/include/clang/AST/StmtOpenMP.h:31 +/// Representation of an OpenMP canonical loop. +/// I think it would be

[PATCH] D94261: [clang] Add powerpc64le-none-linux-gnu to gnu toolchain for PPC64

jdenny accepted this revision. jdenny 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/D94261/new/ https://reviews.llvm.org/D94261 ___ c

[PATCH] D86119: [OPENMP50]Allow overlapping mapping in target constrcuts.

jdenny added a comment. In D86119#2330339 , @ABataev wrote: > In D86119#2330310 , @jdenny wrote: > >> Thanks for working on this. Sorry to take so long to review. Before I try >> to digest the code, I have a few

[PATCH] D86119: [OPENMP50]Allow overlapping mapping in target constrcuts.

jdenny added a comment. Thanks for working on this. Sorry to take so long to review. Before I try to digest the code, I have a few high-level questions. Based on the test suite changes, `TARGET_PARAM` is disappearing from many cases. Can you explain a bit how that supports overlapping and re

[PATCH] D85692: python bindings: fix DeprecationWarning

jdenny added a comment. In D85692#2228860 , @nickdesaulniers wrote: > In D85692#2227531 , @jdenny wrote: > >> Does this mean we officially no longer support python 2, which this change >> breaks? > > Send a patch

[PATCH] D85692: python bindings: fix DeprecationWarning

jdenny added a comment. Does this mean we officially no longer support python 2, which this change breaks? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D85692/new/ https://reviews.llvm.org/D85692 ___ cf

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

This revision was automatically updated to reflect the committed changes. Closed by commit rG002d61db2b77: [OpenMP] Fix `present` for exit from `omp target data` (authored by jdenny). Changed prior to commit: https://reviews.llvm.org/D84422?vs=281330&id=283224#toc Repository: rG LLVM Github

[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] 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] 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] 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] 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] 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] 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 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/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] 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] 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

  1   2   3   4   5   >