[PATCH] D50055: Update the coding standard about NFC changes and whitespace

Meinersbur added inline comments. Comment at: docs/CodingStandards.rst:514-516 +line of code. Some common editors will automatically remove trailing whitespace +when saving a file which causes unrelated changes to appear in diffs and +commits. JDevlieghere wrote:

[PATCH] D50214: Add inherited attributes before parsed attributes.

Meinersbur added inline comments. Comment at: lib/AST/DeclBase.cpp:854-859 + auto I = Attrs.begin(), E = Attrs.end(); + for (; I != E; ++I) { +if (!(*I)->isInherited()) + break; + } + Attrs.insert(I, A); aaron.ballman wrote: > The unfortunate part ab

[PATCH] D50805: [Sema] Don't warn on returning the address of a label

Meinersbur added a comment. In https://reviews.llvm.org/D50805#1201956, @Quuxplusone wrote: > If you added a new option `-Wret-addr-label` as suggested above (for a total > patch of +2 lines), then is it accurate to say: > > - if `-Wret-addr-label` was enabled by default, we know of at least one

[PATCH] D47267: [UnrollAndJam] Add unroll_and_jam pragma handling

Meinersbur added a comment. This is straightforward in that it clones the implementation of `#pragma unroll` for `unroll_and_jam`. Hence, it also shares the same problem of clang's LoopHints, namely that the order of loop transformations (if there are multiple on the same loop: loop distributi

[PATCH] D47267: [UnrollAndJam] Add unroll_and_jam pragma handling

Meinersbur added a comment. In https://reviews.llvm.org/D47267#1109912, @dmgreen wrote: > > Could we maybe disable the #pragma clang loop unroll_and_jam spelling ftm > > to avoid compatibility issues? > > Sure, I'm not against. It sounds like you have opinions on how this should > work. That's

[PATCH] D47267: [UnrollAndJam] Add unroll_and_jam pragma handling

Meinersbur added a comment. Yes, this is what I had in mind. Thank you. I am preparing an RFC on what I am trying to do. This should clarify our goals and to what extend `#pragma clang loop` conflicts with it. https://reviews.llvm.org/D47267 ___ c

[PATCH] D52117: Generate llvm.loop.parallel_accesses instead of llvm.mem.parallel_loop_access metadata.

Meinersbur created this revision. Meinersbur added reviewers: hfinkel, amusman, ABataev, tyler.nowicki. Meinersbur added a dependency: D52116: Introduce llvm.loop.parallel_accesses and llvm.access.group metadata.. Instead of generating llvm.mem.parallel_loop_access metadata, generate llvm.access

[PATCH] D52118: [Loopinfo] Remove one latch case in getLoopID. NFC.

Meinersbur created this revision. Meinersbur added reviewers: hfinkel, jdoerfert. getLoopID has different control flow for two cases: If there is a single loop latch and for any other number of loop latches (0 and more than one). The latter case should return the same result if there is a single

[PATCH] D48808: [CodeGen] Emit parallel_loop_access for each loop in the loop stack.

Meinersbur added a comment. ping Repository: rC Clang https://reviews.llvm.org/D48808 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D47267: [UnrollAndJam] Add unroll_and_jam pragma handling

