[PATCH] D59712: [APSInt][OpenMP] Fix isNegative, etc. for unsigned types

jdenny added a comment. In D59712#1439966 , @lebedev.ri wrote: > Might warrant test coverage in `llvm/unittest/ADT/APSIntTest.cpp`. Thanks. I'll work on that. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/

[PATCH] D59712: [APSInt][OpenMP] Fix isNegative, etc. for unsigned types

jdenny updated this revision to Diff 191938. jdenny added a comment. Extend llvm/unittests/ADT/APSIntTest.cpp. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59712/new/ https://reviews.llvm.org/D59712 Files: clang/test/OpenMP/distribute_collapse_

[PATCH] D59712: [APSInt][OpenMP] Fix isNegative, etc. for unsigned types

jdenny added a comment. Ping. To be clear, my goal here was not to change the OpenMP behavior. My goal was to fix APSInt behavior. If people feel that the old OpenMP behavior is better somehow, we should surely find another way to implement that. Repository: rG LLVM Github Monorepo CHANG

[PATCH] D59712: [APSInt][OpenMP] Fix isNegative, etc. for unsigned types

jdenny added a comment. Thanks. Then the patch just needs someone to review from the ADT side. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59712/new/ https://reviews.llvm.org/D59712 ___ cfe-commits

[PATCH] D60732: [analyzer] NFC: Use -verify=... in MoveChecker tests.

jdenny added a comment. > Just wanted to give a bit more visibility to the underrated technology of > writing -verify=a,b,c instead of the #ifdefclutter. It's great to see this getting more use. > It's also a bit wonky because it seems that you have to write an exponential > number of flags to

[PATCH] D60732: [analyzer] NFC: Use -verify=... in MoveChecker tests.

jdenny added a comment. In D60732#1469026 , @jdenny wrote: > Sometimes you can avoid an explosion of FileCheck prefixes using -D. I'm not > aware of anything like -D for -verify. I mean a -D that expands a variable within an expected diagnostic. Repo

[PATCH] D59712: [APSInt][OpenMP] Fix isNegative, etc. for unsigned types

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

[PATCH] D59168: [runtimes] Move libunwind, libc++abi and libc++ to lib/clang/ and include/

jdenny added a comment. Is there anything I can do to help this patch make progress? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59168/new/ https://reviews.llvm.org/D59168 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://li

[PATCH] D59712: [APSInt][OpenMP] Fix isNegative, etc. for unsigned types

jdenny marked an inline comment as done. jdenny added inline comments. Comment at: llvm/unittests/ADT/APSIntTest.cpp:188-194 + EXPECT_FALSE(CharBoundaryUnder.isNegative()); + EXPECT_TRUE(CharBoundaryUnder.isNonNegative()); + EXPECT_TRUE(CharBoundaryUnder.isStrictlyPositive());

[PATCH] D59712: [APSInt][OpenMP] Fix isNegative, etc. for unsigned types

jdenny updated this revision to Diff 195452. jdenny added a comment. Duplicate new APSInt test but for signed type. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59712/new/ https://reviews.llvm.org/D59712 Files: clang/test/OpenMP/distribute_collapse_messages.cpp clang/test/OpenMP/d

[PATCH] D59712: [APSInt][OpenMP] Fix isNegative, etc. for unsigned types

jdenny added a comment. In D59712#1469301 , @lebedev.ri wrote: > In D59712#1469295 , @craig.topper > wrote: > > > Wondering if it would be better to assert for asking for the sign of an > > unsigned APSInt. I coul

[PATCH] D59712: [APSInt][OpenMP] Fix isNegative, etc. for unsigned types

jdenny added a comment. In D59712#1469392 , @lebedev.ri wrote: > In D59712#1469358 , @jdenny wrote: > > > In D59712#1469301 , @lebedev.ri > > wrote: > > > > > In D59712#1469

[PATCH] D60845: [VerifyDiagnosticConsumer] Document -verify= in doxygen

