[PATCH] D141010: OpenMPOpt: Check nested parallelism in target region

[Rafael Herrera via Phabricator via llvm-commits]

randreshg added inline comments.


================
Comment at: llvm/lib/Transforms/IPO/OpenMPOpt.cpp:602
+  /// Flag that indicates if the kernel has nested Parallelism
+  bool NestedParallelism = false;
+
----------------
tianshilei1992 wrote:
> Is it good enough to just have two states? Is it possible that we can't tell if a parallel region is nested or not, or do we just conservatively take it as `true`?
What others states we might possibly have? The only way we can't tell if a parallel region is nested or not is when we have functions with parallel regions in another TU, for this case we conservatively take it as `false`, for all the other cases this approach should work.


================
Comment at: llvm/lib/Transforms/IPO/OpenMPOpt.cpp:4369
+            !FnAA.ReachedUnknownParallelRegions.empty();
+        if (mayContainParRegion)
+          NestedParallelism = true;
----------------
josemonsalve2 wrote:
> josemonsalve2 wrote:
> > Any reason you want to keep this variable? Why not add the condition directly into the ID?
> Or probably better make NesterParallelism equal you the condition. 
Not really, we can remove the variable and add it to the IF statement


Repository:
  rOMP OpenMP

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

https://reviews.llvm.org/D141010