[PATCH] D70739: [OPENMP50]Add device/isa context selector support.

ABataev marked an inline comment as done. ABataev added inline comments. Comment at: clang/include/clang/Sema/Sema.h:9320 using OMPCtxSelectorData = - OpenMPCtxSelectorData, ExprResult>; + OpenMPCtxSelectorData, ExprResult>; jdoerfert wrote: > ABat

[PATCH] D70739: [OPENMP50]Add device/isa context selector support.

ABataev marked an inline comment as done. ABataev added inline comments. Comment at: clang/include/clang/Sema/Sema.h:9320 using OMPCtxSelectorData = - OpenMPCtxSelectorData, ExprResult>; + OpenMPCtxSelectorData, ExprResult>; jdoerfert wrote: > ABat

[PATCH] D70804: [Frontend] Allow OpenMP offloading to aarch64

ABataev requested changes to this revision. ABataev added a comment. This revision now requires changes to proceed. Tests are required Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D70804/new/ https://reviews.llvm.org/D70804 _

[PATCH] D70726: [OpenMP50] Add parallel master construct

ABataev added inline comments. Comment at: clang/include/clang/AST/StmtOpenMP.h:1879 + Create(const ASTContext &C, SourceLocation StartLoc, SourceLocation EndLoc, + ArrayRef Clauses, Stmt *AssociatedStmt, bool HasCancel); + `HasCancel` param? =

[PATCH] D70973: [OPENMP50]Treat context selectors as expressions, not just strings.

ABataev created this revision. ABataev added a reviewer: jdoerfert. Herald added a subscriber: guansong. Herald added a project: clang. After some discussion, it was decided to allow not only identifiers as context selectors, but also string literals. Repository: rG LLVM Github Monorepo https

[PATCH] D70726: [OpenMP50] Add parallel master construct

ABataev added inline comments. Comment at: clang/test/OpenMP/parallel_master_codegen.cpp:1 +// expected-no-diagnostics +#ifndef HEADER A codegen test for `if`, `proc_bind` and `allocate` clauses? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D70973: [OPENMP50]Treat context selectors as expressions, not just strings.

ABataev marked 2 inline comments as done. ABataev added inline comments. Comment at: clang/include/clang/Basic/DiagnosticParseKinds.td:1230 - "unknown '%0' device kind trait in the 'device' context selector set, expected" - " one of 'host', 'nohost', 'cpu', 'gpu' or 'fpga'">;

[PATCH] D70726: [OpenMP50] Add parallel master construct

ABataev added inline comments. Comment at: clang/include/clang/AST/StmtOpenMP.h:1875 + /// \param AssociatedStmt Statement, associated with the directive. + /// \param HasCancel true if current directive has inner cancel directive. + /// No such param. =

[PATCH] D70726: [OpenMP50] Add parallel master construct

ABataev accepted this revision. ABataev added a comment. This revision is now accepted and ready to land. LG Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D70726/new/ https://reviews.llvm.org/D70726 ___

[PATCH] D70973: [OPENMP50]Treat context selectors as expressions, not just strings.

ABataev updated this revision to Diff 232184. ABataev added a comment. Restored message + some small improvements. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D70973/new/ https://reviews.llvm.org/D70973 Files: clang/include/clang/Basic/Attr.td

[PATCH] D70726: [OpenMP50] Add parallel master construct

ABataev reopened this revision. ABataev added a comment. This revision is now accepted and ready to land. Need to fix tests on Windows Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D70726/new/ https://reviews.llvm.org/D70726 _

[PATCH] D70726: [OpenMP50] Add parallel master construct

ABataev added inline comments. Comment at: clang/test/OpenMP/parallel_master_codegen.cpp:144 +// CK4: [[A_VAL:%.+]] = alloca i32 +// CK4: [[A_CASTED:%.+]] = alloca i{{32|64}} +// CK4: [[ZERO:%.+]] = load i32, i32* [[A_VAL]] I think it would be b

[PATCH] D70726: [OpenMP50] Add parallel master construct

