gtbercea marked an inline comment as done.
gtbercea added inline comments.

================
Comment at: lib/CodeGen/CGOpenMPRuntime.cpp:10364
+  if (const auto *FD = dyn_cast<FunctionDecl>(D)) {
+    if (OMPDeclareTargetDeclAttr::isDeclareTargetDeclaration(FD)) {
+      HasEmittedDeclareTargetRegion = true;
----------------
ABataev wrote:
> gtbercea wrote:
> > ABataev wrote:
> > > gtbercea wrote:
> > > > ABataev wrote:
> > > > > ABataev wrote:
> > > > > > No need for the braces
> > > > > What if `declare target` is used only for variabes but not for the 
> > > > > functions?
> > > > Even more reason to error in that case since it may contain clauses 
> > > > like link or to which need for requires directives to be used 
> > > > consistently.
> > > But I don't see that your patch works in this situation. Currently, it 
> > > will emit the error only if the declare target function is found, no?
> > Actually it will work even when just variables are used in the declare 
> > target region. There is another problem which I have a fix for. I will 
> > update the patch in a bit. 
> Why will it work in this case? If you have just a declare target region in 
> the code for the variables and nothing else. You don't have target regions, 
> declare target functions etc. It won't work in this case.
It will work because the OffloadEntriesInfoManager.empty() will return false in 
that case.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D60568/new/

https://reviews.llvm.org/D60568



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to