[PATCH] D69088: [Lex] #pragma clang transform

Meinersbur added a comment. Herald added a subscriber: sstefan1. ping Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D69088/new/ https://reviews.llvm.org/D69088 ___ cfe-commits mailing list

[PATCH] D69088: [Lex] #pragma clang transform

Meinersbur added a comment. In D69088#2023114 , @kbarton wrote: > @Meinersbur I missed the RFC and discussion on the cfe-dev mailing list. > Could you post a link here so that it's included in the history? See the collection of links in a previous

[PATCH] D69088: [Lex] #pragma clang transform

kbarton added a comment. @Meinersbur I missed the RFC and discussion on the cfe-dev mailing list. Could you post a link here so that it's included in the history? I don't have any opposition to this, and it seems that you have addressed all the comments from reviewers. So, unless there was

[PATCH] D69088: [Lex] #pragma clang transform

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

[PATCH] D69088: [Lex] #pragma clang transform

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

[PATCH] D69088: [Lex] #pragma clang transform

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

[PATCH] D69088: [Lex] #pragma clang transform

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

[PATCH] D69088: [Lex] #pragma clang transform

Meinersbur added a comment. In D69088#1772141 , @rjmccall wrote: > This is a major new language feature, and code review is probably not the > right venue for reviewing it; there should be a thread on cfe-dev. My > apologies if that's already been

[PATCH] D69088: [Lex] #pragma clang transform

greened added a comment. In D69088#1772141 , @rjmccall wrote: > This is a major new language feature, and code review is probably not the > right venue for reviewing it; there should be a thread on cfe-dev. My > apologies if that's already been

[PATCH] D69088: [Lex] #pragma clang transform

rjmccall added a comment. This is a major new language feature, and code review is probably not the right venue for reviewing it; there should be a thread on cfe-dev. My apologies if that's already been discussed and I missed it. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST

[PATCH] D69088: [Lex] #pragma clang transform

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

[PATCH] D69088: [Lex] #pragma clang transform

Meinersbur updated this revision to Diff 230094. Meinersbur added a comment. - Address @ABataev's review Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D69088/new/ https://reviews.llvm.org/D69088 Files: clang/include/clang/Basic/LangOptions.def

[PATCH] D69088: [Lex] #pragma clang transform

ABataev added inline comments. Comment at: clang/lib/Parse/ParsePragma.cpp:3062 +// TODO: Handle nested pragmas as in r325369. +assert(!Tok.isAnnotation()); +assert(Tok.isNot(tok::annot_pragma_transform)); Add a message in this assert.

[PATCH] D69088: [Lex] #pragma clang transform

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

[PATCH] D69088: [Lex] #pragma clang transform

Meinersbur updated this revision to Diff 227561. Meinersbur added a comment. - Implement -f(no-)experimental-transform-pragma Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D69088/new/ https://reviews.llvm.org/D69088 Files:

[PATCH] D69088: [Lex] #pragma clang transform

Meinersbur updated this revision to Diff 227550. Meinersbur added a comment. - Use PRAGMA_ANNOTATION - Monorepo layout Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D69088/new/ https://reviews.llvm.org/D69088 Files:

[PATCH] D69088: [Lex] #pragma clang transform

hsaito added a comment. In D69088#1715210 , @Meinersbur wrote: > I'd rather just enable them with a command-line switch, such as > `-fexperimental-transform`. This direction works for me. `-fexperimental-transform-pragma` might be better, though.

[PATCH] D69088: [Lex] #pragma clang transform

Meinersbur added a comment. In D69088#1715038 , @hsaito wrote: > If there is a precedence, just follow that. Else, how to spell an > experimental clang pragma would be a good discussion topic by itself. If I > need to provide a discussion starter, I'd

[PATCH] D69088: [Lex] #pragma clang transform

hsaito added a comment. In D69088#1714575 , @Meinersbur wrote: > In D69088#1714020 , @hsaito wrote: > > > Personally, I like the intent. I don't foresee a clear (enough) path to get > > there. This leads to

[PATCH] D69088: [Lex] #pragma clang transform

Meinersbur added a comment. In D69088#1713623 , @hsaito wrote: > @Meinersbur, if I remember correctly, there was an RFC discussion on this > topic, right? If yes, would you post the pointer to that? I need a refresher > on what has been

[PATCH] D69088: [Lex] #pragma clang transform