jdenny created this revision. jdenny added reviewers: NoQ, Charusso, hfinkel, rsmith. Herald added a project: clang. Previously, it was only documented by `-cc1 -help`, so people weren't aware of it, as discussed in D60732 . Repository: rG LLVM Github Monorepo

[PATCH] D60845: [VerifyDiagnosticConsumer] Document -verify= in doxygen

jdenny updated this revision to Diff 195742. jdenny added a comment. Clarify the behavior of multiple -verify options. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D60845/new/ https://reviews.llvm.org/D60845 Files: clang/include/clang/Frontend/VerifyDiagnosticConsumer.h Index: clan

[PATCH] D60845: [VerifyDiagnosticConsumer] Document -verify= in doxygen

jdenny added a comment. Thanks for the accepts. In D60845#1470986 , @NoQ wrote: > In D60845#1470980 , @Charusso wrote: > > > I really like live working examples, I hope not just me. Could you link > > https://gith

[PATCH] D59168: [runtimes] Move libunwind, libc++abi and libc++ to lib/clang/ and include/

jdenny added a comment. In D59168#1470578 , @phosek wrote: > In D59168#1469186 , @jdenny wrote: > > > > > > I was also thinking about alternative names for the library path, > specifically for headers we use `inclu

[PATCH] D60845: [VerifyDiagnosticConsumer] Document -verify= in doxygen

jdenny added a comment. In D60845#1472291 , @rsmith wrote: > In D60845#1471741 , @jdenny wrote: > > > In D60845#1471002 , @rsmith wrote: > > > > > I've seen a few projects ou

[PATCH] D59712: [APSInt][OpenMP] Fix isNegative, etc. for unsigned types

jdenny added a comment. In D59712#1469760 , @lebedev.ri wrote: > In D59712#1469693 , @jdenny wrote: > > > In D59712#1469392 , @lebedev.ri > > wrote: > > > > > In D59712#1469

[PATCH] D59712: [APSInt][OpenMP] Fix isNegative, etc. for unsigned types

jdenny added a comment. In D59712#1472342 , @lebedev.ri wrote: > In D59712#1472318 , @jdenny wrote: > > > For me, check-all's success is not affected by the current patch. I built > > all subprojects except llgo,

[PATCH] D59712: [APSInt][OpenMP] Fix isNegative, etc. for unsigned types

jdenny added a comment. In D59712#1472358 , @hfinkel wrote: > > I've never tried running the other tests you mention, for any patch. I > > thought people normally left those to the bots. Should this patch be > > handled differently? > > We have a lot o

[PATCH] D59712: [APSInt][OpenMP] Fix isNegative, etc. for unsigned types

jdenny added a comment. In D59712#1469392 , @lebedev.ri wrote: > Does this pass `check-all`? `check-all` of stage-2? test-suite? For me, all these tests behave with the current patch. As before, the only subproject I did not build was llgo. Repositor

[PATCH] D59712: [APSInt][OpenMP] Fix isNegative, etc. for unsigned types

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

[PATCH] D60845: [VerifyDiagnosticConsumer] Document -verify= in doxygen

This revision was automatically updated to reflect the committed changes. Closed by commit rC358917: [VerifyDiagnosticConsumer] Document -verify= in doxygen (authored by jdenny, committed by ). Changed prior to commit: https://reviews.llvm.org/D60845?vs=195742&id=196127#toc Repositor

[PATCH] D59168: [runtimes] Move libunwind, libc++abi and libc++ to lib/clang/ and include/

jdenny added a comment. Does the following match what you have in mind? $prefix/ include/ c++/ v1/ limits.h ... openmp/ v1/ <-- I don't think openmp has anything like this now omp.h ompt.h ... lib/ c

[PATCH] D59712: [APSInt][OpenMP] Fix isNegative, etc. for unsigned types

