[PATCH] D72811: [WIP][OPENMP5.0] allow lvalue for motion clause

ABataev added inline comments. Comment at: clang/lib/Sema/SemaOpenMP.cpp:15293 + // TypeDecorCnt for B: 0 + if (Depth <= TypeDecorCnt) { +RelevantExpr = cast(DRE); cchen wrote: > ABataev wrote: > > The check is really bad. If you want to handle

[PATCH] D72811: [WIP][OPENMP5.0] allow lvalue for motion clause

cchen marked an inline comment as done. cchen added inline comments. Comment at: clang/lib/Sema/SemaOpenMP.cpp:15293 + // TypeDecorCnt for B: 0 + if (Depth <= TypeDecorCnt) { +RelevantExpr = cast(DRE); ABataev wrote: > The check is really bad.

[PATCH] D72811: [WIP][OPENMP5.0] allow lvalue for motion clause

ABataev added a comment. I would suggest starting with the reworking of the existing implementation with StmtVisitor class and after that step-by-step extend the functionality of the visitor with handling other kinds а expressions. Comment at: clang/lib/Sema/SemaOpenMP.cpp:15

[PATCH] D72811: [WIP][OPENMP5.0] allow lvalue for motion clause

cchen marked an inline comment as done. cchen added a comment. > Quoted Text Comment at: clang/lib/Sema/SemaOpenMP.cpp:15554 +LocatorChecker Checker; +if (Checker.Visit(OrigExpr)) { + llvm::copy(Checker.getComponents(), ABataev wrote:

[PATCH] D72811: [WIP][OPENMP5.0] allow lvalue for motion clause

cchen updated this revision to Diff 243950. cchen added a comment. 1. Refactor based on feedback and use the visitor class to integrate the analysis in checkMapClauseExpressionBase. 2. Handle more lvalue cases Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.

[PATCH] D72811: [WIP][OPENMP5.0] allow lvalue for motion clause

ABataev added inline comments. Comment at: clang/lib/Sema/SemaOpenMP.cpp:15554 +LocatorChecker Checker; +if (Checker.Visit(OrigExpr)) { + llvm::copy(Checker.getComponents(), cchen wrote: > ABataev wrote: > > cchen wrote: > > > ABataev wro

[PATCH] D72811: [WIP][OPENMP5.0] allow lvalue for motion clause

cchen marked an inline comment as done. cchen added inline comments. Comment at: clang/lib/Sema/SemaOpenMP.cpp:15554 +LocatorChecker Checker; +if (Checker.Visit(OrigExpr)) { + llvm::copy(Checker.getComponents(), ABataev wrote: > cchen wro

[PATCH] D72811: [WIP][OPENMP5.0] allow lvalue for motion clause

ABataev added inline comments. Comment at: clang/lib/Sema/SemaOpenMP.cpp:15554 +LocatorChecker Checker; +if (Checker.Visit(OrigExpr)) { + llvm::copy(Checker.getComponents(), cchen wrote: > ABataev wrote: > > General question about several

[PATCH] D72811: [WIP][OPENMP5.0] allow lvalue for motion clause

cchen marked an inline comment as done. cchen added inline comments. Comment at: clang/lib/Sema/SemaOpenMP.cpp:15554 +LocatorChecker Checker; +if (Checker.Visit(OrigExpr)) { + llvm::copy(Checker.getComponents(), ABataev wrote: > General q

[PATCH] D72811: [WIP][OPENMP5.0] allow lvalue for motion clause

ABataev added inline comments. Comment at: clang/lib/Sema/SemaOpenMP.cpp:15201 +namespace { +class LocatorChecker final : public StmtVisitor { + OMPClauseMappableExprCommon::MappableExprComponentList Components; Seems to me, you're skipping many expressions, whi

[PATCH] D72811: [WIP][OPENMP5.0] allow lvalue for motion clause

cchen updated this revision to Diff 241293. cchen added a comment. Update due to negligence Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D72811/new/ https://reviews.llvm.org/D72811 Files: clang/include/clang/Basic/DiagnosticSemaKinds.td clang/

[PATCH] D72811: [WIP][OPENMP5.0] allow lvalue for motion clause

cchen updated this revision to Diff 241291. cchen added a comment. Fix based on feedback Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D72811/new/ https://reviews.llvm.org/D72811 Files: clang/include/clang/Basic/DiagnosticSemaKinds.td clang/lib

[PATCH] D72811: [WIP][OPENMP5.0] allow lvalue for motion clause

ABataev added a comment. In D72811#1843066 , @cchen wrote: > In D72811#1839561 , @ABataev wrote: > > > In D72811#1839538 , @cchen wrote: > > > > > In D72811#1837392

[PATCH] D72811: [WIP][OPENMP5.0] allow lvalue for motion clause

cchen marked an inline comment as done. cchen added a comment. In D72811#1839561 , @ABataev wrote: > In D72811#1839538 , @cchen wrote: > > > In D72811#1837392 , @jdoerfert >

[PATCH] D72811: [WIP][OPENMP5.0] allow lvalue for motion clause

cchen marked an inline comment as done. cchen added inline comments. Comment at: clang/lib/Sema/SemaOpenMP.cpp:15235 + bool VisitUnaryOperator(UnaryOperator *UO) { +DerefCnt++; +CurComponents.emplace_back(UO, nullptr); ABataev wrote: > cchen wrote: > > c

[PATCH] D72811: [WIP][OPENMP5.0] allow lvalue for motion clause

ABataev added a comment. In D72811#1839538 , @cchen wrote: > In D72811#1837392 , @jdoerfert wrote: > > > Thanks for working on this! While you are at it, `*this` is probably one of > > the most important ones to te

[PATCH] D72811: [WIP][OPENMP5.0] allow lvalue for motion clause

cchen marked an inline comment as done. cchen added inline comments. Comment at: clang/lib/Sema/SemaOpenMP.cpp:15235 + bool VisitUnaryOperator(UnaryOperator *UO) { +DerefCnt++; +CurComponents.emplace_back(UO, nullptr); cchen wrote: > ABataev wrote: > > N

[PATCH] D72811: [WIP][OPENMP5.0] allow lvalue for motion clause

cchen marked an inline comment as done. cchen added a comment. In D72811#1837392 , @jdoerfert wrote: > Thanks for working on this! While you are at it, `*this` is probably one of > the most important ones to test and support. Can anyone tell me how to g

[PATCH] D72811: [WIP][OPENMP5.0] allow lvalue for motion clause

ABataev added inline comments. Comment at: clang/lib/CodeGen/CGOpenMPRuntime.cpp:7829 if (EncounteredME) { - const auto *FD = dyn_cast(EncounteredME->getMemberDecl()); - unsigned FieldIndex = FD->getFieldIndex(); - - // Update info about the lo

[PATCH] D72811: [WIP][OPENMP5.0] allow lvalue for motion clause

cchen marked 2 inline comments as done. cchen added inline comments. Comment at: clang/lib/CodeGen/CGOpenMPRuntime.cpp:7829 if (EncounteredME) { - const auto *FD = dyn_cast(EncounteredME->getMemberDecl()); - unsigned FieldIndex = FD->getFieldIndex(); -

[PATCH] D72811: [WIP][OPENMP5.0] allow lvalue for motion clause

ABataev added inline comments. Comment at: clang/lib/CodeGen/CGOpenMPRuntime.cpp:7711-7713 +isa(Next->getAssociatedExpression()) || +isa(Next->getAssociatedExpression()) || +isa(Next->getAssociatedExpression())) &&

[PATCH] D72811: [WIP][OPENMP5.0] allow lvalue for motion clause

jdoerfert added a comment. Thanks for working on this! While you are at it, `*this` is probably one of the most important ones to test and support. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D72811/new/ https://reviews.llvm.org/D72811 ___

[PATCH] D72811: [WIP][OPENMP5.0] allow lvalue for motion clause

cchen updated this revision to Diff 240029. cchen added a comment. Add a checker for locator (test is not complete yet). The logic of finding basic decl in LocatorChecker: - `int *B; omp target map(*B)` - B - pointer count (* in type): 1 - deref count: 1 - B is basic decl since poi

[PATCH] D72811: [WIP][OPENMP5.0] allow lvalue for motion clause

ABataev added inline comments. Comment at: clang/lib/Sema/SemaOpenMP.cpp:15442-15443 + CurComponents.emplace_back(CurE, nullptr); +} else if (auto *CurE = dyn_cast(E)) { + E = CurE->getLHS()->IgnoreParenImpCasts(); } else { cchen wrote: > cchen

[PATCH] D72811: [WIP][OPENMP5.0] allow lvalue for motion clause

cchen marked an inline comment as done. cchen added inline comments. Comment at: clang/lib/Sema/SemaOpenMP.cpp:15442-15443 + CurComponents.emplace_back(CurE, nullptr); +} else if (auto *CurE = dyn_cast(E)) { + E = CurE->getLHS()->IgnoreParenImpCasts(); } else {

[PATCH] D72811: [WIP][OPENMP5.0] allow lvalue for motion clause

cchen marked an inline comment as done. cchen added inline comments. Comment at: clang/lib/Sema/SemaOpenMP.cpp:15442-15443 + CurComponents.emplace_back(CurE, nullptr); +} else if (auto *CurE = dyn_cast(E)) { + E = CurE->getLHS()->IgnoreParenImpCasts(); } else {

[PATCH] D72811: [WIP][OPENMP5.0] allow lvalue for motion clause

ABataev added inline comments. Comment at: clang/lib/Sema/SemaOpenMP.cpp:15442-15443 + CurComponents.emplace_back(CurE, nullptr); +} else if (auto *CurE = dyn_cast(E)) { + E = CurE->getLHS()->IgnoreParenImpCasts(); } else { cchen wrote: > ABata

[PATCH] D72811: [WIP][OPENMP5.0] allow lvalue for motion clause

cchen marked an inline comment as done. cchen added inline comments. Comment at: clang/lib/Sema/SemaOpenMP.cpp:15442-15443 + CurComponents.emplace_back(CurE, nullptr); +} else if (auto *CurE = dyn_cast(E)) { + E = CurE->getLHS()->IgnoreParenImpCasts(); } else {

[PATCH] D72811: [WIP][OPENMP5.0] allow lvalue for motion clause

ABataev added inline comments. Comment at: clang/lib/Sema/SemaOpenMP.cpp:15442-15443 + CurComponents.emplace_back(CurE, nullptr); +} else if (auto *CurE = dyn_cast(E)) { + E = CurE->getLHS()->IgnoreParenImpCasts(); } else { cchen wrote: > cchen

[PATCH] D72811: [WIP][OPENMP5.0] allow lvalue for motion clause

cchen marked an inline comment as done. cchen added inline comments. Comment at: clang/lib/Sema/SemaOpenMP.cpp:15442-15443 + CurComponents.emplace_back(CurE, nullptr); +} else if (auto *CurE = dyn_cast(E)) { + E = CurE->getLHS()->IgnoreParenImpCasts(); } else {

[PATCH] D72811: [WIP][OPENMP5.0] allow lvalue for motion clause

ABataev added a comment. Hmm, we still need to find some basic decl to remap it successfully at the codegen. Not sure that we'll be able to support it in full. It would be good to investigate how we can handle them at the codegen. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTIO

[PATCH] D72811: [WIP][OPENMP5.0] allow lvalue for motion clause

ABataev added a comment. Not sure that we need to dig that deep. Maybe just keep thw original analysis for the known expressions and map all other lvalues without some extra analysis, just like DeclRefExpr nodes? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://review

[PATCH] D72811: [WIP][OPENMP5.0] allow lvalue for motion clause

cchen marked 2 inline comments as done. cchen added inline comments. Comment at: clang/lib/CodeGen/CGOpenMPRuntime.cpp:7709-7712 isa(Next->getAssociatedExpression()) || isa(Next->getAssociatedExpression()) || -isa(Next->getAssocia

[PATCH] D72811: [WIP][OPENMP5.0] allow lvalue for motion clause

ABataev added inline comments. Comment at: clang/lib/CodeGen/CGOpenMPRuntime.cpp:7709-7712 isa(Next->getAssociatedExpression()) || isa(Next->getAssociatedExpression()) || -isa(Next->getAssociatedExpression())) && +

[PATCH] D72811: [WIP][OPENMP5.0] allow lvalue for motion clause

cchen created this revision. cchen added a reviewer: ABataev. Herald added subscribers: cfe-commits, guansong. Herald added a reviewer: jdoerfert. Herald added a project: clang. In OpenMP 5.0 we allow: int *p; int **p; struct S { int struct *sp; double *d; struct S **sp_ar