Meinersbur added a comment. In D69088#1714020 , @hsaito wrote: > Personally, I like the intent. I don't foresee a clear (enough) path to get > there. This leads to hesitation of adding a new non-experimental pragma and > present it to programmers. If

[PATCH] D69088: [Lex] #pragma clang transform

hsaito added a comment. In D69088#1714019 , @Meinersbur wrote: > In D69088#1713933 , @hsaito wrote: > > > Have we established general consensus for the desire to have the flexible > > enough loop optimization pass

[PATCH] D69088: [Lex] #pragma clang transform

Meinersbur added a comment. In D69088#1713933 , @hsaito wrote: > Have we established general consensus for the desire to have the flexible > enough loop optimization pass ordering to accomplish the outcome of the new > directive, and shared vision for

[PATCH] D69088: [Lex] #pragma clang transform

Meinersbur added a comment. In D69088#1713915 , @ABataev wrote: > Just do not allow this form with respect_order clause. What exactly would be the rules what is allowed and what isn't? We can just not allow not mixing `#pragma clang loop` and `#pragma

[PATCH] D69088: [Lex] #pragma clang transform

hsaito added a comment. Have we established general consensus for the desire to have the flexible enough loop optimization pass ordering to accomplish the outcome of the new directive, and shared vision for the path to get there? If we are making this a general clang directive, I'd like to see

[PATCH] D69088: [Lex] #pragma clang transform

ABataev added a comment. In D69088#1713901 , @Meinersbur wrote: > In D69088#1713831 , @tyler.nowicki > wrote: > > > That approach would avoid the inevitable conflicts of having both loop and > > transform pragmas

[PATCH] D69088: [Lex] #pragma clang transform

Meinersbur added a comment. In D69088#1713831 , @tyler.nowicki wrote: > That approach would avoid the inevitable conflicts of having both loop and > transform pragmas on the same loop. I fear it will give us far worse ambiguities. Consider: #pragma

[PATCH] D69088: [Lex] #pragma clang transform

Meinersbur added a comment. In D69088#1713831 , @tyler.nowicki wrote: > In D69088#1713648 , @Meinersbur > wrote: > > > In D69088#1713623 , @hsaito wrote: > > > > >

[PATCH] D69088: [Lex] #pragma clang transform

tyler.nowicki added a comment. In D69088#1713648 , @Meinersbur wrote: > In D69088#1713623 , @hsaito wrote: > > > @Meinersbur, if I remember correctly, there was an RFC discussion on this > > topic, right? If yes,

Re: [PATCH] D69088: [Lex] #pragma clang transform

Sorry if this is answered in the patches but what happens if a loop has both #pragma clang loop and transform defined before it? I guess it probably shouldn't work. Perhaps instead you could create a new option to indicate that the order should be respected. #pragma clang loop respect_order <-

[PATCH] D69088: [Lex] #pragma clang transform

Meinersbur added a comment. In D69088#1713623 , @hsaito wrote: > @Meinersbur, if I remember correctly, there was an RFC discussion on this > topic, right? If yes, would you post the pointer to that? I need a refresher > on what has been

[PATCH] D69088: [Lex] #pragma clang transform

Meinersbur added a comment. In D69088#1713147 , @ABataev wrote: > Why not try to improve the existing #pragma clang loop rather than add a new > pragma with almost the same behavior? The behavior and syntax is different. #pragma clang loop ignores the

[PATCH] D69088: [Lex] #pragma clang transform

hsaito added a comment. @Meinersbur, if I remember correctly, there was an RFC discussion on this topic, right? If yes, would you post the pointer to that? I need a refresher on what has been discussed/settled in the past. Repository: rC Clang CHANGES SINCE LAST ACTION

[PATCH] D69088: [Lex] #pragma clang transform

ABataev added a comment. Why not try to improve the existing #pragma clang loop rather than add a new pragma with almost the same behavior? Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D69088/new/ https://reviews.llvm.org/D69088

[PATCH] D69088: [Lex] #pragma clang transform

Meinersbur created this revision. Meinersbur added reviewers: hfinkel, kbarton, SjoerdMeijer, aaron.ballman, ABataev, fhahn, hsaito, hans, greened, dmgreen, reames, Ayal, asavonic, rtrieu, dorit, rsmith, tyler.nowicki. Herald added a reviewer: bollu. Herald added a reviewer: jdoerfert. Herald