This revision was automatically updated to reflect the committed changes. Closed by commit rC359012: [APSInt][OpenMP] Fix isNegative, etc. for unsigned types (authored by jdenny, committed by ). Changed prior to commit: https://reviews.llvm.org/D59712?vs=195626&id=196274#toc Repository: rC C

[PATCH] D59168: [runtimes] Move libunwind, libc++abi and libc++ to lib/clang/ and include/

jdenny added a comment. In D59168#1479939 , @phosek wrote: > In D59168#1474715 , @jdenny wrote: > > > Does the following match what you have in mind? > > > > $prefix/ > > include/ > > c++/ > > v1

[PATCH] D59168: [runtimes] Move libunwind, libc++abi and libc++ to lib/$target/c++ and include/c++

jdenny added a comment. In D59168#1480801 , @phosek wrote: > In D59168#1480428 , @jdenny wrote: > > > It seems we have roughly the same situation for their ABIs. That is, for > > .so files, someone noted that libc

[PATCH] D59168: [runtimes] Move libunwind, libc++abi and libc++ to lib/$target/c++ and include/c++

jdenny added a comment. In D59168#1484754 , @phosek wrote: > It's the `LIBCXX_ABI_VERSION` CMake option, see > https://github.com/llvm/llvm-project/blob/master/libcxx/CMakeLists.txt#L121 Thanks. Comment at: libunwind/CMakeLists.txt:1

[PATCH] D61466: [Rewrite][NFC] Add FIXME about RemoveLineIfEmpty

jdenny created this revision. jdenny added reviewers: akyrtzi, Eugene.Zelenko, rsmith. Herald added a subscriber: dexonsmith. Herald added a project: clang. I'd like to add these comments to warn others of problems I encountered when trying to use RemoveLineIfEmpty. I originally tried to fix the

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

jdenny created this revision. jdenny added reviewers: akyrtzi, Eugene.Zelenko, rsmith. Herald added subscribers: dexonsmith, mgorny. Herald added a project: clang. Some Rewrite functions are already overloaded to accept CharSourceRange, and this extends others in the same manner. I'm calling the

[PATCH] D61509: [PragmaHandler][OpenMP] Expose `#pragma` location

jdenny created this revision. jdenny added reviewers: ABataev, rnk, Eugene.Zelenko, akyrtzi, rsmith. Herald added subscribers: jdoerfert, dexonsmith, guansong. Herald added a project: clang. Currently, an OpenMP AST node's recorded location starts at the `omp` token after the `#pragma` token, and

[PATCH] D61509: [PragmaHandler][OpenMP] Expose `#pragma` location

jdenny added a comment. In D61509#1489752 , @ABataev wrote: > If the patch is going to be accepted, then definitely it must be solution #1. I certainly have no objection to that and will be happy to implement it, but can you help me to understand the ra

[PATCH] D61509: [PragmaHandler][OpenMP] Expose `#pragma` location

jdenny added a comment. In D61509#1489771 , @ABataev wrote: > In D61509#1489761 , @jdenny wrote: > > > In D61509#1489752 , @ABataev wrote: > > > > > If the patch is going to

[PATCH] D61509: [PragmaHandler][OpenMP] Expose `#pragma` location

jdenny added a comment. Thanks for explaining. I'll proceed with solution #1. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61509/new/ https://reviews.llvm.org/D61509 ___ cfe-commits mailing list cfe-

[PATCH] D61509: [PragmaHandler][OpenMP] Expose `#pragma` location

jdenny updated this revision to Diff 198046. jdenny edited the summary of this revision. jdenny added a comment. Herald added a subscriber: jfb. As discussed, implement OpenMP solution #1 (one-line change in `PragmaOpenMPHandler::HandlePragma` in `ParsePragma.cpp`), and update tests. CHANGES SI

[PATCH] D61509: [PragmaHandler][OpenMP] Expose `#pragma` location

jdenny added a comment. In D61509#1491119 , @lebedev.ri wrote: > I recommend to split this into two parts - changing `PragmaIntroducerKind > Introducer` to > `struct NameMe { PragmaIntroducerKind Kind; SourceLocation Loc};`, and the > actual openmp chan

[PATCH] D61509: [PragmaHandler][OpenMP] Expose `#pragma` location

jdenny added a comment. In D61509#1491209 , @ABataev wrote: > In D61509#1491204 , @lebedev.ri > wrote: > > > In D61509#1491158 , @jdenny wrote: > > > > > In D61509#1491119

[PATCH] D61509: [PragmaHandler][OpenMP] Expose `#pragma` location

jdenny added a comment. In D61509#1491537 , @lebedev.ri wrote: > In D61509#1491397 , @jdenny wrote: > > > In D61509#1491209 , @ABataev wrote: > > > > > In D61509#1491204

[PATCH] D61509: [PragmaHandler][OpenMP] Expose `#pragma` location

jdenny added a comment. In D61509#1491898 , @ABataev wrote: > Again, I don't see a single point why would we need this. I don't think there > is a big difference between the location of pragma keyword and `omp` keyword. @lebedev.ri : You said you're not

[PATCH] D61509: [PragmaHandler][OpenMP] Expose `#pragma` location

jdenny added a comment. Thanks for everyone's responses so far. At this point, I'm going to follow the uncontroversial suggestions from @lebedev.ri: create a `struct PragmaIntroducer`, and split the OpenMP work into a second patch. That will hopefully make it easier for @rsmith or others to c

[PATCH] D61566: Fix for bug 41747: AST Printer doesn't print nested name specifier for out of scope record definitions

jdenny added reviewers: rsmith, aaron.ballman. jdenny accepted this revision. jdenny added a comment. This revision is now accepted and ready to land. LGTM except for nits in the tests. I'm not close to C++ support in Clang, so please give other reviewers a few days to comment just in case. I'v

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

jdenny updated this revision to Diff 198488. jdenny retitled this revision from "[PragmaHandler][OpenMP] Expose `#pragma` location" to "[PragmaHandler] Expose `#pragma` location". jdenny edited the summary of this revision. jdenny set the repository for this revision to rG LLVM Github Monorepo. jd

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

jdenny added a comment. In D61509#1493703 , @lebedev.ri wrote: > It would have been better to submit this refactor as a new patch.. Sorry, I didn't realize that was the norm. I can do that now if it would help. I can also revert changes to this patch

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

jdenny added a comment. In D61509#1493718 , @lebedev.ri wrote: > In D61509#1493714 , @jdenny wrote: > > > In D61509#1493703 , @lebedev.ri > > wrote: > > > > > It would have

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

jdenny added a comment. In D61509#1493729 , @lebedev.ri wrote: > I don't see why this differential can't be updated to only contain the > remaining part > of the diff (for the actual OpenMP change), after splitting the NFC > refactoring part. OK Rep

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

jdenny created this revision. jdenny added reviewers: ABataev, lebedev.ri. Herald added a subscriber: jdoerfert. Herald added a project: clang. Currently, a pragma AST node's recorded location starts at the namespace token (such as `omp` in the case of OpenMP) after the `#pragma` token, and the

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

jdenny updated this revision to Diff 198491. jdenny retitled this revision from "[PragmaHandler] Expose `#pragma` location" to "[OpenMP] Set pragma start loc to `#pragma` loc". jdenny edited the summary of this revision. jdenny added a comment. As requested, replace this patch with just the OpenM

[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] D46919: [libclang] Deprecate CXPrintingPolicy_IncludeTagDefinition

jdenny updated this revision to Diff 166992. jdenny set the repository for this revision to rC Clang. jdenny added a comment. Ping. Rebased. This patch intends to perform cleanup that @rsmith suggested while reviewing another patch from me. If there's no more interest, it's fine with me to ab

[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] D61466: [Rewrite][NFC] Add FIXMEs and tests for RemoveLineIfEmpty bug

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