ABataev added inline comments. Comment at: clang/test/OpenMP/parallel_master_codegen.cpp:67 + +// CK2: define internal {{.*}}void [[OMP_OUTLINED]](i32* noalias %.global_tid., i32* noalias %.bound_tid.) +// CK2-NOT: __kmpc_global_thread_num Not the best i

[PATCH] D70726: [OpenMP50] Add parallel master construct

ABataev accepted this revision. ABataev added a comment. This revision is now accepted and ready to land. LG Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D70726/new/ https://reviews.llvm.org/D70726 ___

[PATCH] D70804: [Frontend] Allow OpenMP offloading to aarch64

ABataev added inline comments. Comment at: clang/test/OpenMP/target_messages.cpp:4 + +// RUN: not %clang_cc1 -fopenmp -std=c++11 -fopenmp-targets=aaa-bbb-ccc-ddd -o - %s 2>&1 | FileCheck %s // CHECK: error: OpenMP target is invalid: 'aaa-bbb-ccc-ddd' Why do you

[PATCH] D70804: [Frontend] Allow OpenMP offloading to aarch64

ABataev added inline comments. Comment at: clang/test/OpenMP/openmp_offload_registration.cpp:3-5 +// RUN: %clang_cc1 -verify -fopenmp -x c -triple x86_64-unknown-linux-gnu -fopenmp-targets=aarch64-unknown-linux-gnu,powerpc64-ibm-linux-gnu -emit-llvm %s -o - | FileCheck %s +// R

[PATCH] D70973: [OPENMP50]Treat context selectors as expressions, not just strings.

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

[PATCH] D71134: [OpenMP] Require trivially copyable type for mapping

