cchen added inline comments.
================
Comment at: clang/lib/Sema/SemaOpenMP.cpp:746
+ }
+ }
+ }
----------------
jdoerfert wrote:
> 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?
Thanks for your further explanation and patience, it really helps a lot for me
to understand!
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D109635/new/
https://reviews.llvm.org/D109635
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
