[Openmp-commits] [PATCH] D137748: [openmp] [test] Fix data structure mismatches for tests that define kmp_depend_info

[Natalia Glagoleva via Phabricator via Openmp-commits]

natgla added inline comments.


================
Comment at: openmp/runtime/test/tasking/bug_nested_proxy_task.c:54
+        struct { // flag as a set of 8 bits
+            unsigned in : 1;
+            unsigned out : 1;
----------------
mstorsjo wrote:
> natgla wrote:
> > mstorsjo wrote:
> > > natgla wrote:
> > > > sorry for being late here. if the bit field is defined as unsigned, the set of bit fields will take at least "unsigned" space (4 bytes). Judging by union with flag: do you want to define all these as kmp_uint8? (same below)
> > > No, here, I want exactly this - and your comment is even more reason for it.
> > > 
> > > Here, we’re redefining an libomp internal struct, and exactly for rains like this, we really should have an exactly matching definition to the one in `kmp.h` in the source tree.
> > > 
> > > You’re right that we probably should change it like you’re suggesting - but then we should change `kmp.h` too, in sync with this.
> > would you mind adding a comment on kmp_depend_info explaining that it's exact copy of kmp_depend_info from kmp.h? (or if there are other structs here, more general comment?)
> Sure, I can do that.
> 
> In practice, I have no idea how this is meant to be used in practice - is this something that regular users of OpenMP are supposed to do, or is it just a intrusive test? But in any case, this brings the testsuite to passing in more cases.
looks like a unit test for runtime to me. It's emulating what compiler would generate (see comment at the top of the test)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137748