[PATCH] D61466: [Rewrite][NFC] Add FIXMEs and tests for RemoveLineIfEmpty bug

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

[PATCH] D61466: [Rewrite][NFC] Add FIXMEs and tests for RemoveLineIfEmpty bug

jdenny added a comment. In D61466#1602917 , @jkorous wrote: > Hi @jdenny, thanks for the warning! Hi. Thanks for the review. > I think having the test disabled is fine - the main upside I see is that we > won't check it fails over and over on our CI s

[PATCH] D61466: [Rewrite][NFC] Add FIXMEs and tests for RemoveLineIfEmpty bug

jdenny updated this revision to Diff 213197. jdenny added a comment. Note the reproducer in the implementation's FIXME. The other FIXME points there, so I figure it's best not to duplicate the note. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61466/new/ https://reviews.llvm.org/D614

[PATCH] D65835: [OpenMP] Fix map/is_device_ptr with DSA on combined directive

jdenny created this revision. jdenny added a reviewer: ABataev. Herald added a subscriber: guansong. Herald added a reviewer: jdoerfert. Herald added a project: clang. For `map`, the following restriction changed in OpenMP 5.0: - OpenMP 4.5 [2.15.5.1, Restrictions]: "A list item cannot appear in

[PATCH] D65835: [OpenMP] Fix map/is_device_ptr with DSA on combined directive

jdenny marked an inline comment as done. jdenny added a comment. In D65835#1618865 , @ABataev wrote: > `is_device_ptr` can be considered as a kind of mapping clause (see 2.19.7 > Data-Mapping Attribute Rules, Clauses, and Directives), so, I assume, clan

[PATCH] D65835: [OpenMP] Fix map/is_device_ptr with DSA on combined directive

jdenny added a comment. In D65835#1619526 , @ABataev wrote: > > Maybe, but I haven't found any statement in either version that states that > > map restrictions apply to is_device_ptr. > > `is_device_ptr` is a kind of mapping clause. I assume we can exten

[PATCH] D65835: [OpenMP] Fix map/is_device_ptr with DSA on combined directive

jdenny marked 2 inline comments as done. jdenny added a comment. In D65835#1619560 , @ABataev wrote: > In D65835#1619549 , @jdenny wrote: > > > In D65835#1619526 , @ABataev w

[PATCH] D65835: [OpenMP] Fix map/is_device_ptr with DSA on combined directive

jdenny added a comment. In D65835#1621202 , @ABataev wrote: > >> See 2.19.7 Data-Mapping Attribute Rules, Clauses, and Directives > > > > I looked again. I'm still not finding any text in that section that > > implies is_device_ptr follows the same rest

[PATCH] D65835: [OpenMP] Fix map/is_device_ptr with DSA on combined directive

jdenny added a comment. In D65835#1623814 , @hfinkel wrote: > In D65835#1623772 , @ABataev wrote: > > > In D65835#1623756 , @kkwli0 wrote: > > > > > In D65835#1622042

[PATCH] D65835: [OpenMP] Fix map/is_device_ptr with DSA on combined directive

jdenny added a comment. In D65835#1623858 , @ABataev wrote: > In D65835#1623854 , @jdenny wrote: > > > In D65835#1623814 , @hfinkel wrote: > > > > > In D65835#1623772

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

jdenny updated this revision to Diff 214542. jdenny retitled this revision from "[OpenMP] Fix map/is_device_ptr with DSA on combined directive" to "[OpenMP] Permit map with DSA on combined directive". jdenny edited the summary of this revision. jdenny added a comment. I made the following changes

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

jdenny updated this revision to Diff 214559. jdenny marked 3 inline comments as done. jdenny edited the summary of this revision. jdenny set the repository for this revision to rG LLVM Github Monorepo. jdenny added a comment. In D65835#1624477 , @ABataev wr

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

jdenny added a comment. In D65835#1624615 , @ABataev wrote: > This is wrong. It affects all possible combinations and not only fof scalar > types, all of them are affected. Are you saying the patch isn't sufficient because other types need to be fixed

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

