This is an archive of the discontinued LLVM Phabricator instance.

Fixed https://bugs.llvm.org/show_bug.cgi?id=41584
ClosedPublic

Authored by AndreyChurbanov on May 15 2019, 6:54 AM.

Diff Detail

Repository
rOMP OpenMP

Event Timeline

tlwilmar accepted this revision.May 15 2019, 7:20 AM

LGTM!

This revision is now accepted and ready to land.May 15 2019, 7:20 AM

For me, this appears to fix my bug report's second reproducer, the one without the num_teams(3). Thanks!

It does not fix the first reproducer, but I applied on an old commit, r359012. I've pulled and will try again later today after the build finishes.

Simply removing a decrement seems very odd. Was it unnecessary in the first place, so did we decrement twice before?

Thanks, Joel.

I actually was not able to reproduce your first problem. But I hope the patch should fix both.

Johannes,

Your guess is true, - it was decremented twice, and with race condition. After "synchronized" decrement there are debug assertion that it is still non-negative. So, if all "broken" decrements happen after the synchronous ones, everything looked fine, otherwise the debug assertion triggered.

This revision was automatically updated to reflect the committed changes.

Thanks, Joel.

I actually was not able to reproduce your first problem. But I hope the patch should fix both.

I still see the assert fail from the first reproducer after applying your patch to r360778, which is from today. This patch seems good, so I'll continue that discussion in bugzilla....

Johannes,

Your guess is true, - it was decremented twice, and with race condition. After "synchronized" decrement there are debug assertion that it is still non-negative. So, if all "broken" decrements happen after the synchronous ones, everything looked fine, otherwise the debug assertion triggered.

Thx!