Initial support for OpenMP 5.0 declare target "as if" behavior for "initializer expressions".
If a variable with static storage duration or a function (except lambda for C++) is referenced in the initializer expression list of a variable with static storage duration that appears as a list item in a to clause on a declare target directive then the name of the referenced variable or function is treated as if it had appeared in a to clause on a declare target directive.
Details
Diff Detail
Event Timeline
clang/lib/Parse/ParseDecl.cpp | ||
---|---|---|
2093–2115 ↗ | (On Diff #506554) | It has nothing to do with parsing, sema analysis. Make it part of Sema::checkDeclIsAllowedInOpenMPTarget |
2107 ↗ | (On Diff #506554) | Try to avoid recursion here |
clang/lib/Parse/ParseDecl.cpp | ||
---|---|---|
2107 ↗ | (On Diff #506554) | Consider the following example- static int var1 = 10; static int *var2 = &var1; #pragma omp declare target int **ptr1=&var2; #pragma omp end declare target In this case, by default ptr1 gets OMPDeclareTargetDeclAttr and the function mentioned above (ParseImplicitDeclareTargetAttr) would add OMPDeclareTargetDeclAttr to var2. But, in order to add OMPDeclareTargetDeclAttr to var1, recursion would be required. |
clang/lib/Parse/ParseDecl.cpp | ||
---|---|---|
2107 ↗ | (On Diff #506554) | I do not ask to remove it, but use extra memory (SamllVector) to save the decls, requiring analysis, and iterate through this vector. |
clang/lib/Parse/ParseDecl.cpp | ||
---|---|---|
2093–2115 ↗ | (On Diff #506554) | The Declaration in Sema::checkDeclIsAllowedInOpenMPTarget is incomplete. VarDecl 0x1582b278 <test3.c:6:1, col:7> col:7 ptr1 'int **' `-OMPDeclareTargetDeclAttr 0x1582b2e0 <line:5:21> Implicit MT_To DT_Any 1 `-<<<NULL>>> At this point, I do not have access to the initializer expression. Any suggestions? |
clang/lib/Parse/ParseDecl.cpp | ||
---|---|---|
2093–2115 ↗ | (On Diff #506554) | The try to do it ActOnOpenMPDeclareTargetName |
clang/lib/Parse/ParseDecl.cpp | ||
---|---|---|
2093–2115 ↗ | (On Diff #506554) | In the absence of a declare target clause, this function is not invoked (in the examples used above, the function is never called). Do you have any suggestions? |
clang/lib/Parse/ParseDecl.cpp | ||
---|---|---|
2093–2115 ↗ | (On Diff #506554) | For such decls use checkDeclIsAllowedInOpenMPTarget |
clang/lib/Parse/ParseDecl.cpp | ||
---|---|---|
2093–2115 ↗ | (On Diff #506554) | As mentioned in my previous comment, the Decl in checkDeclIsAllowedInOpenMPTarget is not complete and I don't have access to the initializer expression hence I am unable to move my functionality into checkDeclIsAllowedInOpenMPTarget. |
clang/lib/Parse/ParseDecl.cpp | ||
---|---|---|
2093–2115 ↗ | (On Diff #506554) | For decls, explicitly marked as DeclareTarget, use The try to do it ActOnOpenMPDeclareTargetName. For decls, referenced in the initializers, use checkDeclIsAllowedInOpenMPTarget. |
clang/lib/Sema/SemaDecl.cpp | ||
---|---|---|
14467–14468 | if (auto *VD = dyn_cast_or_null<VarDecl>(D); LangOpts.OpenMP && VD && VD->hasAttr<OMPDeclareTargetDeclAttr>() && VD->hasGlobalStorage()) | |
clang/lib/Sema/SemaOpenMP.cpp | ||
23091 | static void addOMP... | |
23092 | SmallVectorImpl<Decl *> &DeclVector | |
23093–23100 | if (!DeclRef) return; if (auto *VD = dyn_cast_or_null<VarDecl>(DeclRef->getDecl()) { DeclVar->addAttr(TargetDecl->getAttr<OMPDeclareTargetDeclAttr>()); DeclVector.push_back(DeclVar); } | |
23103–23114 | Use StmtVisitor to implement this correctly. | |
23120 | 100 is to much, use either default value SmallVector<Decl *> DeclVector; or smaller value (4 should be enough) | |
23122 | while (!DeclVector.empty()) | |
23123–23124 | Decl *D = DeclVector.pop_back_val(); | |
23125–23126 | Use dyn_cast instead of isa/cast |
clang/lib/Sema/SemaDecl.cpp | ||
---|---|---|
14467 | В is already checked that is not nullptr, so just a dyn_cast should be enough | |
clang/lib/Sema/SemaOpenMP.cpp | ||
23093–23094 | Why public? | |
23097 | const auto *VD | |
23098 | Use VD instead of this and drop const | |
23099–23100 | Drop this and extra parens | |
23106–23109 | Just Visit(*it)? |
clang/lib/Sema/SemaOpenMP.cpp | ||
---|---|---|
23093–23094 | As the data members are accessed from outside the class (in ActOnOpenMPImplicitDeclareTarget function), I have made them public. | |
23106–23109 | Or I can call VisitDeclRefExpr directly. Would that be more suitable? |
clang/lib/Sema/SemaOpenMP.cpp | ||
---|---|---|
23093–23094 | If you need them, implement getters for them and make them private. | |
23105–23106 | Why just a regular Visit does not work here? Plus, isa + dyn_cast is weird. |
clang/include/clang/Sema/Sema.h | ||
---|---|---|
11322 | Do I understand correctly, that the variable itself is marked explicitly, but the initializer shall be marked implicitly? If so, please fix the function name | |
clang/lib/Sema/SemaOpenMP.cpp | ||
23096–23097 | Remove this->, where not required. | |
23098 |
| |
23101 | Same, as for Push | |
23125–23139 | Better to hide this in GlobalDeclRefChecker |
clang/lib/Sema/SemaDecl.cpp | ||
---|---|---|
14469–14471 | Remove braces | |
clang/lib/Sema/SemaOpenMP.cpp | ||
23090 | Please, add comments, describing the class and its functionality | |
23094 | setTargetDecl | |
23095 |
| |
23097 | const member function | |
23103 | Remove this-> | |
23112 |
|
clang/lib/Sema/SemaOpenMP.cpp | ||
---|---|---|
23108–23109 | for (const Expr *Child : Ex->children()) | |
23125–23130 | Shall just something like this work here: if (Ex) Visit(Ex); |
clang/lib/Sema/SemaOpenMP.cpp | ||
---|---|---|
23114 | Fix the strange symbol in the comment | |
23116 | Function names should be verb phrases (as they represent actions), and command-like function should be imperative. (https://llvm.org/docs/CodingStandards.html#name-types-functions-variables-and-enumerators-properly) |
clang/lib/Sema/SemaOpenMP.cpp | ||
---|---|---|
23135 | FYI I fixed an unused var warning here: https://github.com/llvm/llvm-project/commit/f3ca99a87c566fb5e910071f4cbb474ddb4e7f37 Potentially you could pass the result of the cast into declareTargetInitializer but I don't know enough about the code to change that too. |
Do I understand correctly, that the variable itself is marked explicitly, but the initializer shall be marked implicitly? If so, please fix the function name