-
-
Notifications
You must be signed in to change notification settings - Fork 6.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
undefined reference to `__builtin_ia32_pause' with icc #9081
Comments
And here is the |
Stupid compilers trying to look like gcc but isn't compatible with gcc... So something like this? diff --git a/lib/easy_lock.h b/lib/easy_lock.h
index a4db9fe47..bcd4353b3 100644
--- a/lib/easy_lock.h
+++ b/lib/easy_lock.h
@@ -47,11 +47,13 @@
#ifndef __has_builtin
#define __has_builtin(x) 0
#endif
/* if GCC on i386/x86_64 or if the built-in is present */
-#if ( (defined(__GNUC__) && !defined(__clang__)) && \
+#if ( (defined(__GNUC__) && \
+ !defined(__clang__) && \
+ !defined(__INTEL_COMPILER)) && \
(defined(__i386__) || defined(__x86_64__))) || \
__has_builtin(__builtin_ia32_pause)
#define HAVE_BUILTIN_IA32_PAUSE
#endif |
Hmm. Nope:
but maybe I have the patch wrong. Here is what I have versus 7.84.0: diff --git a/lib/easy_lock.h b/lib/easy_lock.h
index 819f50ce8..16c7110fb 100644
--- a/lib/easy_lock.h
+++ b/lib/easy_lock.h
@@ -40,6 +40,20 @@
#define curl_simple_lock atomic_bool
#define CURL_SIMPLE_LOCK_INIT false
+/* a clang-thing */
+#ifndef __has_builtin
+#define __has_builtin(x) 0
+#endif
+
+/* if GCC on i386/x86_64 or if the built-in is present */
+#if ( (defined(__GNUC__) && \
+ !defined(__clang__) && \
+ !defined(__INTEL_COMPILER)) && \
+ (defined(__i386__) || defined(__x86_64__))) || \
+ __has_builtin(__builtin_ia32_pause)
+#define HAVE_BUILTIN_IA32_PAUSE
+#endif
+
static inline void curl_simple_lock_lock(curl_simple_lock *lock)
{
for(;;) {
@@ -48,7 +62,7 @@ static inline void curl_simple_lock_lock(curl_simple_lock *lock)
/* Reduce cache coherency traffic */
while(atomic_load_explicit(lock, memory_order_relaxed)) {
/* Reduce load (not mandatory) */
-#if defined(__i386__) || defined(__x86_64__)
+#ifdef HAVE_BUILTIN_IA32_PAUSE
__builtin_ia32_pause();
#elif defined(__aarch64__)
asm volatile("yield" ::: "memory"); |
I suppose then that your compiler doesn't define |
No, it does: ❯ cat helloworld.c
#include <stdio.h>
int main() {
printf("Hello, World!\n");
#ifdef __INTEL_COMPILER
printf("I run Intel!\n");
#endif
return 0;
}
❯ icc helloworld.c && ./a.out
Hello, World!
I run Intel!
❯ clang helloworld.c && ./a.out
Hello, World!
❯ |
Ok, but can you understand then why my proposed #ifdef-thing didn't work? |
Well, it looks like ❯ cat curl_c_oddity.c
#include <stdio.h>
int main() {
#ifndef __has_builtin
#define __has_builtin(x) 0
#endif
#if defined(__INTEL_COMPILER)
printf("__INTEL_COMPILER\n");
#endif
#if defined(__GNUC__)
printf("__GNUC__\n");
#endif
#if defined(__clang__)
printf("__clang__\n");
#endif
#if __has_builtin(__builtin_ia32_pause)
printf("__builtin_ia32_pause: %d\n", __has_builtin(__builtin_ia32_pause));
#endif
#if ( (defined(__GNUC__) && \
!defined(__clang__) && \
!defined(__INTEL_COMPILER)) && \
(defined(__i386__) || defined(__x86_64__))) || \
__has_builtin(__builtin_ia32_pause)
printf("HAVE_BUILTIN_IA32_PAUSE\n");
#else
printf("Nope\n");
#endif
return 0;
}
$ icc curl_c_oddity.c && ./a.out
__INTEL_COMPILER
__GNUC__
__builtin_ia32_pause: 1
HAVE_BUILTIN_IA32_PAUSE And on macOS: ❯ icc curl_c_oddity.c&& ./a.out
__INTEL_COMPILER
__GNUC__
__clang__
__builtin_ia32_pause: 1
HAVE_BUILTIN_IA32_PAUSE I think Intel C/C++ define a lot of GNU like macros underneath. Weird thing is it's saying that __builtin_ia32_pause is defined. 🤷🏼 When/If you run Intel C locally, do you see this odd behavior (not sure if you have an Intel oneAPI test in your CI)? Or is it somehow due to how it's installed on my machines? |
This can split out all the built-ins: |
@gvanem Well,
and Linux was much the same. If I remove
|
My Intel Compiler is clang/LLVM based, so no |
@gvanem Ah. Yes, I'm not using |
We don't have any Intel compilers in the CI builds and I don't have it installed anywhere locally either. Clearly this problem is either a problem in your install or a bug in the compiler. If the latter, I presume we should work around it. Maybe like this? --- a/lib/easy_lock.h
+++ b/lib/easy_lock.h
@@ -59,17 +59,23 @@ typedef PVOID SRWLOCK, *PSRWLOCK;
/* a clang-thing */
#ifndef __has_builtin
#define __has_builtin(x) 0
#endif
+#ifndef __INTEL_COMPILER
+/* The Intel compiler tries to look like GCC *and* clang *and* lies in its
+ __has_builtin() function, so override it. */
+
/* if GCC on i386/x86_64 or if the built-in is present */
#if ( (defined(__GNUC__) && !defined(__clang__)) && \
(defined(__i386__) || defined(__x86_64__))) || \
__has_builtin(__builtin_ia32_pause)
#define HAVE_BUILTIN_IA32_PAUSE
#endif
+#endif
+
static inline void curl_simple_lock_lock(curl_simple_lock *lock)
{
for(;;) {
if(!atomic_exchange_explicit(lock, true, memory_order_acquire))
break; |
Well...something happened, but maybe I did things wrong. I have this difference from 7.84.0: --- a/lib/easy_lock.h
+++ b/lib/easy_lock.h
@@ -40,6 +40,24 @@
#define curl_simple_lock atomic_bool
#define CURL_SIMPLE_LOCK_INIT false
+/* a clang-thing */
+#ifndef __has_builtin
+#define __has_builtin(x) 0
+#endif
+
+#ifndef __INTEL_COMPILER
+/* The Intel compiler tries to look like GCC *and* clang *and* lies in its
+ __has_builtin() function, so override it. */
+
+/* if GCC on i386/x86_64 or if the built-in is present */
+#if ( (defined(__GNUC__) && !defined(__clang__)) && \
+ (defined(__i386__) || defined(__x86_64__))) || \
+ __has_builtin(__builtin_ia32_pause)
+#define HAVE_BUILTIN_IA32_PAUSE
+#endif
+
+#endif
+
static inline void curl_simple_lock_lock(curl_simple_lock *lock)
{
for(;;) {
@@ -48,7 +66,7 @@ static inline void curl_simple_lock_lock(curl_simple_lock *lock)
/* Reduce cache coherency traffic */
while(atomic_load_explicit(lock, memory_order_relaxed)) {
/* Reduce load (not mandatory) */
-#if defined(__i386__) || defined(__x86_64__)
+#ifdef HAVE_BUILTIN_IA32_PAUSE
__builtin_ia32_pause();
#elif defined(__aarch64__)
asm volatile("yield" ::: "memory"); and now the error is:
Did I maybe miss something in my changes? |
Note at the end of my
so it's like |
Hmm. And I see a similar issue in #9136 |
Ahhh. That got it! With this: --- a/lib/easy_lock.h
+++ b/lib/easy_lock.h
@@ -36,10 +36,31 @@
#elif defined (HAVE_ATOMIC)
#include <stdatomic.h>
+#if defined(HAVE_SCHED_YIELD)
+#include <sched.h>
+#endif
#define curl_simple_lock atomic_bool
#define CURL_SIMPLE_LOCK_INIT false
+/* a clang-thing */
+#ifndef __has_builtin
+#define __has_builtin(x) 0
+#endif
+
+#ifndef __INTEL_COMPILER
+/* The Intel compiler tries to look like GCC *and* clang *and* lies in its
+ __has_builtin() function, so override it. */
+
+/* if GCC on i386/x86_64 or if the built-in is present */
+#if ( (defined(__GNUC__) && !defined(__clang__)) && \
+ (defined(__i386__) || defined(__x86_64__))) || \
+ __has_builtin(__builtin_ia32_pause)
+#define HAVE_BUILTIN_IA32_PAUSE
+#endif
+
+#endif
+
static inline void curl_simple_lock_lock(curl_simple_lock *lock)
{
for(;;) {
@@ -48,7 +69,7 @@ static inline void curl_simple_lock_lock(curl_simple_lock *lock)
/* Reduce cache coherency traffic */
while(atomic_load_explicit(lock, memory_order_relaxed)) {
/* Reduce load (not mandatory) */
-#if defined(__i386__) || defined(__x86_64__)
+#ifdef HAVE_BUILTIN_IA32_PAUSE
__builtin_ia32_pause();
#elif defined(__aarch64__)
asm volatile("yield" ::: "memory"); I have a Thanks! (NB: If it would be useful to add Intel to your CI system, I've done that in projects before and it's helped us out with weird Intel things. That said, the curl CI seems to be pretty comprehensive, so I'm not sure where to start without some guidance. 😄 ) |
Oh yes, I think it would indeed be very useful! Would you like to make an attempt? |
The Intel compiler tries to look like GCC *and* clang *and* it lies in its __has_builtin() function (returns true when it should return false), so override it. Fixes #9081 Reported-by: Matthew Thompson
I'll give it a go! |
@bagder Well, good news is I am close. Bad news is the CI build is dying. Weird thing is, it's dying before the part where I'd expect (aka at link since #9144 isn't in). Namely it is doing:
https://github.com/mathomp4/curl/runs/7306769953?check_suite_focus=true I have a new job in the queue that should die the same way (I didn't quite follow the curl CI style, so I just renamed some things). |
... as the only caller that cares about what it returns assumes that anyway. This caused icc to warn: asyn-thread.c(505): error #188: enumerated type mixed with another type result = getaddrinfo_complete(data); Repoorted-by: Matthew Thompson Bug: #9081 (comment)
I filed #9146 for that. |
... as the only caller that cares about what it returns assumes that anyway. This caused icc to warn: asyn-thread.c(505): error #188: enumerated type mixed with another type result = getaddrinfo_complete(data); Repoorted-by: Matthew Thompson Bug: #9081 (comment) Closes #9146
Adding a new version of curl. This addresses issue [9081](curl/curl#9081).
Adding a new version of curl. This addresses issue [9081](curl/curl#9081).
Adding a new version of curl. This addresses issue [9081](curl/curl#9081).
Adding a new version of curl. This addresses issue [9081](curl/curl#9081).
I did this
Build curl with Intel compilers
I expected the following
Successful compile
curl/libcurl version
curl 7.84.0
operating system
SLES 12 SP5
Intel oneAPI 2021.3.0 using the "classic" compilers (
icc
,icpc
,ifort
)What happened
When I tried to build curl with
icc
I got some errors at thecurl
link step:I did see there was a similar issue reported in #9058, so I tried grabbing the changes from #9062 but nothing changed. I think I might need a similar workaround for Intel C.
The text was updated successfully, but these errors were encountered: