[PATCH] D104601: [Preprocessor] Implement -fminimize-whitespace.

Meinersbur updated this revision to Diff 360977. Meinersbur added a comment. - Introduce types::isDerivedFromC - Print input type in error message Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D104601/new/ https://reviews.llvm.org/D104601 Files:

[PATCH] D104601: [Preprocessor] Implement -fminimize-whitespace.

Meinersbur added a comment. In D104601#2896091 , @aaron.ballman wrote: > I think there is, but I'm an eternal optimist. :-) This LGTM, but you should > wait a day or two before landing it in case @dblaikie or @rsmith have > concerns. Of course.

[PATCH] D104601: [Preprocessor] Implement -fminimize-whitespace.

Meinersbur updated this revision to Diff 360753. Meinersbur added a comment. - Revert test changes Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D104601/new/ https://reviews.llvm.org/D104601 Files: clang/docs/ClangCommandLineReference.rst

[PATCH] D104601: [Preprocessor] Implement -fminimize-whitespace.

Meinersbur added a comment. I compiled the Linux kernel (default config, excludes most drivers. Takes ~18mins to compile) and indeed found two problems: 1. Linux contains `assembler-with-cpp` files. When compiling with clang, it processes it with `-E` before passing the result to `as`. There

[PATCH] D104601: [Preprocessor] Implement -fminimize-whitespace.

Meinersbur updated this revision to Diff 360745. Meinersbur marked 2 inline comments as done. Meinersbur added a comment. - Error if -fminimize-whitespace is applied to asm files - Join lines with escaped newlines - Added some FIXMEs for grandfathered bugs Repository: rG LLVM Github Monorepo

[PATCH] D104601: [Preprocessor] Implement -fnormalize-whitespace.

Meinersbur added a comment. In D104601#2831746 , @dblaikie wrote: > but for my money this seems pretty problematic - will make quoted text in > compiler diagnostics weird/difficult to read, etc. FWIW, clang has a line length limit of 4096

[PATCH] D104601: [Preprocessor] Implement -fnormalize-whitespace.

Meinersbur updated this revision to Diff 360278. Meinersbur added a comment. - Rename -felide-unnecessary-whitespace to -fminimize-whitespace - Add -fminimize-whitespace only vlid to be used to docs - Adjust some code comments - Rebase Repository: rG LLVM Github Monorepo CHANGES SINCE LAST

[PATCH] D104601: [Preprocessor] Implement -fnormalize-whitespace.

Meinersbur updated this revision to Diff 359666. Meinersbur marked an inline comment as done. Meinersbur added a comment. - Rename -fnormalize-whitespace to -felide-unnecessary-whitespace - error when used without -E - Reabse Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D104601: [Preprocessor] Implement -fnormalize-whitespace.

Meinersbur marked an inline comment as done. Meinersbur added a comment. In D104601#2877855 , @aaron.ballman wrote: > In D104601#2848366 , @Meinersbur > wrote: > >> In D104601#2847400

[PATCH] D104601: [Preprocessor] Implement -fnormalize-whitespace.

Meinersbur added a comment. ping It would be nice if we could get this into clang 13 since the ccache part detects support for `-fnormalize-whitespace` when detecting clang 13+. Probing the compiler for whether it supports `-fnormalize-whitespace` on every run would be prohibitively

[PATCH] D104601: [Preprocessor] Implement -fnormalize-whitespace.

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

[PATCH] D104601: [Preprocessor] Implement -fnormalize-whitespace.

Meinersbur added a comment. In D104601#2847400 , @aaron.ballman wrote: > ... would add that it's very common for implementers to ask developers to run > their code through `-E` mode to submit preprocessed output in order to > reproduce a customer

[PATCH] D104601: [Preprocessor] Implement -fnormalize-whitespace.

Meinersbur updated this revision to Diff 355350. Meinersbur marked 6 inline comments as done. Meinersbur added a comment. - Address review by @aaron.ballman Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D104601/new/ https://reviews.llvm.org/D104601

