[PATCH] D157197: [clang][CodeGen][OpenMP] Fix if-clause for 'target teams loop'

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added inline comments.


================
Comment at: clang/lib/CodeGen/CGStmtOpenMP.cpp:1570-1575
+  // If we are here with a 'target teams loop' then we are emitting the
+  // 'parallel' region of the 'target teams distribute parallel for'
+  // emitted in place of the 'target teams loop'. Based on the specification
+  // noted above, an if-clause associated with a 'target teams loop', be it
+  // 'if(val)' or an 'if(target:val)', will apply only to 'target' and not
+  // the 'parallel' of the 'target teams distribute parallel for'.
----------------
ddpagan wrote:
> ABataev wrote:
> > It does not match the spec. 
> > ```
> > For a combined or composite construct, if no directive-name-modifier is specified then the if clause applies to all constituent constructs to which an if clause can apply.
> > ```
> > So, if(val) should be applied to both target and parallel regions, no?
> > It does not match the spec. 
> > ```
> > For a combined or composite construct, if no directive-name-modifier is specified then the if clause applies to all constituent constructs to which an if clause can apply.
> > ```
> > So, if(val) should be applied to both target and parallel regions, no?
> 
> Hi Alexey - Question for you: does revising the comment above at lines 1570-1575 to the following text help explain in a better way what's being done, and why?
> 
>   If we are handling a 'target teams distribute parallel for' explicitly written
>   in the source, and it has an 'if(val)' clause, the if condition is applied to
>   both 'target' and 'parallel' regions according to
>   OpenMP 5.2 [3.4, if Clause, Semantics, 15-18].
> 
>   However, if we are mapping an explicit 'target teams loop if(val)' onto a
>   'target teams distribute parallel for if(val)', to preserve the 'if' semantics
>   as specified by the user with the 'target teams loop', we apply it just to
>   the 'target' region.
It does not match the spec. Why we shall handle it this way?


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

https://reviews.llvm.org/D157197