rjmccall added a comment. In https://reviews.llvm.org/D39947#922922, @mgrang wrote:
> In https://reviews.llvm.org/D39947#922889, @rjmccall wrote: > > > In https://reviews.llvm.org/D39947#922870, @mgrang wrote: > > > > > Although this patches fixes the above unit test failures, the generated > > > code is very different from the one that the tests expect. As a result, > > > these tests need to be adjusted. Could the reviewers please > > > comment/suggest on whether it is ok to fix the tests as a result of this > > > change? > > > > > > The other way of obtaining deterministic ordering for privates with the > > > same alignment is to use an index for each item inserted into Privates > > > and use it as a tie-breaker. But even in that case the generated code is > > > quite different and tests still need to be adjusted. > > > > > > Fixing the tests may be acceptable. Can you give an example of the > > difference between the old and new test outputs? > > > Please see https://www.diffchecker.com/7V2XFbk4 for the difference in output > for the following test before and after my change: > > clang -cc1 -internal-isystem <MYDIR>/build/llvm/lib/clang/6.0.0/include > -nostdsysteminc -verify -fopenmp -x c++ -triple x86_64-apple-darwin10 > -emit-llvm > <MYDIR>/src/llvm/tools/clang/test/OpenMP/task_firstprivate_codegen.cpp -o - > Does your diff have shuffling enabled on both sides? Neither layout for %struct..kmp_privates.t.3 seems to match the test's match for PRIVATES_TMAIN_TY, so I'm not completely sure which is supposed to be which. Assuming that the right diff is with your patch, something seems quite wrong, because the capture for t_var is being sorted to the end, which is producing a really terrible layout. I think you might actually have accidentally inverted the order: a qsort comparator is supposed to return positive if ``LHS > RHS``, so the fact that it's returning 1 when ``P1->first < P2->first`` means that it's actually a reversed comparison. Would you mind fixing that and then letting us know what test changes remain? Cou https://reviews.llvm.org/D39947 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
