Follow up to safe API patch

[jlpeyton]

Summary

A while back, we made an initial change where dangerous C API functions were replaced with macros that translated the dangerous API function calls to safer function calls
e.g., sprintf() replaced with KMP_SPRINTF() which translates to sprintf_s() on Windows. Currently, the only operating system where this is applicable is Windows. Unix-like systems are still using the dangerous API e.g., KMP_SPRINTF() translates to sprintf(). Hal had brought up a couple concerns over performance:

+// _malloca was suggested, but it is not a drop-in replacement for _alloca
+# define KMP_ALLOCA _alloca

1. "Is the only difference the fact that the size is capped at _ALLOCA_S_THRESHOLD? Would you hit this limit? Are there performance implications to making the change regardless?"

-#define ngo_store_go(dst, src) memcpy((dst), (src), CACHE_LINE)
+#define ngo_store_go(dst, src) KMP_MEMCPY((dst), (src), CACHE_LINE)

2. "The other changes look fine, but this could have performance implications, no? The compiler can optimize away the memcpy, but likely not the memcpy_s, and the associated validation is likely expensive. Should we avoid this here?"

For 1) Yes, it hits the limit, and the runtime should not allocate memory from the heap. Currently performance is only affected on Windows because it is the only operating system using real safe API function calls.
For 2) That particular instance of memcpy only affects hierarchical barrier which is primarily used on Intel(R) MIC architectures. For memcpy() in general, it currently is only affected on Windows and will not affect performance for Unix users.

If users are worried about performance, a CMake options can be added which always maps the macros to the unsafe C function calls.

[jlpeyton]

*ping*

[jlpeyton]

*ping*

[AndreyChurbanov]

LGTM, as our testing showed no performance differences, the change looks safe.

openmp/trunk/runtime/src/kmp_barrier.cpp

 // ICV copying
 #define ngo_load(src)            __m512d Vt = _mm512_load_pd((void *)(src))
 #define ngo_store_icvs(dst, src) _mm512_storenrngo_pd((void *)(dst), Vt)
 #define ngo_store_go(dst, src)   _mm512_storenrngo_pd((void *)(dst), Vt)
 #define ngo_sync()               __asm__ volatile ("lock; addl $0,0(%%rsp)" ::: "memory")
 #else
 #define ngo_load(src)            ((void)0)
 #define ngo_store_icvs(dst, src) copy_icvs((dst), (src))
-#define ngo_store_go(dst, src)   memcpy((dst), (src), CACHE_LINE)
+#define ngo_store_go(dst, src)   KMP_MEMCPY((dst), (src), CACHE_LINE)
 #define ngo_sync()               ((void)0)
 #endif /* KMP_MIC && USE_NGO_STORES */
 
 void __kmp_print_structure(void); // Forward declaration
 
 // ---------------------------- Barrier Algorithms ----------------------------
 
 // Linear Barrier

openmp/trunk/runtime/src/kmp_safe_c_api.h

 
 // Not every unsafe call listed here is handled now, but keeping everything
 // in one place should be handy for future maintenance.
 #if KMP_OS_WINDOWS
 
 # define RSIZE_MAX_STR ( 4UL << 10 ) // 4KB
 
 // _malloca was suggested, but it is not a drop-in replacement for _alloca
-// TODO: test performance and replace with _alloca (as below)
-# define KMP_ALLOCA                  alloca
-//# define KMP_ALLOCA                  _alloca
+# define KMP_ALLOCA                  _alloca
 
 # define KMP_MEMCPY_S                memcpy_s
 # define KMP_SNPRINTF                sprintf_s
 # define KMP_SSCANF                  sscanf_s
 # define KMP_STRCPY_S                strcpy_s
 # define KMP_STRNCPY_S               strncpy_s
 
 // Use this only when buffer size is unknown