Page MenuHomePhabricator

Fix for OMP doacross implementation on 32-bit platforms
ClosedPublic

Authored by AndreyChurbanov on Nov 17 2017, 3:27 AM.

Diff Detail

Repository
rL LLVM

Event Timeline

Do we need all volatile casts that force the different code paths for 32 and 64 bit? In my experiments on Power it worked without them...

runtime/src/kmp_csupport.cpp
3809 ↗(On Diff #123312)

Should we introduce KMP_COMPARE_AND_STORE_RETPTR for this?

runtime/src/kmp_csupport.cpp
3809 ↗(On Diff #123312)

This is the only place in the code where CAS on pointer is interested in exact return value (to perform lock-less shared allocation), so the new macro won't help much I think. So I'd vote to leave it as is.

hbae accepted this revision.Nov 20 2017, 7:40 AM

LGTM

This revision is now accepted and ready to land.Nov 20 2017, 7:40 AM

Do we need all volatile casts that force the different code paths for 32 and 64 bit? In my experiments on Power it worked without them...

Forgot to answer this point. The second cast to volatile (in while statement) used to force the compiler to re-read the value of doacross_flags pointer at each iteration. The first cast (in CAS statement) just repeats the same cast encoded into the macros. So it is not technically necessary, but used here just in case we may some time decide to change the macros, and then this cast will work.

Hahnfeld accepted this revision.Nov 20 2017, 7:58 AM

Thanks for the explanation

This revision was automatically updated to reflect the committed changes.