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

jdenny added a comment. In D81422#2083882 , @arsenm wrote: > The environment variable was removed though? I would also at least expect > this to be an option I can set at cmake time and never have to think about > again Could you set `FILECHECK_OPTS=-d

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

jdenny added a comment. In D81422#2090425 , @mehdi_amini wrote: > I'm thinking about a possible improvement here: we could have FileCheck dump > the input for the current CHECK-LABEL section only: it seems like it could > reduce the output drastically wh

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

jdenny added a comment. In D81422#2090725 , @mehdi_amini wrote: > In D81422#2090618 , @jdenny wrote: > > > In D81422#2090425 , @mehdi_amini > > wrote: > > > > > I'm thinking

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

jdenny added a comment. Perhaps llvm-dev is the right place to discuss the default behavior. Regardless of that default, perhaps there should be an option that limits the dump to N lines that always encloses the first error. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION ht

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

jdenny added a comment. This patch looks like a good idea to me. Thanks. In D81736#2090419 , @clementval wrote: > @jdoerfert Some tests in clang relies on the position of the specific enum in > the declaration. TableGen is sorting the records so the enu

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

jdenny added a comment. Thanks for addressing my comments. As @jdoerfert says, let's give others time to review. Comment at: llvm/test/TableGen/directive1.td:1 +// RUN: llvm-tblgen -gen-directive-decls -I %p/../../include %s | FileCheck %s + I realize that or

[PATCH] D83057: [OpenMP][NFC] Remove hard-coded line numbers from more tests

jdenny created this revision. jdenny added reviewers: ABataev, jdoerfert, hfinkel, grokos, sfantao, Hahnfeld. Herald added subscribers: cfe-commits, sstefan1, guansong, yaxunl. Herald added a project: clang. This is a continuation of D82224 . Repository: rG LLV

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

jdenny created this revision. jdenny added reviewers: ABataev, jdoerfert, hfinkel, Meinersbur, kkwli0, grokos, sfantao, gtbercea. Herald added subscribers: cfe-commits, sstefan1, guansong, yaxunl. Herald added a project: clang. jdenny added a parent revision: D83057: [OpenMP][NFC] Remove hard-code

[PATCH] D83057: [OpenMP][NFC] Remove hard-coded line numbers from more tests

This revision was automatically updated to reflect the committed changes. Closed by commit rGed39becd274d: [OpenMP][NFC] Remove hard-coded line numbers from more tests (authored by jdenny). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D83057/new/ ht

[PATCH] D83057: [OpenMP][NFC] Remove hard-coded line numbers from more tests

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

[PATCH] D83057: [OpenMP][NFC] Remove hard-coded line numbers from more tests

This revision was automatically updated to reflect the committed changes. Closed by commit rGed39becd274d: [OpenMP][NFC] Remove hard-coded line numbers from more tests (authored by jdenny). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D83057/new/ ht

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

jdenny updated this revision to Diff 277215. jdenny added a comment. Rebased. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D83061/new/ https://reviews.llvm.org/D83061 Files: clang/include/clang/AST/OpenMPClause.h clang/include/clang/Basic/DiagnosticParseKinds.td clang/include/cla

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