jdenny added a comment. In D65835#1624617 , @ABataev wrote: > Target teams private map will produce extra private in target context, other > constructs either. Here's what I tried: int a; #pragma omp target teams private(a) map(a) ; The same c

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

jdenny added a comment. In D65835#1624624 , @ABataev wrote: > In D65835#1624619 , @jdenny wrote: > > > In D65835#1624617 , @ABataev wrote: > > > > > Target teams private map

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

jdenny added a comment. In D65835#1624659 , @ABataev wrote: > In D65835#1624629 , @jdenny wrote: > > > In D65835#1624624 , @ABataev wrote: > > > > > In D65835#1624619

[PATCH] D61466: [Rewrite][NFC] Add FIXMEs and tests for RemoveLineIfEmpty bug

jdenny added a comment. In D61466#1613571 , @jdenny wrote: > Note the reproducer in the implementation's FIXME. The other FIXME points > there, so I figure it's best not to duplicate the note. @jkorous, is this change sufficient? Thanks. CHANGES SIN

[PATCH] D66247: [OpenMP] Fix target map for unused variables

jdenny created this revision. jdenny added reviewers: ABataev, jdoerfert, hfinkel, kkwli0. Herald added a subscriber: guansong. Herald added a project: clang. Without this patch, each of the following `map` clauses doesn't map its variable into the target region because the variable is unused in

[PATCH] D66247: [OpenMP] Fix target map for unused variables

jdenny added a comment. In D66247#1630196 , @ABataev wrote: > Do we really need to map such variables? According to standard, "The map > clause specifies how an original list item is mapped from the current task’s > data environment to a corresponding li

[PATCH] D66247: [OpenMP] Fix target map for unused variables

jdenny added a comment. In D66247#1630262 , @ABataev wrote: > In D66247#1630245 , @jdenny wrote: > > > In D66247#1630196 , @ABataev wrote: > > > > > Do we really need to map

[PATCH] D66247: [OpenMP] Fix target map for unused variables

jdenny added a comment. In D66247#1630321 , @ABataev wrote: > Try `map(a) firstprivate(a) defaultmap(scalar:tofrom)`, where `a` is `int`, > for example. The variable must be mapped as `tofrom` in this case but, most > probably, will be mapped as `to`.

[PATCH] D66247: [OpenMP] Fix target map for unused variables

jdenny added a comment. In D66247#1630352 , @ABataev wrote: > Yes, just realized that, defaultmap does not affect explicit firstprivates. > Then just check `map(a) firstprivate(a)` for `int128` type. Check that it > still has `tofrom` mapping. If so, the

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

jdenny updated this revision to Diff 215298. jdenny added a comment. Rebase. Add more tests, as requested at D66247#1630441 . CHANGES SINCE LAST ACTION https://reviews.llvm.org/D65835/new/ https://reviews.llvm.org/D65835 Files: clang/include/clang

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

jdenny added a comment. I think you're referring to stuff like `OpenMPCaptureLevel` in `ScopeInfo.h`. I wrote those changes for this patch first, as can be seen in phabricator history. I needed them for the other patch too, and I thought we were going to end up applying that patch first, so I

[PATCH] D61466: [Rewrite][NFC] Add FIXMEs and tests for RemoveLineIfEmpty bug