ABataev added inline comments. Comment at: clang/lib/Sema/SemaOpenMP.cpp:14917 if (FullCheck && !SemaRef.CurContext->isDependentContext() && - !QTy.isTrivialType(SemaRef.Context)) + !QTy.isTriviallyCopyableType(SemaRef.Context)) SemaRef.Diag(SL, diag::warn_omp_n

[PATCH] D71134: [OpenMP] Require trivially copyable type for mapping

ABataev accepted this revision. ABataev added a comment. This revision is now accepted and ready to land. LG Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71134/new/ https://reviews.llvm.org/D71134 ___

[PATCH] D70973: [OPENMP50]Treat context selectors as expressions, not just strings.

ABataev marked an inline comment as done. ABataev added inline comments. Comment at: clang/include/clang/Basic/DiagnosticParseKinds.td:1230 - "unknown '%0' device kind trait in the 'device' context selector set, expected" - " one of 'host', 'nohost', 'cpu', 'gpu' or 'fpga'">;

[PATCH] D70804: [Frontend] Allow OpenMP offloading to aarch64

ABataev accepted this revision. ABataev added a comment. This revision is now accepted and ready to land. LG Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D70804/new/ https://reviews.llvm.org/D70804 ___

[PATCH] D70973: [OPENMP50]Treat context selectors as expressions, not just strings.

ABataev marked an inline comment as done. ABataev added inline comments. Comment at: clang/include/clang/Basic/DiagnosticParseKinds.td:1230 - "unknown '%0' device kind trait in the 'device' context selector set, expected" - " one of 'host', 'nohost', 'cpu', 'gpu' or 'fpga'">;

[PATCH] D70973: [OPENMP50]Treat context selectors as expressions, not just strings.

ABataev marked an inline comment as done. ABataev added inline comments. Comment at: clang/include/clang/Basic/DiagnosticParseKinds.td:1230 - "unknown '%0' device kind trait in the 'device' context selector set, expected" - " one of 'host', 'nohost', 'cpu', 'gpu' or 'fpga'">;

[PATCH] D71179: [OpenMP][WIP] Initial support for `begin/end declare variant`

ABataev added a comment. I read the spec and don't think that we need all this complex stuff for the implementation. Yiu need judt to check at the codegen phase if the function must be emitted or not. We don't even need to move context checksnfrom codegen, because with the current semantics all

[PATCH] D71179: [OpenMP][WIP] Initial support for `begin/end declare variant`

ABataev added a comment. In D71179#1774448 , @jdoerfert wrote: > In D71179#177 , @ABataev wrote: > > > I read the spec and don't think that we need all this complex stuff for the > > implementation. Yiu need ju

[PATCH] D71179: [OpenMP][WIP] Initial support for `begin/end declare variant`

ABataev added a comment. In D71179#1774470 , @jdoerfert wrote: > In D71179#1774469 , @ABataev wrote: > > > In D71179#1774448 , @jdoerfert > > wrote: > > > > > In D71179#1774

[PATCH] D71179: [OpenMP][WIP] Initial support for `begin/end declare variant`

ABataev added a comment. In D71179#1774487 , @jdoerfert wrote: > In D71179#1774471 , @ABataev wrote: > > > They do this because they have several function definitions with the same > > name. In our case, we have se

[PATCH] D71179: [OpenMP][WIP] Initial support for `begin/end declare variant`

ABataev added a comment. In D71179#1775066 , @hfinkel wrote: > In D71179#1774678 , @ABataev wrote: > > > In D71179#1774487 , @jdoerfert > > wrote: > > > > > In D71179#177447

[PATCH] D71179: [OpenMP][WIP] Initial support for `begin/end declare variant`

ABataev added a comment. In D71179#1775442 , @jdoerfert wrote: > > @jdoerfert , how does the ".ompvariant" work with external functions? I see > > the part of the spec which says, "The symbol name of a function definition > > that appears between a begin

[PATCH] D71179: [OpenMP][WIP] Initial support for `begin/end declare variant`

ABataev added a comment. In D71179#1775687 , @jdoerfert wrote: > > @jdoerfert , also, do we have tests that can go into the test suite / > > libomptarget regression tests demonstrating the collection of problems > > people have currently opened bugs on r

[PATCH] D71179: [OpenMP][WIP] Initial support for `begin/end declare variant`

ABataev added a comment. In D71179#1775834 , @jdoerfert wrote: > >> This is neither true, nor relevant. It is not true because OpenMP 5.0 > >> declare variant is so broken it cannot be used for what it was intended > >> for. That means people (as for exa

[PATCH] D70973: [OPENMP50]Treat context selectors as expressions, not just strings.

ABataev updated this revision to Diff 232917. ABataev added a comment. Herald added subscribers: s.egerton, simoncook, fedor.sergeev, aheejin, rampitec. Reworked attribute translationto try to reuse the context matching and scoring functionality with Sema. Repository: rG LLVM Github Monorepo

[PATCH] D71225: [OpenMP][WIP] atomic update only evaluate once for expression having side effect

ABataev added a comment. It is against the standard, I assume. According to standard, `During the execution of an atomic region, multiple syntactic occurrences of x must designate the same storage location.`. In your case, this is not so. I assume it would be better to implement the diagnostic

[PATCH] D71179: [OpenMP][WIP] Initial support for `begin/end declare variant`

ABataev added a comment. In D71179#1776034 , @jdoerfert wrote: > > Not always. If we see that the context selector does not match, we can skip > > everything between begin/end. It means exactly what I said - > > multiversioning is needed only for `constr

[PATCH] D71179: [OpenMP][WIP] Initial support for `begin/end declare variant`

ABataev added a comment. In D71179#1776108 , @jdoerfert wrote: > In D71179#1776046 , @ABataev wrote: > > > In D71179#1776034 , @jdoerfert > > wrote: > > > > > > Not always.

[PATCH] D71225: [OpenMP][WIP] atomic update only evaluate once for expression having side effect

ABataev added a comment. In D71225#1776113 , @jdoerfert wrote: > In D71225#1776105 , @cchen wrote: > > > Oops, accidentally remove my own comment. I'm not sure why `iarr[foo(), > > foo(), 0]` violate the rule since

[PATCH] D71179: [OpenMP][WIP] Initial support for `begin/end declare variant`

ABataev added a comment. In D71179#1776457 , @jdoerfert wrote: > > You're doing absolutely the same thing as the original declare variant > > implementation. > > I don't think so but if you do why do you oppose this approach? > > > And I don't think it wo

[PATCH] D71179: [OpenMP][WIP] Initial support for `begin/end declare variant`

ABataev added a comment. In D71179#1776487 , @hfinkel wrote: > In D71179#1776467 , @ABataev wrote: > > > In D71179#1776457 , @jdoerfert > > wrote: > > > > > > You're doing a

[PATCH] D71179: [OpenMP][WIP] Initial support for `begin/end declare variant`

ABataev added a comment. In D71179#1776528 , @hfinkel wrote: > In D71179#1776491 , @ABataev wrote: > > > In D71179#1776487 , @hfinkel wrote: > > > > > In D71179#1776467

[PATCH] D71241: [OpenMP][WIP] Use overload centric declare variants

ABataev added a comment. You're merging different functions as multiversiin variants. I don't think this right to overcomplicate the semantics of multiversion functions just because you want to do it. Comment at: clang/lib/Sema/SemaOverload.cpp:9725 + + // TODO: Handle temp

[PATCH] D70973: [OPENMP50]Treat context selectors as expressions, not just strings.

ABataev updated this revision to Diff 233093. ABataev added a comment. Fixed misprint in the comment. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D70973/new/ https://reviews.llvm.org/D70973 Files: clang/include/clang/Basic/Attr.td clang/inclu

[PATCH] D71241: [OpenMP][WIP] Use overload centric declare variants

ABataev added a comment. In D71241#1777661 , @jdoerfert wrote: > In D71241#1776798 , @ABataev wrote: > > > You're merging different functions as multiversiin variants. I don't think > > this right to overcomplicat

[PATCH] D71286: [SYCL] Add support for auxiliary triple specification to Frontend

ABataev added a comment. Most probably, just missed the testing of the frontend part here Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71286/new/ https://reviews.llvm.org/D71286 ___ cfe-commits mailin

[PATCH] D71286: [SYCL] Add support for auxiliary triple specification to Frontend

ABataev added a comment. In D71286#1777917 , @bader wrote: > In D71286#1777894 , @ABataev wrote: > > > Most probably, just missed the testing of the frontend part here > > > Should I add test for both: SYCL and Open

[PATCH] D71294: [OpenMP] Emit warning message about undefined number of evaluation for atomic update/capture

ABataev added a comment. I don't think this kind of analysis must be here. clang has a special analysis phase, maybe better to implement it there? Currently, it may produce false-positive warnings. Also, why only check for array accesses? Are they the only constructs that may lead to undefined

[PATCH] D71241: [OpenMP][WIP] Use overload centric declare variants

ABataev added a comment. In D71241#1777972 , @jdoerfert wrote: > In D71241#109 , @ABataev wrote: > > > In D71241#1777661 , @jdoerfert > > wrote: > > > > > In D71241#1776

[PATCH] D71179: [OpenMP][WIP] Initial support for `begin/end declare variant`

ABataev added a comment. In D71179#1778512 , @hfinkel wrote: > In D71179#1776761 , @ABataev wrote: > > > In D71179#1776528 , @hfinkel wrote: > > > > > In D71179#1776491

[PATCH] D71241: [OpenMP][WIP] Use overload centric declare variants

ABataev added a comment. In D71241#1778564 , @hfinkel wrote: > In D71241#1778134 , @ABataev wrote: > > > > > > ... > > >> > >> > >>> Also, check how -ast-print works with your solution. It returns different > >>>

[PATCH] D71241: [OpenMP][WIP] Use overload centric declare variants

ABataev added a comment. In D71241#1778564 , @hfinkel wrote: > In D71241#1778134 , @ABataev wrote: > > > > > > ... > > >> > >> > >>> Also, check how -ast-print works with your solution. It returns different > >>>

[PATCH] D71241: [OpenMP][WIP] Use overload centric declare variants

ABataev added a comment. There can be another one issue with this solution with inline assembly. I’m not completely sure about it, will try to investigate it tomorrow. I suggest to discuss this solution with Richard Smith (or John McCall). If he/they are ok with this transformation of the AST,

[PATCH] D71241: [OpenMP][WIP] Use overload centric declare variants

ABataev added a comment. In D71241#1778717 , @jdoerfert wrote: > >> There is no evidence that this is more complicated. By all measures, this > >> is less complicated (see also below). It is also actually doing the right > >> thing when it comes to code

[PATCH] D71241: [OpenMP][WIP] Use overload centric declare variants

ABataev added a comment. In D71241#1778963 , @jdoerfert wrote: > In D71241#1778736 , @ABataev wrote: > > > In D71241#1778717 , @jdoerfert > > wrote: > > > > > >> There is no

[PATCH] D71241: [OpenMP][WIP] Use overload centric declare variants

ABataev added a comment. In D71241#1779168 , @JonChesterfield wrote: > Lowering in sema or in codegen seems a standard phase ordering choice. There > will be pros and cons to both. > > I think prior art leans towards sema. Variants are loosely equivalent

[PATCH] D71286: [SYCL] Add support for auxiliary triple specification to Frontend

ABataev accepted this revision. ABataev added a comment. This revision is now accepted and ready to land. LG Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71286/new/ https://reviews.llvm.org/D71286 ___

[PATCH] D71241: [OpenMP][WIP] Use overload centric declare variants

ABataev added a comment. Here is the example that does not work with the proposed solution but works with the existing one: static void cpu() { asm("nop"); } #pragma omp declare variant(cpu) match(device = {kind(cpu)}) static __attribute__((used)) void wrong_asm() { asm ("xxx"); }

[PATCH] D70973: [OPENMP50]Treat context selectors as expressions, not just strings.

ABataev updated this revision to Diff 233372. ABataev added a comment. Rebase + moved function for best variant to FunctionDecl. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D70973/new/ https://reviews.llvm.org/D70973 Files: clang/include/clang/

[PATCH] D70973: [OPENMP50]Treat context selectors as expressions, not just strings.

ABataev updated this revision to Diff 233375. ABataev added a comment. Fixed formatting Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D70973/new/ https://reviews.llvm.org/D70973 Files: clang/include/clang/AST/Decl.h clang/include/clang/Basic/At

[PATCH] D69785: [OpenMP] Introduce the OpenMP-IR-Builder

ABataev added inline comments. Comment at: llvm/include/llvm/Frontend/OpenMP/OMPConstants.h:74 +/// +///{ +namespace types { Extra comments? Comment at: llvm/include/llvm/Frontend/OpenMP/OMPConstants.h:93 +} // namespace types +///} + -

[PATCH] D69785: [OpenMP] Introduce the OpenMP-IR-Builder

ABataev added inline comments. Comment at: llvm/include/llvm/Frontend/OpenMP/OMPConstants.h:74 +/// +///{ +namespace types { jdoerfert wrote: > ABataev wrote: > > Extra comments? > I don't know what you want to tell me. I mean, you have this `///{` here. Do you n

[PATCH] D70258: [OpenMP][IR-Builder] Introduce the finalization stack

ABataev added a comment. Tests? Comment at: llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h:51 + /// at the time, and location, the callback is invoked. + using FinalizeCallbackTy = std::function; + `llvm::function_ref`? Repository: rG LLVM Github Monore

[PATCH] D70258: [OpenMP][IR-Builder] Introduce the finalization stack

ABataev added inline comments. Comment at: llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h:51 + /// at the time, and location, the callback is invoked. + using FinalizeCallbackTy = std::function; + jdoerfert wrote: > ABataev wrote: > > `llvm::function_ref`? >

[PATCH] D71413: [OpenMP][Test] Add check for aux-triple predefined macros

ABataev accepted this revision. ABataev added a comment. This revision is now accepted and ready to land. LG, thanks! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71413/new/ https://reviews.llvm.org/D71413 __

[PATCH] D70258: [OpenMP][IR-Builder] Introduce the finalization stack

ABataev added inline comments. Comment at: llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h:51 + /// at the time, and location, the callback is invoked. + using FinalizeCallbackTy = std::function; + jdoerfert wrote: > ABataev wrote: > > jdoerfert wrote: > > > A

[PATCH] D71241: [OpenMP][WIP] Use overload centric declare variants

ABataev added a comment. In D71241#1780715 , @jdoerfert wrote: > In D71241#1779097 , @ABataev wrote: > > > In D71241#1778963 , @jdoerfert > > wrote: > > > > > In D71241#1778

[PATCH] D70258: [OpenMP][IR-Builder] Introduce the finalization stack

ABataev added inline comments. Comment at: llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h:51 + /// at the time, and location, the callback is invoked. + using FinalizeCallbackTy = std::function; + jdoerfert wrote: > ABataev wrote: > > jdoerfert wrote: > > > A

[PATCH] D71241: [OpenMP][WIP] Use overload centric declare variants

ABataev added a comment. In D71241#1782157 , @jdoerfert wrote: > In D71241#1781955 , @ABataev wrote: > > > In D71241#1780715 , @jdoerfert > > wrote: > > > > > In D71241#1779

[PATCH] D71241: [OpenMP][WIP] Use overload centric declare variants

ABataev added a comment. In D71241#1782317 , @jdoerfert wrote: > In D71241#1782173 , @ABataev wrote: > > > In D71241#1782157 , @jdoerfert > > wrote: > > > > > In D71241#1781

[PATCH] D71241: [OpenMP][WIP] Use overload centric declare variants

ABataev added a comment. In D71241#1782317 , @jdoerfert wrote: > In D71241#1782173 , @ABataev wrote: > > > In D71241#1782157 , @jdoerfert > > wrote: > > > > > In D71241#1781

[PATCH] D71241: [OpenMP][WIP] Use overload centric declare variants

ABataev added a comment. In D71241#1782425 , @JonChesterfield wrote: > > Explain that you're replacing the function written by the user on the fly > > by another one. If they accept it, go ahead. > > That's the observational effect of variants. Replacing

[PATCH] D71241: [OpenMP][WIP] Use overload centric declare variants

ABataev added a comment. In D71241#1782430 , @JonChesterfield wrote: > In D71241#1782427 , @ABataev wrote: > > > In D71241#1782425 , > > @JonChesterfield wrote: > > > > > >

[PATCH] D71241: [OpenMP][WIP] Use overload centric declare variants

ABataev added a comment. In D71241#1782504 , @jdoerfert wrote: > In D71241#1782460 , @JonChesterfield > wrote: > > > > https://clang.llvm.org/docs/InternalsManual.html#the-ast-library > > > > > > Faithfulness¶ >

[PATCH] D71241: [OpenMP][WIP] Use overload centric declare variants

ABataev added a comment. In D71241#1782551 , @hfinkel wrote: > In D71241#1779168 , @JonChesterfield > wrote: > > > Lowering in sema or in codegen seems a standard phase ordering choice. > > There will be pros and

[PATCH] D71241: [OpenMP][WIP] Use overload centric declare variants

ABataev added a comment. In D71241#1782648 , @hfinkel wrote: > In D71241#1782614 , @ABataev wrote: > > > In D71241#1782551 , @hfinkel wrote: > > > > > In D71241#1779168

[PATCH] D71241: [OpenMP][WIP] Use overload centric declare variants

ABataev added a comment. In D71241#1782650 , @jdoerfert wrote: > In D71241#1782614 , @ABataev wrote: > > > Actually, early resolution will break tbe tools, not help them. It will > > definitely break clangd, for ex

[PATCH] D71241: [OpenMP][WIP] Use overload centric declare variants

ABataev added a comment. In D71241#1782700 , @hfinkel wrote: > In D71241#1782668 , @ABataev wrote: > > > > > > ... > > >> While we talk a lot about what you think is bad about this solution it > >> seems we ignore

[PATCH] D71241: [OpenMP][WIP] Use overload centric declare variants

ABataev added a comment. In D71241#1782670 , @hfinkel wrote: > In D71241#1782652 , @ABataev wrote: > > > In D71241#1782648 , @hfinkel wrote: > > > > > In D71241#1782614

[PATCH] D71241: [OpenMP][WIP] Use overload centric declare variants

ABataev added a comment. I don't insist on function redefinition solution. You want to replace functions - fine, but do this at the codegen, not in AST. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71241/new/ https://reviews.llvm.org/D71241 __

[PATCH] D71241: [OpenMP][WIP] Use overload centric declare variants

ABataev added a comment. In D71241#1782742 , @hfinkel wrote: > In D71241#1782703 , @ABataev wrote: > > > > > > ... > > >> > >> > >>> Given we have two implementations, each at different points in the

[PATCH] D71241: [OpenMP][WIP] Use overload centric declare variants

ABataev added a comment. In D71241#1782812 , @hfinkel wrote: > In D71241#1782779 , @ABataev wrote: > > > In D71241#1782742 , @hfinkel wrote: > > > > > In D71241#1782703

[PATCH] D71241: [OpenMP][WIP] Use overload centric declare variants

ABataev added a comment. In D71241#1782898 , @JonChesterfield wrote: > In D71241#1782846 , @ABataev wrote: > > > But I suggest to discuss this with Richard Smith. > > > Is the appeal to authority necessary to resol

[PATCH] D71241: [OpenMP][WIP] Use overload centric declare variants

ABataev added a comment. In D71241#1782963 , @jdoerfert wrote: > In D71241#1782668 , @ABataev wrote: > > > In D71241#1782650 , @jdoerfert > > wrote: > > > > > While we talk

[PATCH] D71241: [OpenMP][WIP] Use overload centric declare variants

ABataev added a comment. In D71241#1782586 , @hfinkel wrote: > In D71241#1782460 , @JonChesterfield > wrote: > > > > https://clang.llvm.org/docs/InternalsManual.html#the-ast-library > > > > > > Faithfulness¶ > >

[PATCH] D71241: [OpenMP][WIP] Use overload centric declare variants

ABataev added a comment. In D71241#1783499 , @hfinkel wrote: > In D71241#1783444 , @ABataev wrote: > > > In D71241#1782586 , @hfinkel wrote: > > > > > In D71241#1782460

[PATCH] D71475: [WIP][OPENMP] Try to fix linear clause crash by emitting alloca for step

ABataev added a comment. Here is the fix: diff --git a/clang/lib/Sema/SemaOpenMP.cpp b/clang/lib/Sema/SemaOpenMP.cpp index e02c1c5..5ce81b0 100644 --- a/clang/lib/Sema/SemaOpenMP.cpp +++ b/clang/lib/Sema/SemaOpenMP.cpp @@ -3830,6 +3830,9 @@ StmtResult Sema::ActOnOpenMPRegionEnd(StmtResu

[PATCH] D67833: [OpenMP 5.0] Codegen support to pass user-defined mapper functions to runtime

ABataev added inline comments. Comment at: clang/include/clang/AST/OpenMPClause.h:4918 const OMPMappableExprListSizeTy &Sizes) - : OMPMappableExprListClause(OMPC_map, Locs, Sizes, &MapperQualifierLoc, - &MapperIdInfo)

[PATCH] D67833: [OpenMP 5.0] Codegen support to pass user-defined mapper functions to runtime

ABataev added inline comments. Comment at: clang/include/clang/AST/OpenMPClause.h:4918 const OMPMappableExprListSizeTy &Sizes) - : OMPMappableExprListClause(OMPC_map, Locs, Sizes, &MapperQualifierLoc, - &MapperIdInfo)

