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

================
Comment at: lib/CodeGen/CGOpenMPRuntime.h:644
+  /// Flag for keeping track of weather a target region has been emitted.
+  bool hasEmittedTargetRegion = false;
+
----------------
ABataev wrote:
> gtbercea wrote:
> > gtbercea wrote:
> > > ABataev wrote:
> > > > gtbercea wrote:
> > > > > ABataev wrote:
> > > > > > Why do you need this? I think your function should be called 
> > > > > > without any conditions. It does not depend on the presence of the 
> > > > > > target regions or not. Plus, your check is not consistent if you're 
> > > > > > calling the function from another module, which has target region 
> > > > > > inside.
> > > > > This does not determine if the function is called or not. This helps 
> > > > > determine the flags with which the function is called.
> > > > It does not matter, it still does not work correctly if the target 
> > > > region is called in the function from another module
> > > If the target is in another compilation unit, that unit will need to have 
> > > a requires directive.
> > If you can come up with an example which you think doesn't work I'm happy 
> > to try it.
> If the module without target directives was compiled with unified memory and 
> the module with target directives compiled without unfied memory support? Is 
> this a problem? Shall we diagnose it?
The requires directives in a module without targets are just not taken into 
consideration. In general, a target region is needed before the requires flags 
are checked for compatibility with flags in other modules.


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