[PATCH] D104601: [Preprocessor] Implement -fnormalize-whitespace.

Meinersbur added a comment. In D104601#2842686 , @dblaikie wrote: > One other thing: This wouldn't be usable when using debug info, presumably, > because it'd refer to the wrong lines, etc. This is considered in the ccache patch

[PATCH] D104601: [Preprocessor] Implement -fnormalize-whitespace.

Meinersbur added a comment. During a trial phase while compiling everything twice with ccache I got the following results. Only unify_mode:: $ ccache -d . -s cache directory . primary config ./ccache.conf secondary config (readonly)

[PATCH] D104601: [Preprocessor] Implement -fnormalize-whitespace.

Meinersbur added a comment. Some bikeshedding: Calls to `AvoidConcat` could be avoided by always inserting a space between tokens at the cost of making the output larger. In the current form, the flag could also be named `-fminimize-whitespace`. Repository: rG LLVM Github Monorepo CHANGES

[PATCH] D104601: [Preprocessor] Implement -fnormalize-whitespace.

Meinersbur added a comment. In D104601#2831746 , @dblaikie wrote: > This is probably more @aaron.ballman 's wheelhouse, but for my money this > seems pretty problematic - will make quoted text in compiler diagnostics > weird/difficult to read, etc. It

[PATCH] D104601: [Preprocessor] Implement -fnormalize-whitespace.

Meinersbur updated this revision to Diff 353220. Meinersbur edited the summary of this revision. Meinersbur added a comment. - Undo "typechange" changes Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D104601/new/ https://reviews.llvm.org/D104601

[PATCH] D104601: [Preprocessor] Implement -fnormalize-whitespace.

Meinersbur created this revision. Herald added subscribers: dang, jvesely. Meinersbur requested review of this revision. Herald added a reviewer: jdoerfert. Herald added subscribers: openmp-commits, libcxx-commits, cfe-commits, sstefan1. Herald added projects: clang, libc++, OpenMP. Herald added a

[PATCH] D95496: [clang][OpenMP] Augment ForStmt/CXXForRangeStmt for OpenMP canonical loops.

Meinersbur abandoned this revision. Meinersbur added a comment. Obsolete since D94973 was committed. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D95496/new/ https://reviews.llvm.org/D95496

[PATCH] D99459: [OpenMP] Implement '#pragma omp unroll'.

Meinersbur marked an inline comment as done. Meinersbur added inline comments. Comment at: clang/lib/CodeGen/CGStmtOpenMP.cpp:2600-2604 + RValue FactorRVal = EmitAnyExpr(FactorExpr, AggValueSlot::ignored(), + /*ignoreResult=*/true); +

[PATCH] D103666: [Clang][OpenMP] Refactor checking for mutually exclusive clauses. NFC.

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGc41a8fbfbb09: [Clang][OpenMP] Refactor checking for mutually exclusive clauses. NFC. (authored by Meinersbur). Repository: rG LLVM Github

[PATCH] D103665: [Clang][OpenMP] Add static version of getSingleClause. NFC.

This revision was automatically updated to reflect the committed changes. Closed by commit rGd466ca087aae: [Clang][OpenMP] Add static version of getSingleClauseClauseT. NFC. (authored by Meinersbur). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D99459: [OpenMP] Implement '#pragma omp unroll'.

