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

[David Pagan via Phabricator via cfe-commits]

ddpagan 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'.
----------------
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.


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

https://reviews.llvm.org/D157197