[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:4364
+            *this, IRPosition::function(*ParallelRegion), DepClassTy::OPTIONAL);
+        if (!FnAA.ReachedKnownParallelRegions.empty() ||
+            !FnAA.ReachedUnknownParallelRegions.empty())
----------------
josemonsalve2 wrote:
> jdoerfert wrote:
> > josemonsalve2 wrote:
> > > 
> > > ```
> > > NestedParallelism = !FnAA.ReachedKnownParallelRegions.empty() || !FnAA.ReachedUnknownParallelRegions.empty()
> > > ```
> > If you change it, then `|=`, the current way is OK too.
> > 
> > One thing to add though: `!FnAA.getState().isValidState() || ...`
> Rafa. I believe this comment by johannes has not been solved. The the if statement before the assignment. It can also be added to this line. 
I think what he meant was to check if the AA was a valid state before reading 
ReachedKnownParallelRegions and 
ReachedUnknownParallelRegions. I can also add it to the same line like this:

```
NestedParallelism |= (!FnAA.ReachedKnownParallelRegions.empty() || !FnAA.ReachedUnknownParallelRegions.empty()) && FnAA.getState().isValidState()
```

Johannes, please correct me if I’m wrong. 


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

https://reviews.llvm.org/D141010