This is an archive of the discontinued LLVM Phabricator instance.

Add non-SSE wrapper for __kmp_{load,store}_mxcsr
ClosedPublic

Authored by dim on Apr 19 2019, 1:28 PM.

Details

Summary

To be able to successfully build OpenMP on FreeBSD/i386, which still
uses i486 as its default processor, I had to provide wrappers for the
__kmp_load_mxcsr and __kmp_store_mxcsr functions.

If the compiler signals that SSE is not available, loading and storing
mxcsr does not make sense anway, so in that case the inline functions
are empty. This gives the minimum amount of code churn.

See also https://svnweb.freebsd.org/changeset/base/345283

Event Timeline

dim created this revision.Apr 19 2019, 1:28 PM
hfinkel added a subscriber: hfinkel.May 1 2019, 6:34 AM

Does the code that uses __kmp_load_mxcsr expect the pointed-to value to be initialized?

dim added a comment.May 1 2019, 6:54 AM

Does the code that uses __kmp_load_mxcsr expect the pointed-to value to be initialized?

Hm, that's a good one. I expect that callers will just save the value somewhere in a struct, to restore it later via __kmp_load_mxcsr, which is a nop. But it is probably best to set the pointed-to value to zero, just to be safe.

jlpeyton added inline comments.May 1 2019, 9:15 AM
runtime/src/kmp.h
1259

This needs to play well with Windows. Can we stick these in the above operating system split section:

#if KMP_OS_UNIX
// Code for __kmp_x86_cpuid, __kmp_load_x87...
// Your current patch 
#else 
// Windows still has these as external function in assembly file
...
static inline void __kmp_load_mxcsr(const kmp_uint32 *p) { _mm_setcsr(*(p)); }
static inline void __kmp_store_mxcsr(kmp_uint32 *p) { *p = _mm_getcsr(); }
#endif // KMP_OS_UNIX
dim updated this revision to Diff 197619.May 1 2019, 12:53 PM

Address review comments:

  • Assign zero to pointed-to value in __kmp_store_mxcsr()
  • Use SSE specific stuff in KMP_OS_UNIX part only
dim added a comment.May 4 2019, 5:58 AM

So does this look better now?

jlpeyton accepted this revision.May 6 2019, 8:25 AM

LGTM

This revision is now accepted and ready to land.May 6 2019, 8:25 AM
This revision was automatically updated to reflect the committed changes.