Page MenuHomePhabricator

[OpenMP][OMPT] fix reduction test for 32-bit x86
ClosedPublic

Authored by protze.joachim on Feb 2 2020, 10:03 AM.

Details

Summary

fix for: https://bugs.llvm.org/show_bug.cgi?id=44733

function __kmp_determine_reduction_method has strange architecture specific optimizations to determine the reduction algorithm. For 32-bit we need at least 3 variables to avoid atomic reduction.

This patch adds reduction variables to the testcase.

see: llvm-project/openmp/runtime/src/kmp_runtime.cpp:8143

Diff Detail

Event Timeline

protze.joachim created this revision.Feb 2 2020, 10:03 AM
Herald added a project: Restricted Project. · View Herald Transcript
Herald added subscribers: jfb, guansong. · View Herald Transcript

While it fixes i386 tests, it seems to break amd64 now. Do you need me to paste the output or can you reproduce?

@protze.joachim for 64 bit, we still need at least 5 threads IIRC

openmp/runtime/test/ompt/synchronization/reduction/tree_reduce.c
1–3

Do we need %sort-threads? It would be good to have either have both sorted, or none.

Thanks @Hahnfeld for pointing into the right direction.

Hahnfeld accepted this revision.Feb 3 2020, 10:11 AM

LGTM from the code, I didn't test that the issue is indeed fixed on i386 though

This revision is now accepted and ready to land.Feb 3 2020, 10:11 AM
mgorny accepted this revision.Feb 3 2020, 12:06 PM

I can confirm the tests pass both on i386 and amd64 for me, with this patch.

This revision was automatically updated to reflect the committed changes.