Meinersbur accepted this revision. Meinersbur added a comment. This revision is now accepted and ready to land. LGTM Comment at: lib/Sema/SemaStmtAttr.cpp:183 {nullptr, nullptr}}; - for (const auto *I : Attrs) { [nit] unrelated whitespace

[PATCH] D48100: Append new attributes to the end of an AttributeList.

Meinersbur updated this revision to Diff 158610. Meinersbur added a comment. Rebase after de-listifying in r336945 Repository: rC Clang https://reviews.llvm.org/D48100 Files: include/clang/Sema/ParsedAttr.h lib/AST/ItaniumMangle.cpp lib/Parse/ParseDeclCXX.cpp lib/Parse/ParseObjc.cpp

[PATCH] D48100: Append new attributes to the end of an AttributeList.

Meinersbur reopened this revision. Meinersbur added a comment. This revision is now accepted and ready to land. Herald added a subscriber: jrtc27. Reopen after revert (and to be able to update the diff) Repository: rC Clang https://reviews.llvm.org/D48100 __

[PATCH] D48100: Append new attributes to the end of an AttributeList.

Meinersbur added a comment. I am unsure how to proceed. Commit since already accepted? Wait for reconfirmation? Open new differential? Repository: rC Clang https://reviews.llvm.org/D48100 ___ cfe-commits mailing list cfe-commits@lists.llvm.org h

[PATCH] D48100: Append new attributes to the end of an AttributeList.

Meinersbur marked an inline comment as done. Meinersbur added inline comments. Comment at: test/Sema/attr-ownership.c:22 void f15(int, int) - __attribute__((ownership_returns(foo, 1))) // expected-note {{declared with index 1 here}} - __attribute__((ownership_returns(foo, 2)

[PATCH] D48100: Append new attributes to the end of an AttributeList.

Meinersbur updated this revision to Diff 158639. Meinersbur marked an inline comment as done. Meinersbur added a comment. - Remove TODOs about the attribute order Repository: rC Clang https://reviews.llvm.org/D48100 Files: include/clang/Sema/ParsedAttr.h lib/AST/ItaniumMangle.cpp lib/P

[PATCH] D48100: Append new attributes to the end of an AttributeList.

This revision was automatically updated to reflect the committed changes. Closed by commit rL338800: Append new attributes to the end of an AttributeList. (authored by Meinersbur, committed by ). Herald added a subscriber: llvm-commits. Repository: rL LLVM https://reviews.llvm.org/D48100 File

[PATCH] D48100: Append new attributes to the end of an AttributeList.

This revision was automatically updated to reflect the committed changes. Closed by commit rC338800: Append new attributes to the end of an AttributeList. (authored by Meinersbur, committed by ). Changed prior to commit: https://reviews.llvm.org/D48100?vs=158639&id=158880#toc Repository: rC

[PATCH] D50214: Add inherited attributes before parsed attributes.

Meinersbur created this revision. Meinersbur added reviewers: hfinkel, aaron.ballman, nicholas, dlj, echristo, erichkeane. Currently, attributes from previous declarations ('inherited attributes') are added to the end of a declaration's list of attributes. Before r338800, the attribute list was

[PATCH] D50216: Pass IsInherited when merging attribute lists [RFC]

Meinersbur created this revision. Herald added subscribers: cfe-commits, eraman. Attempt to solve the the diagnostic marker order ('note: previous declaration here' pointing to a source location before the main error marker) from https://reviews.llvm.org/D48100. The approach is to pass a `IsInh

[PATCH] D48100: Append new attributes to the end of an AttributeList.

Meinersbur added a comment. I have two approaches to tackle the wrong marker order: https://reviews.llvm.org/D50215 and https://reviews.llvm.org/D50216. IMHO both are too invasive to be justified for the small issue. Repository: rL LLVM https://reviews.llvm.org/D48100 ___

[PATCH] D48808: [CodeGen] Emit parallel_loop_access for each loop in the loop stack.

This revision was automatically updated to reflect the committed changes. Closed by commit rC338810: [CodeGen] Emit parallel_loop_access for each loop in the loop stack. (authored by Meinersbur, committed by ). Changed prior to commit: https://reviews.llvm.org/D48808?vs=155288&id=158912#toc Re

[PATCH] D50214: Add inherited attributes before parsed attributes.

Meinersbur added a comment. For this patch the goal is to have the attributes in the AST in an order that is less surprising to consumers (including out-of-tree). If we change it now, new/revised code/diagnostics will be written to match this order. I agree that ideally, most attributes (except

[PATCH] D50055: Update the coding standard about NFC changes and whitespace

Meinersbur added inline comments. Comment at: docs/CodingStandards.rst:514-516 +line of code. Some common editors will automatically remove trailing whitespace +when saving a file which causes unrelated changes to appear in diffs and +commits. Just to note that t

[PATCH] D33774: [CodeGen] Make __attribute__(const) calls speculatable

Meinersbur added a comment. Definition of `__attibute__((const))` from https://gcc.gnu.org/onlinedocs/gcc/Common-Function-Attributes.html#Common-Function-Attributes > Many functions do not examine any values except their arguments, and have no > effects except the return value. Basically this i

[PATCH] D47267: [UnrollAndJam] Add unroll_and_jam pragma handling

Meinersbur added a comment. The RFC: https://lists.llvm.org/pipermail/cfe-dev/2018-May/058141.html https://reviews.llvm.org/D47267 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D47267: [UnrollAndJam] Add unroll_and_jam pragma handling

Meinersbur added a comment. In https://reviews.llvm.org/D47267#1122425, @dmgreen wrote: > I noticed in the paper that you used the name "unrollandjam", minus > underscores. Should I change this use that spelling here? I have no strong > opinion of one over the other (was just using what I had f

[PATCH] D47267: [UnrollAndJam] Add unroll_and_jam pragma handling

Meinersbur added a comment. In https://reviews.llvm.org/D47267#1123013, @dmgreen wrote: > I see your point about the mix of underscores. "nounroll_and_jam" also comes > from the Intel docs, and theres already "nounroll" vs "unroll". The "no" > becomes a qualifier on "unroll_and_jam". "no_unroll

[PATCH] D48100: Append new attributes to the end of an AttributeList.

Meinersbur created this revision. Meinersbur added reviewers: hfinkel, TylerNowicki, ABataev, thakis, rjmccall, george.burgess.iv, nicholas, nlewycky. Herald added subscribers: atanasyan, zzheng. ... instead of prepending it at the beginning (the original behavior since implemented in r122535 20

[PATCH] D48100: Append new attributes to the end of an AttributeList.

Meinersbur added inline comments. Comment at: lib/Sema/SemaOverload.cpp:6194 static SmallVector getOrderedEnableIfAttrs(const FunctionDecl *Function) { SmallVector Result; nicholas wrote: > This function shouldn't be necessary any more. All it's doing now is

[PATCH] D48100: Append new attributes to the end of an AttributeList.

Meinersbur updated this revision to Diff 151209. Meinersbur added a comment. - Remove obsolete comment - Refactor getOrderedEnableIfAttrs Repository: rC Clang https://reviews.llvm.org/D48100 Files: include/clang/Sema/AttributeList.h lib/AST/ItaniumMangle.cpp lib/Parse/ParseDecl.cpp l

[PATCH] D48100: Append new attributes to the end of an AttributeList.

Meinersbur marked 4 inline comments as done. Meinersbur added inline comments. Comment at: lib/AST/ItaniumMangle.cpp:710-711 Out << "Ua9enable_ifI"; // FIXME: specific_attr_iterator iterates in reverse order. Fix that and use // it here. +for (AttrVec::const_i

[PATCH] D48100: Append new attributes to the end of an AttributeList.

Meinersbur updated this revision to Diff 151378. Meinersbur marked an inline comment as done. Meinersbur added a comment. - Remove FIXME - Add comment about O(n^2) execution time when adding attributes - Do not store enable_if attributes in temporary list Repository: rC Clang https://reviews.

[PATCH] D48100: Append new attributes to the end of an AttributeList.

Meinersbur marked an inline comment as done. Meinersbur added a comment. In https://reviews.llvm.org/D48100#1132208, @aaron.ballman wrote: > Can you also simplify `hasSameOverloadableAttrs()` in ASTReaderDecl.cpp > similar to what you did in SemaOverload.cpp (the copy seems spurious)? Changed

[PATCH] D48100: Append new attributes to the end of an AttributeList.

This revision was automatically updated to reflect the committed changes. Closed by commit rC335084: Append new attributes to the end of an AttributeList. (authored by Meinersbur, committed by ). Changed prior to commit: https://reviews.llvm.org/D48100?vs=151378&id=151998#toc Repository: rC

[PATCH] D56928: Support attribute used in member funcs of class templates

Meinersbur added a comment. Thanks for looking into my almost 6 year old bug! Comment at: test/CodeGenCXX/attr-used-member-function-implicit-instantiation.cpp:1 +// RUN: %clang_cc1 -triple x86_64-unknown-unknown -emit-llvm -O0 -o - %s \ +// RUN: | FileCheck %s ---

[PATCH] D52117: Generate llvm.loop.parallel_accesses instead of llvm.mem.parallel_loop_access metadata.

Meinersbur updated this revision to Diff 177320. Meinersbur marked an inline comment as done. Meinersbur added a comment. - Allow multiple access groups per instructions, i.e. an instruction can be in multiple access groups. This allows a simple 'union' operation that occurs when inlining into a

[PATCH] D52117: Generate llvm.loop.parallel_accesses instead of llvm.mem.parallel_loop_access metadata.

Meinersbur updated this revision to Diff 177324. Meinersbur added a comment. - Fix wrong patch upload - Simplify access group emission ... .. possible due to the added possibility for instructions to belong to multiple access groups in D52116 . However, the numbe

[PATCH] D55468: Use zip_longest for list comparisons.

Meinersbur created this revision. Meinersbur added reviewers: dblaikie, hfinkel. Use zip_longest in two locations that compare iterator ranges. zip_longest allows the iteration using a range-based for-loop and to be symmetric over both ranges instead of prioritizing one over the other. In that l

[PATCH] D55468: Use zip_longest for iterator range comparisons. NFC.

Meinersbur updated this revision to Diff 177444. Meinersbur marked 4 inline comments as done. Meinersbur added a comment. - Address @dblaikie's review Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55468/new/ https://reviews.llvm.org/D55468 Files: lib/Sema/Sema

[PATCH] D55468: Use zip_longest for iterator range comparisons. NFC.

Meinersbur added inline comments. Comment at: lib/Sema/SemaOverload.cpp:8979 +// has fewer enable_if attributes than Cand2, and vice versa. +if (std::get<0>(Pair) == None) return Comparison::Worse; dblaikie wrote: > I'd probably write this as "if (

[PATCH] D55468: Use zip_longest for iterator range comparisons. NFC.

Meinersbur updated this revision to Diff 177447. Meinersbur added a comment. - Fix xor - Store tuple elements in variables Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55468/new/ https://reviews.llvm.org/D55468 Files: lib/Sema/SemaOverload.cpp lib/Serializa

[PATCH] D55468: Use zip_longest for iterator range comparisons. NFC.

Meinersbur marked 3 inline comments as done. Meinersbur added inline comments. Comment at: lib/Serialization/ASTReaderDecl.cpp:2922 +// Return false if the number of enable_if attributes is different. +if (std::get<0>(Pair).hasValue() != std::get<1>(Pair).hasValue()) +

[PATCH] D55468: Use zip_longest for iterator range comparisons. NFC.

This revision was automatically updated to reflect the committed changes. Meinersbur marked an inline comment as done. Closed by commit rC348762: Use zip_longest for iterator range comparisons. NFC. (authored by Meinersbur, committed by ). Changed prior to commit: https://reviews.llvm.org/D5546

[PATCH] D58638: [OpenMP 5.0] Parsing/sema support for from clause with mapper modifier

This revision was automatically updated to reflect the committed changes. Closed by commit rL354817: [OpenMP 5.0] Parsing/sema support for from clause with mapper modifier. (authored by Meinersbur, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Changed prio

[PATCH] D57978: [CodeGen] Generate follow-up metadata for loops with more than one transformation.

Meinersbur added a comment. ping The Polly-powered additional transformations now also generate this kind of metadata. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D57978/new/ https://reviews.llvm.org/D57978 ___

[PATCH] D57978: [CodeGen] Generate follow-up metadata for loops with more than one transformation.

Meinersbur added a comment. ping Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D57978/new/ https://reviews.llvm.org/D57978 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman

[PATCH] D57978: [CodeGen] Generate follow-up metadata for loops with more than one transformation.

Meinersbur updated this revision to Diff 192194. Meinersbur marked 5 inline comments as done. Meinersbur added a comment. - Rebase - Add two test cases for all attributes combined (as inner and outer loop for of an unroll-and-jam) - Of two nested unroll-and-jams, apply the inner first - Typos and

[PATCH] D57978: [CodeGen] Generate follow-up metadata for loops with more than one transformation.

Meinersbur added a comment. In D57978#1440894 , @dmgreen wrote: > This could maybe do with a few extra tests. Am I correct in saying something > like this: > > #pragma unroll_and_jam(4) > for(int j = 0; j < n; j++) { > #pragma unroll(4) > for(

[PATCH] D57978: [CodeGen] Generate follow-up metadata for loops with more than one transformation.

This revision was automatically updated to reflect the committed changes. Closed by commit rL357415: [CodeGen] Generate follow-up metadata for loops with more than one… (authored by Meinersbur, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Changed prior to

[PATCH] D57978: [CodeGen] Generate follow-up metadata for loops with more than one transformation.

Meinersbur added a comment. @dmgreen Thank you for the review! Repository: rL LLVM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D57978/new/ https://reviews.llvm.org/D57978 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://l

[PATCH] D60749: [Test] Remove obsolete test.

Meinersbur created this revision. Meinersbur added reviewers: hfinkel, aaron.ballman, tyler.nowicki, TylerNowicki. Meinersbur added a project: clang. The FIXME of this test case has been addressed in r335084/r338800. Its execution still does not succeed because of multiple syntax errors. First,

[PATCH] D60749: [Test] Remove obsolete test.

This revision was automatically updated to reflect the committed changes. Closed by commit rC358507: [Test] Remove obsolete test. (authored by Meinersbur, committed by ). Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D60749/new/ https://reviews.llvm.org/D60749 Fil

[PATCH] D52118: [Loopinfo] Remove one latch case in getLoopID. NFC.

Meinersbur added a comment. In https://reviews.llvm.org/D52118#1235897, @jdoerfert wrote: > > save an iteration over the loop's basic blocks (which is what getLoopLatch > > does) > > I'm not sure this is true. getLoopLatch() in LoopInfoImpl.h > only traverses the children of the header in the

[PATCH] D52118: [Loopinfo] Remove one latch case in getLoopID. NFC.

This revision was automatically updated to reflect the committed changes. Closed by commit rL342406: [Loopinfo] Remove one latch-case in getLoopID. NFC. (authored by Meinersbur, committed by ). Herald added a subscriber: llvm-commits. Changed prior to commit: https://reviews.llvm.org/D52118?vs=

[PATCH] D50214: Add inherited attributes before parsed attributes.

Meinersbur added a comment. ping Repository: rC Clang https://reviews.llvm.org/D50214 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D50214: Add inherited attributes before parsed attributes.

Meinersbur closed this revision. Meinersbur added a comment. Commited as https://reviews.llvm.org/rL342861. Repository: rC Clang https://reviews.llvm.org/D50214 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin

[PATCH] D52117: Generate llvm.loop.parallel_accesses instead of llvm.mem.parallel_loop_access metadata.

Meinersbur updated this revision to Diff 167099. Meinersbur added a comment. Herald added subscribers: llvm-commits, dexonsmith, steven_wu, eraman, mehdi_amini. - Rebase - Use call access group if instruction's access group is not set Repository: rL LLVM https://reviews.llvm.org/D52117 File

[PATCH] D52117: Generate llvm.loop.parallel_accesses instead of llvm.mem.parallel_loop_access metadata.

Meinersbur updated this revision to Diff 168096. Meinersbur added a comment. - Upload diff for clang portion (instead of https://reviews.llvm.org/D52116) Repository: rC Clang https://reviews.llvm.org/D52117 Files: lib/CodeGen/CGLoopInfo.cpp lib/CodeGen/CGLoopInfo.h test/CodeGenCXX/prag

[PATCH] D52117: Generate llvm.loop.parallel_accesses instead of llvm.mem.parallel_loop_access metadata.

Meinersbur updated this revision to Diff 168251. Meinersbur marked 2 inline comments as done. Meinersbur added a comment. - Address @pekka.jaaskelainen's review. Repository: rC Clang https://reviews.llvm.org/D52117 Files: lib/CodeGen/CGLoopInfo.cpp lib/CodeGen/CGLoopInfo.h test/CodeGen

[PATCH] D56326: [OpenMP 5.0] Parsing/sema support for "omp declare mapper" directive

Meinersbur added a comment. @lildmh Thanks for the patch. I can commit this patch for you. The tests run fine on my machine. However, for committing on other's behalf, we were asked to make the contributor aware that the contribution will be published under a new license

[PATCH] D56326: [OpenMP 5.0] Parsing/sema support for "omp declare mapper" directive

This revision was automatically updated to reflect the committed changes. Closed by commit rC352906: [OpenMP 5.0] Parsing/sema support for "omp declare mapper" directive. (authored by Meinersbur, committed by ). Herald added a project: clang. Repository: rC Clang CHANGES SINCE LAST ACTION ht

[PATCH] D57978: [CodeGen] Generate follow-up metadata for loops with more than one transformation.

Meinersbur created this revision. Meinersbur added reviewers: hfinkel, aaron.ballman, hsaito, dmgreen, anemet, rjmccall, Anastasia, pekka.jaaskelainen, meheff, tyler.nowicki. Herald added a subscriber: zzheng. Herald added a project: clang. Before this patch, CGLoop would dump all transformations

[PATCH] D57978: [CodeGen] Generate follow-up metadata for loops with more than one transformation.

Meinersbur marked an inline comment as done. Meinersbur added inline comments. Comment at: lib/CodeGen/CGLoopInfo.cpp:107 + FollowupLoopProperties.push_back( + MDNode::get(Ctx, MDString::get(Ctx, "llvm.loop.unroll.disable"))); + Anastasia wrote: > Will this

[PATCH] D58523: [OpenMP 5.0] Parsing/sema support for to clause with mapper modifier

This revision was automatically updated to reflect the committed changes. Closed by commit rL354698: [OpenMP 5.0] Parsing/sema support for to clause with mapper modifier. (authored by Meinersbur, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Changed prior

[PATCH] D61446: Generalize the pass registration mechanism used by Polly to any third-party tool

Meinersbur requested changes to this revision. Meinersbur added a comment. This revision now requires changes to proceed. It still fails with the same error. `LINK_POLLY_INTO_TOOLS` is only set after the polly subdirectory is processed. Because it is an `option`, the ON value will be stored in t

[PATCH] D61446: Generalize the pass registration mechanism used by Polly to any third-party tool

Meinersbur added a comment. This fails with Polly/Linux regression tests: FAIL: Polly :: Simplify/gemm.ll (1148 of 1149) TEST 'Polly :: Simplify/gemm.ll' FAILED Script: -- : 'RUN: at line 1'; opt -polly-process-unprofi

[PATCH] D61446: Generalize the pass registration mechanism used by Polly to any third-party tool

Meinersbur added a comment. Sorry, you might have tested it with `LLVM_LINK_POLLY_INTO_TOOLS=OFF` and/or `BUILD_SHARED_LIBS`/`LLVM_LINK_LLVM_DYLIV` where it might work, but unfortunately not with the default configuration using static component libraries. Repository: rG LLVM Github Monorepo

[PATCH] D61446: Generalize the pass registration mechanism used by Polly to any third-party tool

Meinersbur added a comment. Just an idea: We could avoid the explicit calls to 'initializeXYZPass' in opt/bugpoint/clang by adding Polly.cpp as a source file or object library to the executables. This would guarantee that its static initializer is called without dynamic library. Repository:

[PATCH] D61446: Generalize the pass registration mechanism used by Polly to any third-party tool

Meinersbur requested changes to this revision. Meinersbur added a comment. This revision now requires changes to proceed. In D61446#1533076 , @serge-sans-paille wrote: > That's what I tried to do when always adding Polly.cpp to PollyCore, but it > doesn'

[PATCH] D64744: Loop #pragma tail_predicate

Meinersbur added a comment. IMHO, this loops like an option of a particular transformation, not an independent pragma. E.g. #pragma clang loop vectorize(enable) vectorize_remainder(predicated) There could be multiple choices for how to execute remainder iterations, e.g. instead of an epilogu

[PATCH] D64744: #pragma clang loop predicate(enable|disable)

Meinersbur added a comment. [serious] Need to update docs. I'd expect `predicate` to be an option of another transformation. For vectorization, by convention the #pragma name would be `vectorize_predicate` and the metadata `llvm.loop.vectorize.predicate`. As a standalone, it is not clear what

[PATCH] D64744: #pragma clang loop vectorize_predicate(enable|disable)

Meinersbur added a comment. I prefer having the documentation change to be in the same patch as the functional change. Makes it easier to check whether they match. Comment at: clang/test/CodeGenCXX/pragma-loop.cpp:163 -// CHECK: ![[LOOP_1]] = distinct !{![[LOOP_1]], ![[UNROL

[PATCH] D61446: Generalize the pass registration mechanism used by Polly to any third-party tool

Meinersbur requested changes to this revision. Meinersbur added a comment. This revision now requires changes to proceed. Thank you for adding the Bye pass. It is really useful! Is there a specific reason to not modify the Hello pass? --- If I enable both passes statically (`LLVM_BYE_LINK_INTO_

[PATCH] D64564: Loop pragma parsing. NFC.

Meinersbur accepted this revision. Meinersbur added a comment. This revision is now accepted and ready to land. LGTM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D64564/new/ https://reviews.llvm.org/D64564 ___ cfe-commits mailing list cfe-c

[PATCH] D64564: Loop pragma parsing. NFC.

Meinersbur requested changes to this revision. Meinersbur added inline comments. This revision now requires changes to proceed. Comment at: lib/Parse/ParsePragma.cpp:1011 + Str = llvm::StringSwitch(Str) + .Case("loop", "clang loop " + Str.str()) + .Ca

[PATCH] D61446: Generalize the pass registration mechanism used by Polly to any third-party tool

Meinersbur added a comment. Did you resolve the conflicting `llvmGetPassPluginInfo` symbols for windows? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61446/new/ https://reviews.llvm.org/D61446 ___ cfe-commits mailing list cfe-commits@lists

[PATCH] D59474: [OpenMP 5.0] Codegen support for user-defined mappers

This revision was automatically updated to reflect the committed changes. Closed by commit rL367773: [OpenMP 5.0] Codegen support for user-defined mappers. (authored by Meinersbur, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Changed prior to commit: ht

[PATCH] D65776: [Clang] Pragma vectorize_predicate implies vectorize

Meinersbur added a comment. Mmmh, I would have expected this to work the same way as `vectorize_width`. According to the docs: > The following example implicitly enables vectorization and interleaving by > specifying a vector width and interleaving count: > `#pragma clang loop vectorize_width(

[PATCH] D65776: [Clang] Pragma vectorize_predicate implies vectorize

Meinersbur added a comment. Looking at the similar situation of `unroll(enable)`/`unroll_count(4)`, `unroll_count` also does not set `llvm.loop.unroll.enable`, but it is handled by the LoopUnroll pass itself: bool ExplicitUnroll = PragmaCount > 0 || PragmaFullUnroll ||

[PATCH] D64564: Loop pragma parsing. NFC.

Meinersbur added inline comments. Comment at: clang/lib/Parse/ParsePragma.cpp:1010 + StringRef Str = PragmaName.getIdentifierInfo()->getName(); + StringRef ClangLoopStr = "clang loop " + Str.str(); + Str = llvm::StringSwitch(Str) [serious] Use-after-free here

[PATCH] D66199: [docs] loop pragmas

Meinersbur added a comment. Since this is user documentation, we should only add it here once it is true. Comment at: docs/LanguageExtensions.rst:3068-3069 +There are loop hints that control transformations (e.g. vectorization, loop +unrolling) and there loop hints that set t

[PATCH] D66199: [docs] loop pragmas

Meinersbur added inline comments. Comment at: docs/LanguageExtensions.rst:3068-3069 +There are loop hints that control transformations (e.g. vectorization, loop +unrolling) and there loop hints that set transformation options (e.g. +``vectorize_width``, ``unroll_count``). Prag

[PATCH] D64564: Loop pragma parsing. NFC.

Meinersbur accepted this revision. Meinersbur added inline comments. This revision is now accepted and ready to land. Comment at: clang/lib/Parse/ParsePragma.cpp:1010 + StringRef Str = PragmaName.getIdentifierInfo()->getName(); + std::string ClangLoopStr = (Twine("clang loop ")

[PATCH] D61446: Generalize the pass registration mechanism used by Polly to any third-party tool

Meinersbur added inline comments. Comment at: llvm/cmake/modules/AddLLVM.cmake:808 +if(NOT llvm-pass-plugins) +# Target used to hold global properties referencable from generator-expression [serious] I get the following error: ``` CMake Error at cmake/modu

[PATCH] D61446: Generalize the pass registration mechanism used by Polly to any third-party tool

Meinersbur requested changes to this revision. Meinersbur added a comment. This revision now requires changes to proceed. Windows seems to work. Good job! Linux works with static libraries, but not with `BUILD_SHARED_LIBS=ON`: $ bin/opt : CommandLine Error: Option 'polly-dependences-computeo

[PATCH] D64564: Loop pragma parsing. NFC.

Meinersbur added inline comments. Comment at: clang/lib/Parse/ParsePragma.cpp:1009 static std::string PragmaLoopHintString(Token PragmaName, Token Option) { - std::string PragmaString; - if (PragmaName.getIdentifierInfo()->getName() == "loop") { -PragmaString = "clang loop

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

Meinersbur added a comment. +1 Such a solution also came up in https://bugs.llvm.org/show_bug.cgi?id=41514#c1 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61643/new/ https://reviews.llvm.org/D61643 _

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

Meinersbur added a comment. 1. Is there a diagnostic that would point to the `omp` token? As much as I like complete info (such as SourceLoc of semicolons), I cannot think of a use case for it. 2. I would like to see all of them all adjusted. There is an immediate improvement in that improves t

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

Meinersbur added a comment. In D61509#1512311 , @jdenny wrote: > 2. I too think it likely makes sense to adjust them all eventually. But do > people think it's important to write patches adjusting all pragmas before > pushing the adjustment for any of t

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

Meinersbur added a comment. In D61509#1512330 , @jdenny wrote: > In D61509#1512321 , @Meinersbur > wrote: > > > In D61509#1512311 , @jdenny wrote: > > > > > 2. I too think i

[PATCH] D49281: [Unroll/UnrollAndJam/Vectorizer/Distribute] Add followup loop attributes.

This revision was automatically updated to reflect the committed changes. Closed by commit rC348944: [Unroll/UnrollAndJam/Vectorizer/Distribute] Add followup loop attributes. (authored by Meinersbur, committed by ). Herald added a subscriber: cfe-commits. Changed prior to commit: https://review

[PATCH] D52117: Generate llvm.loop.parallel_accesses instead of llvm.mem.parallel_loop_access metadata.

Meinersbur updated this revision to Diff 178944. Meinersbur added a comment. - Fix typo Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D52117/new/ https://reviews.llvm.org/D52117 Files: lib/CodeGen/CGLoopInfo.cpp lib/CodeGen/CGLoopInfo.h test/CodeGenCXX/prag

[PATCH] D52117: Generate llvm.loop.parallel_accesses instead of llvm.mem.parallel_loop_access metadata.

This revision was automatically updated to reflect the committed changes. Closed by commit rC349823: [CodeGen] Generate llvm.loop.parallel_accesses instead of llvm.mem. (authored by Meinersbur, committed by ). Changed prior to commit: https://reviews.llvm.org/D52117?vs=178944&id=179141#toc Rep

[PATCH] D61446: Generalize the pass registration mechanism used by Polly to any third-party tool

Meinersbur requested changes to this revision. Meinersbur added inline comments. This revision now requires changes to proceed. Comment at: llvm/test/Feature/load_extension.ll:1 +; RUN: not test -f %llvmshlibdir/libBye%shlibext || opt %s -load=%llvmshlibdir/libBye%shlibext -good

[PATCH] D61446: Generalize the pass registration mechanism used by Polly to any third-party tool

Meinersbur added a comment. Keep in mind that for static linking you will need something that pulls-in a symbol from the pass static library. In this patch, `NewPMDriver.cpp` does it for `opt` by calling `get##Ext##PluginInfo()`. In clang, it is `BackendUtil.cpp`. But nothing for `bugpoint` hen

[PATCH] D67978: [OpenMP 5.0] Fix user-defined mapper lookup in sema

This revision was automatically updated to reflect the committed changes. Closed by commit rL373023: [OpenMP 5.0] Fix user-defined mapper lookup in sema (authored by Meinersbur, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Changed prior to commit: https

[PATCH] D61446: Generalize the pass registration mechanism used by Polly to any third-party tool

Meinersbur added inline comments. Comment at: polly/lib/Support/RegisterPasses.cpp:726 + +#ifndef LLVM_POLLY_LINK_INTO_TOOLS +extern "C" LLVM_ATTRIBUTE_WEAK ::llvm::PassPluginLibraryInfo Meinersbur wrote: > [serious] `LLVM_POLLY_LINK_INTO_TOOLS` is a cmake config

[PATCH] D48100: Append new attributes to the end of an AttributeList.

Meinersbur added inline comments. Comment at: test/Sema/attr-micromips.c:9 -__attribute__((micromips,mips16)) void foo5(); // expected-error {{'micromips' and 'mips16' attributes are not compatible}} \ +__attribute__((micromips,mips16)) void foo5(); // expected-error {{'mips

[PATCH] D48100: Append new attributes to the end of an AttributeList.

Meinersbur added a comment. In https://reviews.llvm.org/D48100#1142866, @erichkeane wrote: > I'm currently attempting to remove the AttributeList's linked-listness. Thank you. This should also resolve the non-optimal asymptotic execution time to append new attributes at the end of the list.

[PATCH] D48734: [Sema] Consider all format_arg attributes.

Meinersbur created this revision. Meinersbur added reviewers: dlj, rsmith, Jean-Daniel. If a function has multiple format_arg attributes, clang only considers the first it finds (because AttributeLists are in reverse order, it is textual last) and ignores all others. Loop over all FormatArgAttr

  1   2   3   4   5   >