[PATCH] D109635: [WIP][OpenMP] Support construct trait set for Clang

[Johannes Doerfert via Phabricator via cfe-commits]

jdoerfert added inline comments.


================
Comment at: clang/lib/Sema/SemaOpenMP.cpp:746
+        }
+      }
+  }
----------------
jdoerfert wrote:
> So, SmalVector has a pop_back_val which we could use to verify the properties are the ones we expect. However, we would need to first put them into a vector and if insert is false we would need to reverse the vector.
> Maybe that's the way to go:
> 1) Where you call `handleConstructTrait` right now, just put the traits in a vector.
> 2) Make it `handleConstructTraits` and take a vector of construct traits.
> 3) For insert == true, just append them all.
> 4) For insert == falser reverse the vector and do pop_back_val with an assertion that they match the result of vector.pop_back_val();
> 
> Does that make sense?
> 
> (Apologies for making you rewrite this twice.)
This is not what I meant and I doubt this works. Let me try to explain with an example:

`handleDeclareVariantConstructTrait` for a DSKind `parallel_for` will first add a `parallel` then a `for` construct trait.
When we leave the `parallel for` we should pop them of our construct trait stack again, and preferably verify we do see the ones we expect.
So let's assume the stack was like this before

```
[ teams ]
// enter parallel for code generation
[ teams, parallel, for]
...
// exit parallel for code generation
```
at this point we want to pop `for` and then `parallel` from the end.

If the last one on the stack was not `for` or the second to last was not `parallel` something went wrong.
Since `handleDeclareVariantConstructTrait` does give use the traits "in order", thus [parallel, for], we need to do something different to verify this is the suffix of the stack.

I proposed to accumulate all traits in `handleDeclareVariantConstructTrait` rather than passing them to `handleConstructTrait` one by one.
So there would be a single call to `handleConstructTrait` at the end with all the traits, here [parallel, for].
If we are entering the scope we just append them to the stack: [teams] + [parallel, for] = [teams, parallel, for]
If we are exiting the scope (Insert == false, or name it ScopeEntry instead), we reverse the trait set and compare it against what we push from the stack:
  stack = [teams, parallel, for] 
  trait_set = [parallel, for] (passed to `handleConstructTrait` by `handleDeclareVariantConstructTrait) 
  verification:
    for (trait in reverse(trait_set)) {
      stack_last = stack.pop_back_val();
      assert(trait == stack_last && "Something left a trait on the stack!");
    }

Does this make sense?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109635