jdenny added a comment. Ping. Maybe we can start with the simplest question from the review summary: > The Clang codegen test files are difficult to maintain because of their > sizes, but I tried to insert `present` tests into them anyway to be > consistent with the existing `always` and `clos

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

jdenny added inline comments. Comment at: clang/lib/CodeGen/CGOpenMPRuntime.cpp:9566 +MEHandler.generateAllInfo(BasePointers, Pointers, Sizes, MapTypes, + CapturedVarSet, /*PresentModifierOnly=*/true); In today's OpenMP/LLVM cal

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

jdenny added inline comments. Comment at: clang/lib/CodeGen/CGOpenMPRuntime.cpp:9566 +MEHandler.generateAllInfo(BasePointers, Pointers, Sizes, MapTypes, + CapturedVarSet, /*PresentModifierOnly=*/true); ABataev wrote: > jdenny wr

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

jdenny added inline comments. Comment at: clang/lib/CodeGen/CGOpenMPRuntime.cpp:9566 +MEHandler.generateAllInfo(BasePointers, Pointers, Sizes, MapTypes, + CapturedVarSet, /*PresentModifierOnly=*/true); ABataev wrote: > jdenny wr

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

jdenny accepted this revision. jdenny added a comment. This revision is now accepted and ready to land. In D81736#2103822 , @clementval wrote: > @jdoerfert @jdenny Should we wait until Monday to go ahead with this patch? I > have several other patches tha

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

jdenny created this revision. jdenny added reviewers: ABataev, jdoerfert, hfinkel. Herald added subscribers: sstefan1, guansong, yaxunl. jdenny added reviewers: grokos, sfantao, Hahnfeld. Otherwise, it's painful to insert new code. There are many existing examples in the same test file where the

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

jdenny added a comment. My cmake skills are lacking. Why are there are so many new DEPENDS relationships where there were none before? Is it because omp_gen is generating a header file that's included (indirectly) in all those places, where apparently that sort of dependency was unusual? Hav

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

jdenny accepted this revision. jdenny added a comment. This revision is now accepted and ready to land. If you feel reasonably confident that each new DEPENDS is needed, then this LGTM. Otherwise, give it a day to see if anyone with stronger cmake skills than me has a comment. Repository: r

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

jdenny added a comment. @jdoerfert Thanks! Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D82224/new/ https://reviews.llvm.org/D82224 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-b

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

This revision was automatically updated to reflect the committed changes. Closed by commit rG01ddb2a7b044: [OpenMP][NFC] Remove hard-coded line numbers from test (authored by jdenny). Herald added a project: clang. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews

[PATCH] D78024: [FileCheck] - Fix the false positive when -implicit-check-not is used with an unknown -check-prefix.

jdenny marked an inline comment as done. jdenny added a subscriber: SjoerdMeijer. jdenny added inline comments. Comment at: llvm/lib/Support/FileCheck.cpp:1375-1377 + // We do not allow using -implicit-check-not when an explicitly specified + // check prefix is not present in t

[PATCH] D78024: [FileCheck] - Fix the false positive when -implicit-check-not is used with an unknown -check-prefix.

jdenny marked an inline comment as done. jdenny added inline comments. Comment at: llvm/lib/Support/FileCheck.cpp:1375-1377 + // We do not allow using -implicit-check-not when an explicitly specified + // check prefix is not present in the input buffer. + if ((Req.IsDefaultChe

[PATCH] D79276: [FileCheck] Support comment directives

jdenny created this revision. jdenny added reviewers: probinson, thopre, hfinkel, jhenderson, jroelofs, jdoerfert. Herald added subscribers: cfe-commits, hiraditya, arichardson, Anastasia. Herald added projects: clang, LLVM. Sometimes you want to disable a FileCheck directive without removing it

[PATCH] D79276: [FileCheck] Support comment directives

jdenny marked 4 inline comments as done. jdenny added a comment. Thanks for the review. I've replied to a few comments, and I'll address the rest later. In D79276#2017101 , @jhenderson wrote: > I hesitate to suggest this, but is it worth using `REM` as

[PATCH] D79276: [FileCheck] Support comment directives

jdenny added a comment. In D79276#2017742 , @jdenny wrote: > Thanks for the review. I've replied to a few comments, and I'll address the > rest later. > > In D79276#2017101 , @jhenderson > wrote: > > > I hesitate

[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] D79276: [FileCheck] Support comment directives

jdenny updated this revision to Diff 261962. jdenny marked 11 inline comments as done. jdenny added a comment. Addressed most reviewer comments. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D79276/new/ https://reviews.llvm.org/D79276 Files: clang/test/CodeGen/default-address-space.c

[PATCH] D79276: [FileCheck] Support comment directives

jdenny added inline comments. Comment at: llvm/docs/CommandGuide/FileCheck.rst:57 + By default, FileCheck ignores any occurrence in ``match-filename`` of any check + prefix if it is preceded on the same line by "``COM:``" or "``RUN:``". See the + section `The "COM:" directive`

[PATCH] D79276: [FileCheck] Support comment directives

jdenny updated this revision to Diff 261998. jdenny added a comment. Fixed a missed rename in the test code in the last update. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D79276/new/ https://reviews.llvm.org/D79276 Files: clang/test/CodeGen/default-address-space.c clang/test/Code

[PATCH] D79276: [FileCheck] Support comment directives

jdenny updated this revision to Diff 262141. jdenny marked 14 inline comments as done. jdenny added a comment. Applied most reviewer comments. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D79276/new/ https://reviews.llvm.org/D79276 Files: clang/test/CodeGen/default-address-space.c

[PATCH] D79276: [FileCheck] Support comment directives

jdenny added inline comments. Comment at: llvm/lib/Support/FileCheck.cpp:1928 static const char *DefaultCheckPrefixes[] = {"CHECK"}; +static const char *DefaultCommentPrefixes[] = {"COM", "RUN"}; jhenderson wrote: > Similar to what I mentioned in D79375, perha

[PATCH] D79276: [FileCheck] Support comment directives

jdenny updated this revision to Diff 262185. jdenny added a comment. My last update accidentally included D79375 . This strips it back out. Sorry about that. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D79276/new/ https://reviews.llvm.org/D79276 Fil

[PATCH] D79276: [FileCheck] Support comment directives

jdenny updated this revision to Diff 262406. jdenny marked 8 inline comments as done. jdenny added a comment. Addressed remaining reviewer comments except one, which is still under discussion. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D79276/new/ https://reviews.llvm.org/D79276 Fil

[PATCH] D79276: [FileCheck] Support comment directives

jdenny added inline comments. Comment at: llvm/test/FileCheck/comment/blank-comments.txt:9 + +CHECK: .chk:2:8: remark: CHECK: expected string found in input jhenderson wrote: > I'm assuming that FileCheck treats the entire line after the first `CHECK:` > here as

[PATCH] D79276: [FileCheck] Support comment directives

jdenny marked 5 inline comments as done. jdenny added a comment. In D79276#2024411 , @jhenderson wrote: > LGTM, but might not hurt to have someone else looking at it. I'll probably wait until Monday to give others more time to participate. Thanks for th

[PATCH] D79276: [FileCheck] Support comment directives

jdenny marked an inline comment as done. jdenny added inline comments. Comment at: llvm/test/FileCheck/comment/after-words.txt:1 +# Comment prefixes are not recognized at ends of words. + thopre wrote: > How about characters that cannot be part of an identifier?

[PATCH] D79276: [FileCheck] Support comment directives

jdenny marked an inline comment as done. jdenny added inline comments. Comment at: llvm/test/FileCheck/comment/after-words.txt:1 +# Comment prefixes are not recognized at ends of words. + jdenny wrote: > thopre wrote: > > How about characters that cannot be part

[PATCH] D79276: [FileCheck] Support comment directives

jdenny marked 2 inline comments as done. jdenny added a comment. Thanks! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D79276/new/ https://reviews.llvm.org/D79276 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org

[PATCH] D79276: [FileCheck] Support comment directives

This revision was automatically updated to reflect the committed changes. Closed by commit rG9a9a5f9893c8: [FileCheck] Support comment directives (authored by jdenny). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D79276/new/ https://reviews.llvm.org

[PATCH] D79276: [FileCheck] Support comment directives

jdenny marked an inline comment as done. jdenny added inline comments. Comment at: llvm/test/FileCheck/comment/suffixes.txt:1-2 +# Comment prefixes plus check directive suffixes are not comment directives +# and are treated as plain text. + jhenderson wrote: > jd

[PATCH] D39694: [VerifyDiagnosticConsumer] support -verify=

jdenny updated this revision to Diff 125223. jdenny added a comment. Rebased on master/trunk fetched today. https://reviews.llvm.org/D39694 Files: include/clang/Basic/DiagnosticDriverKinds.td include/clang/Basic/DiagnosticOptions.h include/clang/Driver/CC1Options.td lib/Frontend/Compile

[PATCH] D39694: [VerifyDiagnosticConsumer] support -verify=

jdenny added a comment. In https://reviews.llvm.org/D39694#944642, @hfinkel wrote: > I think this is a good idea. Thanks for the review. I've replied to each comment and will make revisions after we agree on the behavioral issue you raised. Comment at: include/clang/Driver

[PATCH] D40752: [OpenMP] Fix assert fail after target implicit map checks

jdenny added a comment. r319774 works for my current use cases. Thanks. While we're on this topic, do you happen to know the rationale behind the OpenMP restriction for which err_omp_union_type_not_allowed diagnoses violations? I googled but couldn't find the rationale. If you would prefer

[PATCH] D39694: [VerifyDiagnosticConsumer] support -verify=

jdenny added a comment. Comment at: lib/Frontend/CompilerInvocation.cpp:1095 + if (Diags) { +Diags->Report(diag::err_drv_invalid_value) << "-verify=" << Prefix; +Diags->Report(diag::note_drv_verify_prefix_unique); hfinkel wrote: > jdenny w

[PATCH] D40752: [OpenMP] Fix assert fail after target implicit map checks

jdenny added a comment. In https://reviews.llvm.org/D40752#945255, @ABataev wrote: > In https://reviews.llvm.org/D40752#945234, @jdenny wrote: > > > r319774 works for my current use cases. Thanks. > > > > While we're on this topic, do you happen to know the rationale behind the > > OpenMP restr

[PATCH] D40752: [OpenMP] Fix assert fail after target implicit map checks

jdenny added a comment. In https://reviews.llvm.org/D40752#945572, @ABataev wrote: > In https://reviews.llvm.org/D40752#945571, @jdenny wrote: > > > In https://reviews.llvm.org/D40752#945255, @ABataev wrote: > > > > > In https://reviews.llvm.org/D40752#945234, @jdenny wrote: > > > > > > > r319774

[PATCH] D39694: [VerifyDiagnosticConsumer] support -verify=

jdenny updated this revision to Diff 125640. jdenny edited the summary of this revision. jdenny added a comment. This update includes all of Hal's suggestions. I'm also thinking of converting prefix storage from a std::vector to a std::map so that lookup should be faster during parsing. https:

[PATCH] D40752: [OpenMP] Fix assert fail after target implicit map checks

jdenny added a comment. Alexey: I see that you committed the error message change, so I think this issue is done. Is Abandon Revision correct in this scenario? Sorry, I'm new here. https://reviews.llvm.org/D40752 ___ cfe-commits mailing list cfe

[PATCH] D39694: [VerifyDiagnosticConsumer] support -verify=

jdenny marked 13 inline comments as done. jdenny added a comment. I marked the comments related to Hal's suggestions as done to avoid confusion for future reviews. I'm not used to using this sort of tool for reviews. Hopefully it's appropriate for the author to do that rather than the reviewer

[PATCH] D39694: [VerifyDiagnosticConsumer] support -verify=

jdenny added a comment. In https://reviews.llvm.org/D39694#949066, @rsmith wrote: > I've not done a detailed review of the string manipulation here, but this > looks like a great feature, thanks! Hi Richard. Thanks for your feedback. I'll fix the line wrapping you pointed out. https://rev

[PATCH] D40995: [TextDiagnosticBuffer] Fix diagnostic note emission order.

jdenny created this revision. The frontend currently groups diagnostics from the command line according to diagnostic level, but that places all notes last. Fix that by emitting such diagnostics in the order they were generated. https://reviews.llvm.org/D40995 Files: include/clang/Frontend/T

[PATCH] D39694: [VerifyDiagnosticConsumer] support -verify=

jdenny updated this revision to Diff 126086. jdenny added a comment. This update does the following: 1. Fixes the line wrapping that Richard pointed out. 2. Converts from std::vector to std::set for more efficient prefix lookup. 3. Grows a dependence on https://reviews.llvm.org/D40995 (which is

[PATCH] D40995: [TextDiagnosticBuffer] Fix diagnostic note emission order.

jdenny added a comment. Hi Richard. Thanks for accepting. I don't have commit privileges. Would you please commit for me? https://reviews.llvm.org/D40995 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/

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

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] 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] 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] D65835: [OpenMP] Permit map with DSA on combined directive

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

[PATCH] D65835: [OpenMP] Permit map with DSA on combined directive

jdenny updated this revision to Diff 216420. jdenny edited the summary of this revision. jdenny set the repository for this revision to rG LLVM Github Monorepo. jdenny added a comment. As requested, backed out the changes related to firstprivate scalars, and updated tests. Repository: rG LLVM

[PATCH] D65835: [OpenMP] Permit map with DSA on combined directive

jdenny added a comment. I want to be sure we're on the same page. Due to the changes I just backed out, the following two examples now generate different code: int a = 0; #pragma omp target map(a) #pragma omp teams firstprivate(a) ; int a = 0; #pragma omp target teams firstpriv

[PATCH] D65835: [OpenMP] Permit map with DSA on combined directive

jdenny added a comment. In D65835#1639585 , @ABataev wrote: > In D65835#1639584 , @jdenny wrote: > > > I want to be sure we're on the same page. Due to the changes I just backed > > out, the following two examples

[PATCH] D65835: [OpenMP] Permit map with DSA on combined directive

jdenny added a comment. In D65835#1639612 , @ABataev wrote: > In D65835#1639593 , @jdenny wrote: > > > In D65835#1639585 , @ABataev wrote: > > > > > In D65835#1639584

[PATCH] D65835: [OpenMP] Permit map with DSA on combined directive

jdenny added a comment. In D65835#1639622 , @ABataev wrote: > We don't need this new level counter to correctly handle this situation. Just > check for the combined target directive in `Sema::isOpenMPCapturedByRef` and > return true if it is mapped as to

[PATCH] D65835: [OpenMP] Permit map with DSA on combined directive

jdenny added a comment. In D65835#1639700 , @ABataev wrote: > Chwck 2 ifs: `if (IsVariableUsedInMapClause)` and the second `if (IsByRef && > Ty.getNonReferenceType()->isScalarType())`. If the variable is mapped, > `IsByRef` is set to `true`, but later it

[PATCH] D65835: [OpenMP] Permit map with DSA on combined directive

jdenny updated this revision to Diff 216444. jdenny edited the summary of this revision. jdenny added a comment. Restore previous version of patch, and rebase. I tried, and this patch is not sufficient to fix PR40253. If there are indeed common changes, it seems it's just as well to put them he

[PATCH] D65835: [OpenMP] Permit map with DSA on combined directive

jdenny marked an inline comment as done. jdenny added inline comments. Comment at: clang/lib/Sema/Sema.cpp:2108 void Sema::PushCapturedRegionScope(Scope *S, CapturedDecl *CD, RecordDecl *RD, CapturedRegionKind K) { + CapturedRegionScopeInfo *

[PATCH] D65835: [OpenMP] Permit map with DSA on combined directive

jdenny marked an inline comment as done. jdenny added inline comments. Comment at: clang/lib/Sema/Sema.cpp:2108 void Sema::PushCapturedRegionScope(Scope *S, CapturedDecl *CD, RecordDecl *RD, CapturedRegionKind K) { + CapturedRegionScopeInfo *

[PATCH] D65835: [OpenMP] Permit map with DSA on combined directive

jdenny updated this revision to Diff 216469. jdenny marked 7 inline comments as done. jdenny set the repository for this revision to rG LLVM Github Monorepo. jdenny added a comment. Make suggested changes for passing around the capture level. Repository: rG LLVM Github Monorepo CHANGES SINCE

[PATCH] D65835: [OpenMP] Permit map with DSA on combined directive

jdenny updated this revision to Diff 216502. jdenny marked 8 inline comments as done. jdenny added a comment. Make suggested changes to default arguments, comments on literals, and parameter names. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D65835/new/ https://reviews.llvm.org/D65835

[PATCH] D65835: [OpenMP] Permit map with DSA on combined directive

jdenny added inline comments. Comment at: clang/lib/Sema/SemaExpr.cpp:17748 ObjCAvailabilityCheckExpr(Version, AtLoc, RParen, Context.BoolTy); -} \ No newline at end of file +} ABataev wrote: > Restore original code here OK, I did. What's the reason for n

[PATCH] D65835: [OpenMP] Permit map with DSA on combined directive

jdenny marked 4 inline comments as done. jdenny added a comment. Thanks. Comment at: clang/lib/Sema/SemaExpr.cpp:17748 ObjCAvailabilityCheckExpr(Version, AtLoc, RParen, Context.BoolTy); -} \ No newline at end of file +} ABataev wrote: > jdenny wrote: > >

[PATCH] D65835: [OpenMP] Permit map with DSA on combined directive

This revision was automatically updated to reflect the committed changes. jdenny marked 2 inline comments as done. Closed by commit rL369619: [OpenMP] Permit map with DSA on combined directive (authored by jdenny, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commit

[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] 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] 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] 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] 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] 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 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 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/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 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/TreeTransform.h:8326-8331 +template +StmtResult +TreeTransform::TransformOMPCanonicalLoop(OMPCanonicalLoop *L) { + llvm_unreachable("OMPCanonicalLoop must be handled by the " + "OMPExecutableDirective th

[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] 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] 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] 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 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] 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] 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] 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] 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#2835874 , @ggeorgakoudis wrote: > LGTM too! Thanks. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D104714/new/ https://reviews.llvm.org/D104714 ___ cfe-commits mai

<    1   2   3   4   5   >