Meinersbur marked 8 inline comments as done. Meinersbur added inline comments. Comment at: clang/include/clang/AST/StmtOpenMP.h:463-478 + static const SpecificClause *getSingleClause(ArrayRef Clauses) { +auto ClausesOfKind = getClausesOfKind(Clauses); -if

[PATCH] D103666: [Clang][OpenMP] Refactor checking for mutually exclusive clauses. NFC.

Meinersbur created this revision. Meinersbur added a reviewer: ABataev. Meinersbur added a project: OpenMP. Herald added subscribers: guansong, yaxunl. Meinersbur requested review of this revision. Herald added a reviewer: jdoerfert. Herald added a subscriber: sstefan1. Herald added a project:

[PATCH] D103665: [Clang][OpenMP] Add static version of getSingleClause NFC.

Meinersbur updated this revision to Diff 349762. Meinersbur edited the summary of this revision. Meinersbur added a comment. Add context Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D103665/new/ https://reviews.llvm.org/D103665 Files:

[PATCH] D103665: [Clang][OpenMP] Add static version of getSingleClause NFC.

Meinersbur created this revision. Meinersbur added a reviewer: ABataev. Meinersbur added a project: OpenMP. Herald added subscribers: guansong, yaxunl. Meinersbur requested review of this revision. Herald added a reviewer: jdoerfert. Herald added subscribers: cfe-commits, sstefan1. Herald added a

[PATCH] D102180: [Clang][OpenMP] Emit dependent PreInits before directive.

Meinersbur added a comment. Instead of reverting, I fixed the test in rG64e5a3bbdde2 . Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D102180/new/ https://reviews.llvm.org/D102180

[PATCH] D102180: [Clang][OpenMP] Emit dependent PreInits before directive.

This revision was automatically updated to reflect the committed changes. Closed by commit rG07a6beb40215: [Clang][OpenMP] Emit dependent PreInits before directive. (authored by Meinersbur). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D102180/new/

[PATCH] D102180: [Clang][OpenMP] Emit dependent PreInits before directive.

Meinersbur updated this revision to Diff 347508. Meinersbur added a comment. - Merge branch 'arcpatch-D102180' into HEAD - Use SmallVector Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D102180/new/ https://reviews.llvm.org/D102180 Files:

[PATCH] D102180: [Clang][OpenMP] Emit dependent PreInits before directive.

Meinersbur added inline comments. Comment at: clang/lib/Sema/SemaOpenMP.cpp:12579 Stmt *Body = nullptr; - SmallVector OriginalInits; + SmallVector>, 4> OriginalInits(1); if (!OMPLoopBasedDirective::doForAllLoops( ABataev wrote: > Why still

[PATCH] D102180: [Clang][OpenMP] Emit dependent PreInits before directive.

Meinersbur updated this revision to Diff 345562. Meinersbur marked 2 inline comments as done. Meinersbur added a comment. - Rebase - Address review Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D102180/new/ https://reviews.llvm.org/D102180 Files:

[PATCH] D101595: [Clang][OpenMP] Allow unified_shared_memory for Pascal-generation GPUs.

This revision was automatically updated to reflect the committed changes. Closed by commit rG83ff0ff46337: [Clang][OpenMP] Allow unified_shared_memory for Pascal-generation GPUs. (authored by Meinersbur). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D99459: [OpenMP] Implement '#pragma omp unroll'.

Meinersbur added a comment. Removed [WIP] flag; ready to review. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D99459/new/ https://reviews.llvm.org/D99459 ___ cfe-commits mailing list

[PATCH] D102180: [Clang][OpenMP] Emit dependent PreInits before directive.

Meinersbur updated this revision to Diff 345029. Meinersbur added a comment. - Preserve relative order between OriginalInits and interleaved transformation's PreInits Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D102180/new/

[PATCH] D102180: [Clang][OpenMP] Emit dependent PreInits before directive.

Meinersbur updated this revision to Diff 344145. Meinersbur added a comment. - Remove unused 'this' capture Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D102180/new/ https://reviews.llvm.org/D102180 Files: clang/include/clang/AST/StmtOpenMP.h

[PATCH] D102180: [Clang][OpenMP] Emit dependent PreInits before directive.

Meinersbur updated this revision to Diff 344134. Meinersbur added a comment. - Don't return false for void lambda Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D102180/new/ https://reviews.llvm.org/D102180 Files:

[PATCH] D102180: [Clang][OpenMP] Emit dependent PreInits before directive.

Meinersbur updated this revision to Diff 344119. Meinersbur added a comment. - Address review comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D102180/new/ https://reviews.llvm.org/D102180 Files: clang/include/clang/AST/StmtOpenMP.h

[PATCH] D102180: [Clang][OpenMP] Emit dependent PreInits before directive.

Meinersbur added inline comments. Comment at: clang/lib/AST/StmtOpenMP.cpp:128-129 +llvm::function_ref Callback, +llvm::function_ref +OnTransformationCallback) { CurStmt = CurStmt->IgnoreContainers(); ABataev wrote: > Do we need `bool` return

[PATCH] D102180: [Clang][OpenMP] Emit dependent PreInits before directive.

Meinersbur created this revision. Meinersbur added a reviewer: ABataev. Meinersbur added projects: OpenMP, clang. Herald added subscribers: guansong, yaxunl. Meinersbur requested review of this revision. Herald added a reviewer: jdoerfert. Herald added a subscriber: sstefan1. The PreInits of a

[PATCH] D101503: [OpenMPIRBuilder] Add createOffloadMaptypes and createOffloadMapnames functions

Meinersbur accepted this revision. Meinersbur added a comment. This revision is now accepted and ready to land. LGTM, thank you. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D101503/new/ https://reviews.llvm.org/D101503

[PATCH] D101595: [Clang][OpenMP] Allow unified_shared_memory for Pascal-generation GPUs.

Meinersbur updated this revision to Diff 342190. Meinersbur edited the summary of this revision. Meinersbur added a comment. This revision is now accepted and ready to land. - Fix test case Will wait for D101498 is resolved, otherwise I cannot test this

[PATCH] D101595: [Clang][OpenMP] Allow unified_shared_memory for Pascal-generation GPUs.

Meinersbur planned changes to this revision. Meinersbur added a comment. Will fix the failed `required_codegen.cpp` test. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D101595/new/ https://reviews.llvm.org/D101595

[PATCH] D101503: [OpenMPIRBuilder] Add createOffloadMaptypes and createOffloadMapnames functions

Meinersbur added a comment. In D101503#2729385 , @jdoerfert wrote: > I assume clang tests apply here. It replaces clang functionality after all. The policy so far was to have LLVM functionality tested in LLVM itself. See

[PATCH] D101503: [OpenMPIRBuilder] Add createOffloadMaptypes and createOffloadMapnames functions

Meinersbur added a comment. testcase That is, something that calls createOffloadMaptypes/createOffloadMapnames independently of any front-end. Can be as simply as calling it and checking

[PATCH] D101595: [Clang][OpenMP] Allow unified_shared_memory for Pascal-generation GPUs.

Meinersbur created this revision. Meinersbur added reviewers: ye-luo, JonChesterfield, ABataev, patricklyster, kkwli0. Meinersbur added projects: OpenMP, clang. Herald added subscribers: guansong, yaxunl. Meinersbur requested review of this revision. Herald added a reviewer: jdoerfert. Herald

[PATCH] D99484: Use `GNUInstallDirs` to support custom installation dirs.

Meinersbur added a comment. In D99484#2724435 , @Ericson2314 wrote: > I effectively replaced `${CMAKE_INSTALL_PREFIX}/lib` with > `${CMAKE_INSTALL_LIBDIR}`, which evaluates (by default) merely to `lib`. The > fix was `${CMAKE_INSTALL_FULL_LIBDIR}` which

[PATCH] D99484: Use `GNUInstallDirs` to support custom installation dirs.

Meinersbur added a comment. In D99484#2724055 , @Ericson2314 wrote: > Fix bug makeing polly path full, and rebase fixing conflicts What was the problem? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D89974: [driver][CUDA] Use CMake's FindCUDA as default --cuda-path.

Meinersbur abandoned this revision. Meinersbur added a comment. Abandoned in favor of D101266 . Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D89974/new/ https://reviews.llvm.org/D89974

[PATCH] D91054: [Clang][OpenMP] Frontend work for sections - D89671

Meinersbur added a comment. In D91054#2709712 , @jdoerfert wrote: > OK, we really need to provide the PrivCB impl so we can start removing clang > code. > This is generally fine with me, @fghanim @Meinersbur any concerns? It might be OK to start with

[PATCH] D91054: [Clang][OpenMP] Frontend work for sections - D89671

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

[PATCH] D91054: [Clang][OpenMP] Frontend work for sections - D89671

Meinersbur added a comment. What is the status of this? The parent D89671 has been accepted, but not committed. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D91054/new/ https://reviews.llvm.org/D91054

[PATCH] D95976: [OpenMP] Simplify offloading parallel call codegen

Meinersbur accepted this revision. Meinersbur added a comment. This revision is now accepted and ready to land. This test seem to pass on Windows now. Please still fix the clang-format remarks, such as going over 80 characters on a line. Comment at:

[PATCH] D95976: [OpenMP] Simplify offloading parallel call codegen

Meinersbur added inline comments. Comment at: clang/lib/CodeGen/CGOpenMPRuntimeGPU.cpp:591-592 return Bld.CreateAnd(Bld.CreateNUWSub(NumThreads, Bld.getInt32(1)), Bld.CreateNot(Mask), "master_tid"); } This is another undefined

[PATCH] D100696: Fixed typos

Meinersbur added inline comments. Comment at: polly/docs/ReleaseNotes.rst:9 - These release notes are for the next release of Polly and it describes + These release notes are for the next release of Polly and describe the new features that have recently been committed to

[PATCH] D95976: [OpenMP] Simplify offloading parallel call codegen

Meinersbur added inline comments. Comment at: clang/lib/CodeGen/CGOpenMPRuntimeGPU.cpp:1141 llvm::Value *IsWorker = Bld.CreateICmpULT(RT.getGPUThreadID(CGF), getThreadLimit(CGF)); Bld.CreateCondBr(IsWorker, WorkerBB, MasterCheckBB); There seem to be

[PATCH] D95976: [OpenMP] Simplify offloading parallel call codegen

Meinersbur added a comment. I have not looked at the other mentioned problem yet: > another that attribute regexes are not recognized > (https://reviews.llvm.org/harbormaster/unit/view/552593/ at > nvptx_target_codegen.cpp:723:17) Which might still be there. I would want for Harbormaster to

[PATCH] D95976: [OpenMP] Simplify offloading parallel call codegen

Meinersbur requested changes to this revision. Meinersbur added inline comments. This revision now requires changes to proceed. Comment at: clang/lib/CodeGen/CGOpenMPRuntimeGPU.cpp:569-570 + else +ThreadLimit = Bld.CreateNUWSub(RT.getGPUNumThreads(CGF), +

[PATCH] D95976: [OpenMP] Simplify offloading parallel call codegen

Meinersbur added a comment. The transposition problem arises from: Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D95976/new/ https://reviews.llvm.org/D95976 ___ cfe-commits mailing list

[PATCH] D99484: Use `GNUInstallDirs` to support custom installation dirs.

Meinersbur added a comment. In D99484#2668225 , @Ericson2314 wrote: > Additionally, I have cleaned up the `polly` code so that I need not change it > so much, and it should still work with absolute or relative paths quite > flexibly. Looks fine to me.

[PATCH] D99484: Use `GNUInstallDirs` to support custom installation dirs.

Meinersbur added inline comments. Comment at: polly/cmake/CMakeLists.txt:82-96 +set(POLLY_INSTALL_PREFIX "") set(POLLY_CONFIG_LLVM_CMAKE_DIR "${LLVM_BINARY_DIR}/${LLVM_INSTALL_PACKAGE_DIR}") -set(POLLY_CONFIG_CMAKE_DIR "${POLLY_INSTALL_PREFIX}/${POLLY_INSTALL_PACKAGE_DIR}")

[PATCH] D99459: [OpenMP] Implement '#pragma omp unroll'. WIP.

Meinersbur created this revision. Meinersbur added projects: OpenMP, clang. Herald added subscribers: dexonsmith, martong, arphaman, zzheng, guansong, yaxunl. Meinersbur requested review of this revision. Herald added a reviewer: jdoerfert. Herald added subscribers: llvm-commits, cfe-commits,

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

Meinersbur added a comment. Is is that same problem that D98265 addresses? 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.

Meinersbur added a comment. In D94973#2591493 , @jdenny wrote: >> I even think that representing semantic information alongside of syntax is >> one of the principles of Clang's AST. > > Interesting. Is that documented somewhere?

[PATCH] D97819: [CMake] Enable Polly for Fuchsia toolchain build

Meinersbur added a comment. As a contributor to Polly, this makes me happy to see ;-) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D97819/new/ https://reviews.llvm.org/D97819 ___ cfe-commits mailing

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

Meinersbur added a comment. In D94973#2590867 , @jdenny wrote: > One property of this patch that has bothered me is that OMPCanonicalLoop is > not a loop. Instead, it's an AST node that is sandwiched between a directive > and a loop to contain extra

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

Meinersbur added inline comments. Comment at: clang/lib/Sema/TreeTransform.h:8321 +TreeTransform::TransformOMPCanonicalLoop(OMPCanonicalLoop *L) { + // The OMPCanonicalLoop will be recreated when transforming the loop-associted + // directive. jdenny wrote: >

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

Meinersbur added inline comments. Comment at: clang/lib/Sema/TreeTransform.h:8321 +TreeTransform::TransformOMPCanonicalLoop(OMPCanonicalLoop *L) { + // The OMPCanonicalLoop will be recreated when transforming the loop-associted + // directive. jdenny wrote: >

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

Meinersbur marked an inline comment as done. Meinersbur added a comment. In D94973#2575395 , @jdenny wrote: > This patch has no effect if the OpenMP IR builder is not enabled, and it's > disabled by default. Is that right? Yes, this is how it is

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

Meinersbur added inline comments. Comment at: clang/lib/CodeGen/CodeGenFunction.h:292 + /// nest would extend. + SmallVector OMPLoopNestStack; + jdenny wrote: > Unless I missed something, the only accesses to `OMPLoopNestStack` are > `push_back`, `clear`,

[PATCH] D76342: [OpenMP] Implement '#pragma omp tile'

Meinersbur added a comment. In D76342#2565686 , @ABataev wrote: > I can commit it as soon as you accepted it if you don't mind of course. I do not mind Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

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

Meinersbur added inline comments. Comment at: clang/lib/Sema/SemaOpenMP.cpp:5215 + Expr *NegIncAmount = + AssertSuccess(Actions.BuildUnaryOp(nullptr, {}, UO_Minus, NewStep)); + Expr *BackwardDist = AssertSuccess( Meinersbur wrote: > jdenny

[PATCH] D76342: [OpenMP] Implement '#pragma omp tile'

Meinersbur accepted this revision. Meinersbur added a comment. This revision is now accepted and ready to land. Who is going to commit? Comment at: clang/include/clang/AST/StmtOpenMP.h:651-677 + static bool + doForAllLoops(Stmt *CurStmt, bool TryImperfectlyNestedLoops, +

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

Meinersbur marked 6 inline comments as done. Meinersbur added a subscriber: rsmith. Meinersbur added inline comments. Comment at: clang/lib/CodeGen/CodeGenFunction.cpp:94 // been "emitted" to the outside, thus, modifications are still sensible. - if

[PATCH] D94779: [Clang] Ensure vector predication loop metadata is always emitted when pragma is specified.

Meinersbur added a comment. I just pushed the patch with an slight addition to the summary motivitating why always emitting the predicate metadata: https://github.com/llvm/llvm-project/commit/74ddacd30de54e19b28218bc8563bd0f34f32cad Repository: rG LLVM Github Monorepo CHANGES SINCE LAST

[PATCH] D94779: [Clang] Ensure vector predication loop metadata is always emitted when pragma is specified.

This revision was automatically updated to reflect the committed changes. Closed by commit rG74ddacd30de5: [Clang] Ensure vector predication loop metadata is always emitted when pragma… (authored by malharJ, committed by Meinersbur). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST

[PATCH] D94779: [Clang] Ensure vector predication loop metadata is always emitted when pragma is specified.

Meinersbur accepted this revision. Meinersbur added a comment. Thank you for the clarification. It would be great if that could be communicated in the summary/commit message. It's sounds like emitting `llvm.loop.vectorize.predicate.enable` when vectorization is disabled is the motivation

[PATCH] D94779: [Clang] Ensure vector predication loop metadata is always emitted when pragma is specified.

Meinersbur added a comment. To illustrate how complicated interpreting the vectorize pragma has become in the mid-end: TransformationMode llvm::hasVectorizeTransformation(Loop *L) { Optional Enable = getOptionalBoolLoopAttribute(L, "llvm.loop.vectorize.enable"); if (Enable

[PATCH] D94779: [Clang] Ensure vector predication loop metadata is always emitted when pragma is specified.

Meinersbur added a comment. In D94779#2556274 , @dmgreen wrote: > This makes sense to me. If the user specifies the pragma then we send that > through to the vectorizer, whatever it is. As much as `vectorize_width(1) > interleave_count(4)

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

Meinersbur added inline comments. Comment at: clang/include/clang/AST/StmtOpenMP.h:97 +/// `std::vector::iterator::difference_type` aka `ptrdiff_t`). +/// Therefore, the distance function will be +/// [&](size_t ) { Result = __end - __begin; } jdenny wrote: >

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

Meinersbur added inline comments. Comment at: clang/include/clang/AST/StmtOpenMP.h:41 +/// purposes: +/// * Loop variable: The user-accessible variable with different value for each +/// iteration. jdenny wrote: > I suggest the terms "loop user variable" and

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

Meinersbur added inline comments. Comment at: clang/lib/AST/Stmt.cpp:1275 -->isScalarType())) && -"captures by copy are expected to have a scalar type!"); break; jdoerfert wrote: > Why does this have

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

Meinersbur added inline comments. Comment at: clang/lib/Sema/SemaExpr.cpp:17274 + if (IsTopScope && Kind != Sema::TryCapture_Implicit) { +ByRef = (Kind == Sema::TryCapture_ExplicitByRef); + } else if (S.getLangOpts().OpenMP && RSI->CapRegionKind == CR_OpenMP) {

[PATCH] D76342: [OpenMP] Implement '#pragma omp tile'

Meinersbur added inline comments. Comment at: clang/include/clang/AST/StmtOpenMP.h:448 + /// Number of collapsed loops as specified by 'collapse' clause. + unsigned CollapsedNum = 0; + ABataev wrote: > Meinersbur wrote: > > Inaccurate for the tile directive. >

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

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

[PATCH] D76342: [OpenMP] Implement '#pragma omp tile'

Meinersbur added a comment. Thank you for the update. From comparing the diffs, here is the list of main changes I found: 1. Create `OMPLoopBasedDirective` as a new base class for OMPLoopDirective (and OMPTileDirective) for CollapsedNum and some utility methods for analyzing the

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

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

[PATCH] D95496: [clang][OpenMP] Augment ForStmt/CXXForRangeStmt for OpenMP canonicial loop.

Meinersbur created this revision. Meinersbur added reviewers: jdoerfert, AMDChirag, anchu-rajendran, kiranchandramohan, SouraVX, ftynse, kiranktp, fghanim, ABataev, craig.topper, dblaikie, rsmith, aaron.ballman. Herald added subscribers: dexonsmith, martong, guansong, hiraditya, yaxunl. Herald

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

Meinersbur created this revision. Meinersbur added reviewers: jdoerfert, AMDChirag, anchu-rajendran, kiranchandramohan, SouraVX, ftynse, kiranktp, fghanim, ABataev. Meinersbur added projects: OpenMP, clang. Herald added subscribers: dexonsmith, rriddle, arphaman, guansong, hiraditya, yaxunl.

[PATCH] D90159: [DDG] Data Dependence Graph - DOT printer

Meinersbur added a comment. Can I help fixing the Windows build problem? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D90159/new/ https://reviews.llvm.org/D90159 ___ cfe-commits mailing list

[PATCH] D90159: [DDG] Data Dependence Graph - DOT printer

Meinersbur accepted this revision. Meinersbur added a comment. Thanks. Still LGTM. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D90159/new/ https://reviews.llvm.org/D90159 ___ cfe-commits mailing list

[PATCH] D90159: [DDG] Data Dependence Graph - DOT printer

Meinersbur accepted this revision. Meinersbur added a comment. This revision is now accepted and ready to land. Thanks, LGTM. Comment at: llvm/include/llvm/Analysis/DDG.h:489 + }, + [&] { OS << ", "; }); + return OS.str(); `llvm::interleaveComma`

[PATCH] D89158: [NewPM] Provide method to run all pipeline callbacks, used for -O0

Meinersbur accepted this revision. Meinersbur added a comment. This revision is now accepted and ready to land. In D89158#2389520 , @aeubanks wrote: > Thanks, I wasn't aware of this. Looks like other `opt -passes='default'` > tests also look at if it

[PATCH] D89158: [NewPM] Provide method to run all pipeline callbacks, used for -O0

Meinersbur requested changes to this revision. Meinersbur added a comment. This revision now requires changes to proceed. Could you try configuring LLVM with `-DLLVM_BYE_LINK_INTO_TOOLS=ON`? I assume the test will break because it unconditionally adds a FunctionPass. Repository: rG LLVM

[PATCH] D91109: [OMPIRBuilder] Start 'Create' methods with lower case. NFC.

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGe5dba2d7e5a6: [OMPIRBuilder] Start Create methods with lower case. NFC. (authored by Meinersbur). Repository: rG LLVM Github Monorepo CHANGES

[PATCH] D91109: [OMPIRBuilder] Start 'Create' methods with lower case. NFC.

Meinersbur created this revision. Meinersbur added a reviewer: jdoerfert. Meinersbur added a project: OpenMP. Herald added subscribers: cfe-commits, rdzhabarov, tatianashp, msifontes, jurahul, Kayjukh, grosul1, Joonsoo, liufengdb, aartbik, lucyrfox, mgester, arpith-jacob, antiagainst, shauheen,

[PATCH] D91109: [OMPIRBuilder] Start 'Create' methods with lower case. NFC.

Meinersbur updated this revision to Diff 304013. Meinersbur added a comment. Last update pushed only the change Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D91109/new/ https://reviews.llvm.org/D91109 Files:

[PATCH] D89158: [NewPM] Provide method to run all pipeline callbacks, used for -O0

Meinersbur added a comment. My point is that it is possible for a pass plugin (such as `llvm/examples/Bye`) that can add themselves to the -O0 pipeline. If those plugins are linked statically, it will break tests that assume that no passes are added at those extension points. I don't quite

[PATCH] D89158: [NewPM] Provide method to run all pipeline callbacks, used for -O0

Meinersbur added a comment. In D89158#2384156 , @aeubanks wrote: > Are there passes that should be running at -O0 that aren't target specific? > Otherwise the callbacks should only add passes if the optimization level > isn't -O0. At least for the

[PATCH] D91109: [OMPIRBuilder] Start 'Create' methods with lower case. NFC.

Meinersbur updated this revision to Diff 304011. Meinersbur added a comment. Missed reference of Create in comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D91109/new/ https://reviews.llvm.org/D91109 Files:

<    1   2   3   4   5   >