[PATCH] D71241: [OpenMP][WIP] Use overload centric declare variants

ABataev added a comment. Most probably, we can use this solution without adding a new expression. `DeclRefExpr` class can contain 2 decls: FoundDecl and the Decl being used. You can use FoundDecl to point to the original function and used decl to point to the function being called in this conte

[PATCH] D71241: [OpenMP][WIP] Use overload centric declare variants

ABataev added a comment. In D71241#1787265 , @hfinkel wrote: > In D71241#1786959 , @jdoerfert wrote: > > > In D71241#1786530 , @ABataev wrote: > > > > > Most probably, we can

[PATCH] D71241: [OpenMP][WIP] Use overload centric declare variants

ABataev added a comment. Hal, are we going to support something like this? void cpu() { asm("nop"); } #pragma omp declare variant(cpu) match(device = {kind(cpu)}) void wrong_asm() { asm ("xxx"); } Currently, there is an error when we try to emit the assembler output. Repository:

[PATCH] D71475: [WIP][OPENMP] Try to fix linear clause crash by emitting alloca for step

ABataev added a comment. Here is more proper fix. We don't need to capture just `k` here, instead, we need to capture the whole expression. Linear clause has a little bit different processing rather than all other clauses caused by a non-perfect design. Add codegen tests for all combined constr