This revision was automatically updated to reflect the committed changes. Closed by commit rL369049: [Rewrite][NFC] Add FIXMEs and tests for RemoveLineIfEmpty bug (authored by jdenny, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Changed prior to commit:

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

jdenny updated this revision to Diff 207301. jdenny added a comment. Rebased, fixed a comment, and applied a clang-format suggestion. Ping. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61467/new/ https://reviews.llvm.org/D61467 Files: clang/include/clang/Rewrite/Core/Rewriter.h c

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

jdenny updated this revision to Diff 208247. jdenny added a comment. @jdoerfert Thanks for your reply. I added some comments to the tests. Let me know whether that's what you're looking for. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61467/new/ https://reviews.llvm.org/D61467 Fil

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

This revision was automatically updated to reflect the committed changes. Closed by commit rL365258: [Rewrite] Extend to further accept CharSourceRange (authored by jdenny, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Changed prior to commit: https://re

[PATCH] D64289: [OpenMP] Fix 128-bit long double support on target

jdenny created this revision. jdenny added a reviewer: ABataev. Herald added subscribers: jdoerfert, guansong. Herald added a project: clang. For example, without this fix, when the host is x86_64, long double is sometimes rejected when offloading to x86_64: $ cat test.c int main() { long

[PATCH] D64289: [OpenMP] Fix 128-bit long double support on target

jdenny added inline comments. Comment at: clang/lib/Sema/SemaOpenMP.cpp:1594 + !Context.getTargetInfo().hasFloat128Type() && + Context.getTargetInfo().getLongDoubleWidth() != 128) || (Ty->isIntegerType() && Context.getTypeSize(Ty) == 128 && ABa

[PATCH] D64289: [OpenMP] Fix 128-bit long double support on target

jdenny added inline comments. Comment at: clang/lib/Sema/SemaOpenMP.cpp:1594 + !Context.getTargetInfo().hasFloat128Type() && + Context.getTargetInfo().getLongDoubleWidth() != 128) || (Ty->isIntegerType() && Context.getTypeSize(Ty) == 128 && ABa

[PATCH] D64289: [OpenMP] Fix 128-bit long double support on target

jdenny added inline comments. Comment at: clang/lib/Sema/SemaOpenMP.cpp:1594 + !Context.getTargetInfo().hasFloat128Type() && + Context.getTargetInfo().getLongDoubleWidth() != 128) || (Ty->isIntegerType() && Context.getTypeSize(Ty) == 128 && ABa

[PATCH] D64289: [OpenMP] Fix 128-bit long double support on target

jdenny added inline comments. Comment at: clang/lib/Sema/SemaOpenMP.cpp:1594 + !Context.getTargetInfo().hasFloat128Type() && + Context.getTargetInfo().getLongDoubleWidth() != 128) || (Ty->isIntegerType() && Context.getTypeSize(Ty) == 128 && ABa

[PATCH] D64289: [OpenMP] Fix 128-bit long double support on target

jdenny added inline comments. Comment at: clang/lib/Sema/SemaOpenMP.cpp:1594 + !Context.getTargetInfo().hasFloat128Type() && + Context.getTargetInfo().getLongDoubleWidth() != 128) || (Ty->isIntegerType() && Context.getTypeSize(Ty) == 128 && ABa

[PATCH] D64289: [OpenMP] Fix 128-bit long double support on target

jdenny added inline comments. Comment at: clang/lib/Sema/SemaOpenMP.cpp:1594 + !Context.getTargetInfo().hasFloat128Type() && + Context.getTargetInfo().getLongDoubleWidth() != 128) || (Ty->isIntegerType() && Context.getTypeSize(Ty) == 128 && ABa

[PATCH] D64289: [OpenMP] Fix 128-bit long double support on target

jdenny added inline comments. Comment at: clang/lib/Sema/SemaOpenMP.cpp:1594 + !Context.getTargetInfo().hasFloat128Type() && + Context.getTargetInfo().getLongDoubleWidth() != 128) || (Ty->isIntegerType() && Context.getTypeSize(Ty) == 128 && ABa

[PATCH] D64289: [OpenMP] Fix 128-bit long double support on target

jdenny updated this revision to Diff 208286. jdenny added a comment. This incorporates all the improvements I think we've discussed so far: - In diagnostics, distinguish between unsupported types and non-equivalent types. - Don't treat `__float128` and `long double` as equivalent. - Compare exac

[PATCH] D64289: [OpenMP] Fix 128-bit long double support on target

jdenny added inline comments. Comment at: clang/lib/Sema/SemaOpenMP.cpp:1596 << Ty << E->getSourceRange(); + if (Ty->isRealFloatingType()) { +llvm::APFloatBase::Semantics Sem = llvm::APFloatBase::SemanticsToEnum( ABataev wrote: > Why do we need all

[PATCH] D64289: [OpenMP] Fix 128-bit long double support on target

jdenny added inline comments. Comment at: clang/lib/Sema/SemaOpenMP.cpp:1596 << Ty << E->getSourceRange(); + if (Ty->isRealFloatingType()) { +llvm::APFloatBase::Semantics Sem = llvm::APFloatBase::SemanticsToEnum( ABataev wrote: > jdenny wrote: > > A

[PATCH] D64289: [OpenMP] Fix 128-bit long double support on target

jdenny added inline comments. Comment at: clang/lib/Sema/SemaOpenMP.cpp:1596 << Ty << E->getSourceRange(); + if (Ty->isRealFloatingType()) { +llvm::APFloatBase::Semantics Sem = llvm::APFloatBase::SemanticsToEnum( ABataev wrote: > jdenny wrote: > > A

[PATCH] D64289: [OpenMP] Fix 128-bit long double support on target

jdenny added inline comments. Comment at: clang/lib/Sema/SemaOpenMP.cpp:1596 << Ty << E->getSourceRange(); + if (Ty->isRealFloatingType()) { +llvm::APFloatBase::Semantics Sem = llvm::APFloatBase::SemanticsToEnum( ABataev wrote: > jdenny wrote: > > A

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

jdenny added inline comments. Comment at: cfe/trunk/unittests/Rewrite/CMakeLists.txt:12 clangRewrite + clangTooling ) thakis wrote: > This makes RewriteTests depend on clangTooling, and in follow-ups on > clangFrontend and clangSerialization. Rewrite used

[PATCH] D64289: [OpenMP] Fix 128-bit long double support on target

jdenny abandoned this revision. jdenny added inline comments. Comment at: clang/lib/Sema/SemaOpenMP.cpp:1596 << Ty << E->getSourceRange(); + if (Ty->isRealFloatingType()) { +llvm::APFloatBase::Semantics Sem = llvm::APFloatBase::SemanticsToEnum( ABat

[PATCH] D64289: [OpenMP] Fix 128-bit long double support on target

jdenny added inline comments. Comment at: clang/lib/Sema/SemaOpenMP.cpp:1596 << Ty << E->getSourceRange(); + if (Ty->isRealFloatingType()) { +llvm::APFloatBase::Semantics Sem = llvm::APFloatBase::SemanticsToEnum( ABataev wrote: > jdenny wrote: > > A

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

jdenny added inline comments. Comment at: cfe/trunk/unittests/Rewrite/CMakeLists.txt:12 clangRewrite + clangTooling ) jdenny wrote: > thakis wrote: > > This makes RewriteTests depend on clangTooling, and in follow-ups on > > clangFrontend and clangSeriali

[PATCH] D61466: [Rewrite][NFC] Add FIXMEs and tests for RemoveLineIfEmpty bug

jdenny updated this revision to Diff 208795. jdenny retitled this revision from "[Rewrite][NFC] Add FIXME about RemoveLineIfEmpty" to "[Rewrite][NFC] Add FIXMEs and tests for RemoveLineIfEmpty bug". jdenny edited the summary of this revision. jdenny added a comment. Rebase, and add a unit test t

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

jdenny added a comment. Ping. This seems like a straight-forward extension to the Rewriter API. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61467/new/ https://reviews.llvm.org/D61467 ___ cfe-commits

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

jdenny added inline comments. Comment at: clang/unittests/Rewrite/RewriterTest.cpp:1 +//===- unittests/Rewrite/RewriteTest.cpp - RewriteBuffer tests ===// +// Oops. Need to change the description here. Repository: rG LLVM Github Monorepo CHANGES

[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

<    1   2   3   4   5   >