ABataev added a comment.
Tests?
================
Comment at: include/clang/Basic/LangOptions.def:193
LANGOPT(OpenMPUseTLS , 1, 0, "Use TLS for threadprivates or runtime
calls")
+LANGOPT(OpenMPImplicitDeclareTarget , 1, 0, "Enable implicit declare target
extension - marks automatically declarations and definitions with declare
target attribute")
LANGOPT(OpenMPIsDevice , 1, 0, "Generate code only for OpenMP target
device")
----------------
Where is it assigned a value 1? Currently it is disabled by default (initial
value is 0).
================
Comment at: include/clang/Sema/SemaInternal.h:65
+// mode.
+inline bool DeclAttrsMatchOffloadMode(const LangOptions &LangOpts, Decl *D,
+ bool InOpenMPDeviceRegion) {
----------------
Function should start from small letter
================
Comment at: include/clang/Sema/SemaInternal.h:70
+
+ return DeclAttrsMatchCUDAMode(LangOpts, D);
+}
----------------
This function should be called only we're using CUDA NVPTX as an offloading
device. Also, this function requires that `LangOpts.CUDA` was true. Do we set
`LangOpts.CUDA` to trues when trying to offload to CUDE devices at all? If no,
this function is useless.
================
Comment at: lib/Parse/ParseOpenMP.cpp:785-789
+ // Save the declarations so that we can create the declare target group
+ // later on.
+ if (Ptr)
+ for (auto *V : Ptr.get())
+ Decls.push_back(V);
----------------
Why do you need it?
================
Comment at: lib/Parse/ParseOpenMP.cpp:810-813
+ // If we have decls generate the group so that code can be generated for it
+ // later on.
+ if (!Decls.empty())
+ return Actions.BuildDeclaratorGroup(Decls);
----------------
Again, why do you need it?
================
Comment at: lib/Sema/SemaDecl.cpp:12661-12665
+ // In case of OpenMPImplicitDeclareTarget, semantically parsed function body
+ // is visited to mark inner callexpr with OMPDeclareTargetDeclAttr attribute.
+ if (getLangOpts().OpenMP && getLangOpts().OpenMPImplicitDeclareTarget)
+ checkDeclImplicitlyUsedOpenMPTargetContext(dcl);
+
----------------
I think this should be done only if the function has attached
`OMPDeclareTargetDeclAttr` and if `!getLangOpts().IsOpenMPDevice`. Also, I
don't like the idea of recursive scanning. It is better to add such markings on
the fly (when trying to build the expression)
================
Comment at: lib/Sema/SemaOpenMP.cpp:1170
+
+/// Traverse declaration of /param D to check whether it has
+/// OMPDeclareTargetDeclAttr or not. If so, it marks definition with
----------------
`\param`
================
Comment at: lib/Sema/SemaOpenMP.cpp:1177-1178
+ // Revealed function calls are marked with OMPDeclareTargetDeclAttr
+ // attribute,
+ // in case -fopenmp-implicit-declare-target extension is enabled.
+ ImplicitDeviceFunctionChecker FunctionCallChecker(SemaRef);
----------------
Formatting
================
Comment at: lib/Sema/SemaOpenMP.cpp:1178
+ // attribute,
+ // in case -fopenmp-implicit-declare-target extension is enabled.
+ ImplicitDeviceFunctionChecker FunctionCallChecker(SemaRef);
----------------
There is no such option yet.
================
Comment at: lib/Sema/SemaOpenMP.cpp:1191
+
+ if (FunctionDecl *FD = dyn_cast<FunctionDecl>(D)) {
+ if (FD->hasBody()) {
----------------
`auto *FD`
================
Comment at: lib/Sema/SemaOpenMP.cpp:1193
+ if (FD->hasBody()) {
+ for (auto RI : FD->redecls()) {
+ if (RI->hasAttr<OMPDeclareTargetDeclAttr>()) {
----------------
`const auto *RI`
================
Comment at: lib/Sema/SemaOpenMP.cpp:1194-1201
+ if (RI->hasAttr<OMPDeclareTargetDeclAttr>()) {
+ Attr *A = OMPDeclareTargetDeclAttr::CreateImplicit(
+ Context, OMPDeclareTargetDeclAttr::MT_To);
+ D->addAttr(A);
+
+ ImplicitDeclareTargetCheck(*this, FD);
+ return;
----------------
Could you reuse the same attribute attached to one of the redeclarations?
================
Comment at: lib/Sema/SemaOpenMP.cpp:1212-1214
+ Attr *A = OMPDeclareTargetDeclAttr::CreateImplicit(
+ SemaRef.Context, OMPDeclareTargetDeclAttr::MT_To);
+ Class->addAttr(A);
----------------
Reuse the existing attribute
================
Comment at: lib/Sema/SemaOpenMP.cpp:1233-1235
+ if (FunctionDecl *Callee = Call->getDirectCallee()) {
+ return VisitFunctionDecl(Callee);
+ }
----------------
Remove braces
================
Comment at: lib/Sema/SemaOpenMP.cpp:1245-1247
+ const RecordType *RT =
+ SemaRef.Context.getBaseElementType(Ty)->getAs<RecordType>();
+ CXXRecordDecl *RD = cast<CXXRecordDecl>(RT->getDecl());
----------------
`const auto *RD = SemaRef.Context.getBaseElementType(Ty)->getAsCXXRecordDecl();`
Repository:
rL LLVM
https://reviews.llvm.org/D38798
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