[PATCH] D67833: [OpenMP 5.0] Codegen support to pass user-defined mapper functions to runtime

ABataev added inline comments. Comment at: clang/include/clang/AST/OpenMPClause.h:4918 const OMPMappableExprListSizeTy &Sizes) - : OMPMappableExprListClause(OMPC_map, Locs, Sizes, &MapperQualifierLoc, - &MapperIdInfo)

[PATCH] D71475: [WIP][OPENMP] Try to fix linear clause crash by emitting alloca for step

ABataev added inline comments. Comment at: clang/lib/Sema/SemaOpenMP.cpp:1128 + (A == OMPC_firstprivate && (Data.Attributes == OMPC_lastprivate || + Data.Attributes == OMPC_linear)) || (A == OMPC_lastprivate && Data.Attr

[PATCH] D71241: [OpenMP][WIP] Use overload centric declare variants

ABataev added a comment. In D71241#1787998 , @jdoerfert wrote: > In D71241#1787888 , @ABataev wrote: > > > Hal, are we going to support something like this? > > > I'm not Hal but I will answer anyway. > > > void c

[PATCH] D71241: [OpenMP][WIP] Use overload centric declare variants

ABataev added a comment. In D71241#1788003 , @jdoerfert wrote: > In D71241#1787652 , @hfinkel wrote: > > > In D71241#1787571 , @ABataev wrote: > > > > > In D71241#1787265

[PATCH] D71475: [WIP][OPENMP] Try to fix linear clause crash by emitting alloca for step

ABataev added inline comments. Comment at: clang/lib/Sema/SemaOpenMP.cpp:1128 + (A == OMPC_firstprivate && (Data.Attributes == OMPC_lastprivate || + Data.Attributes == OMPC_linear)) || (A == OMPC_lastprivate && Data.Attr

[PATCH] D71475: [WIP][OPENMP] Try to fix linear clause crash by emitting alloca for step

ABataev added inline comments. Comment at: clang/lib/Sema/SemaOpenMP.cpp:1128 + (A == OMPC_firstprivate && (Data.Attributes == OMPC_lastprivate || + Data.Attributes == OMPC_linear)) || (A == OMPC_lastprivate && Data.Attr

[PATCH] D71475: [WIP][OPENMP] Try to fix linear clause crash by emitting alloca for step

ABataev added a comment. The fixed patch. Several codegen tests require some adjustment. diff --git a/clang/lib/Sema/SemaOpenMP.cpp b/clang/lib/Sema/SemaOpenMP.cpp index afe0f1a..ecb0fb2 100644 --- a/clang/lib/Sema/SemaOpenMP.cpp +++ b/clang/lib/Sema/SemaOpenMP.cpp @@ -919,6 +919,11 @@

[PATCH] D70258: [OpenMP][IR-Builder] Introduce the finalization stack

ABataev added a comment. In D70258#1788825 , @rjmccall wrote: > So it's never that OpenMP has things that need to be finalized before exiting > (e.g. if something in frontend-emitted code throws an exception), it's just > that OpenMP might need to gene

[PATCH] D67833: [OpenMP 5.0] Codegen support to pass user-defined mapper functions to runtime

ABataev added a comment. Looks much better in general, need to resolve last one issue with number of elements/size and you're ready. Comment at: clang/lib/CodeGen/CGOpenMPRuntime.cpp:8981-8982 + // Convert the size in bytes into the number of array elements. + Size = MapperC

[PATCH] D67833: [OpenMP 5.0] Codegen support to pass user-defined mapper functions to runtime

ABataev added inline comments. Comment at: clang/lib/CodeGen/CGOpenMPRuntime.cpp:9244 +llvm::Function * +CGOpenMPRuntime::getUserDefinedMapperFunc(const OMPDeclareMapperDecl *D) { + auto I = UDMMap.find(D); lildmh wrote: > ABataev wrote: > > `getOrEmitUserDefine

<    12   13   14   15   16   17   18   19   20   21   >