On 11/10/2011 12:16 AM, Jakub Jelinek wrote:
> On Wed, Nov 09, 2011 at 04:32:58PM -0800, Richard Henderson wrote:
>> Not pretty at all. But given the corresponding irritation in writing
>> assembler
>> wrapper functions, it seems like it's about a wash.
>>
>> Tested with and without HAVE_AS_AVX on x86_64-linux.
> Shouldn't -mavx be also not passed in that case? Then you wouldn't need
> to undef __AVX__ and we wouldn't risk gcc doesn't decide to optimize memcpy
> or something similar using AVX instructions...
>
You are correct. Thanks for noticing this; I was a bit frazzled after fighting
with autofoo for so long yesterday.
Tested on x86_64-linux, with avx and with avx forcibly disabled.
r~
commit b8190fde2cd04078f8448576fb021060526b51d5
Author: rth <rth@138bc75d-0d04-0410-961f-82ee72b054a4>
Date: Thu Nov 10 17:09:04 2011 +0000
libitm: Don't add -mavx if the assembler doesn't support avx.
* config/x86/x86_avx.cc: Remove #undef __AVX__ hack. Tidy comments.
* Makefile.am (x86_avx.lo): Only add -mavx if ARCH_X86_AVX.
* configure.ac (ARCH_X86_AVX): New conditional.
* Makefile.in, configure: Rebuild.
git-svn-id: svn+ssh://gcc.gnu.org/svn/gcc/trunk@181261
138bc75d-0d04-0410-961f-82ee72b054a4
diff --git a/libitm/ChangeLog b/libitm/ChangeLog
index 8aeb589..4fb699e 100644
--- a/libitm/ChangeLog
+++ b/libitm/ChangeLog
@@ -1,3 +1,10 @@
+2011-11-10 Richard Henderson <r...@redhat.com>
+
+ * config/x86/x86_avx.cc: Remove #undef __AVX__ hack. Tidy comments.
+ * Makefile.am (x86_avx.lo): Only add -mavx if ARCH_X86_AVX.
+ * configure.ac (ARCH_X86_AVX): New conditional.
+ * Makefile.in, configure: Rebuild.
+
2011-11-09 Richard Henderson <r...@redhat.com>
* acinclude.m4 (LIBITM_CHECK_AS_AVX): New.
diff --git a/libitm/Makefile.am b/libitm/Makefile.am
index 4578986..b4674a5 100644
--- a/libitm/Makefile.am
+++ b/libitm/Makefile.am
@@ -48,6 +48,8 @@ libitm_la_SOURCES = \
if ARCH_X86
libitm_la_SOURCES += x86_sse.cc x86_avx.cc
x86_sse.lo : XCFLAGS += -msse
+endif
+if ARCH_X86_AVX
x86_avx.lo : XCFLAGS += -mavx
endif
diff --git a/libitm/Makefile.in b/libitm/Makefile.in
index 8816580..7426146 100644
--- a/libitm/Makefile.in
+++ b/libitm/Makefile.in
@@ -1259,7 +1259,7 @@ uninstall-am: uninstall-dvi-am uninstall-html-am
uninstall-info-am \
vpath % $(strip $(search_path))
@ARCH_X86_TRUE@x86_sse.lo : XCFLAGS += -msse
-@ARCH_X86_TRUE@x86_avx.lo : XCFLAGS += -mavx
+@ARCH_X86_AVX_TRUE@x86_avx.lo : XCFLAGS += -mavx
all-local: $(STAMP_GENINSRC)
diff --git a/libitm/config/x86/x86_avx.cc b/libitm/config/x86/x86_avx.cc
index cd20fe2..6a5e297 100644
--- a/libitm/config/x86/x86_avx.cc
+++ b/libitm/config/x86/x86_avx.cc
@@ -24,24 +24,20 @@
#include "config.h"
-// ??? This is pretty gross, but we're going to frob types of the functions.
-// Is this better or worse than just admitting we need to do this in pure
-// assembly?
-
-#ifndef HAVE_AS_AVX
-#undef __AVX__
-#endif
-
#include "libitm_i.h"
#include "dispatch.h"
extern "C" {
#ifndef HAVE_AS_AVX
+// If we don't have an AVX capable assembler, we didn't set -mavx on the
+// command-line either, which means that libitm.h defined neither this type
+// nor the functions in this file. Define the type and unconditionally
+// wrap the file in extern "C" to make up for the lack of pre-declaration.
typedef float _ITM_TYPE_M256 __attribute__((vector_size(32), may_alias));
#endif
-// ??? Re-define the memcpy implementations so that we can frob the
+// Re-define the memcpy implementations so that we can frob the
// interface to deal with possibly missing AVX instruction set support.
#ifdef HAVE_AS_AVX
@@ -52,10 +48,10 @@ typedef float _ITM_TYPE_M256
__attribute__((vector_size(32), may_alias));
#else
/* Emit vmovaps (%rax),%ymm0. */
#define RETURN(X) \
- asm volatile(".byte 0xc5,0xfc,0x28,0x00" : "=m"(X) : "a"(&X));
+ asm volatile(".byte 0xc5,0xfc,0x28,0x00" : "=m"(X) : "a"(&X))
/* Emit vmovaps %ymm0,(%rax); vzeroupper. */
#define STORE(X,Y) \
- asm volatile(".byte 0xc5,0xfc,0x29,0x00,0xc5,0xf8,0x77" : "=m"(X) : "a"(&X));
+ asm volatile(".byte 0xc5,0xfc,0x29,0x00,0xc5,0xf8,0x77" : "=m"(X) : "a"(&X))
#define OUTPUT(T) void
#define INPUT(T,X)
#endif
@@ -92,4 +88,4 @@ _ITM_LM256 (const _ITM_TYPE_M256 *ptr)
GTM::GTM_LB (ptr, sizeof (*ptr));
}
-}
+} // extern "C"
diff --git a/libitm/configure b/libitm/configure
index b30ced1..c0317cc 100644
--- a/libitm/configure
+++ b/libitm/configure
@@ -603,6 +603,8 @@ LTLIBOBJS
LIBOBJS
ARCH_FUTEX_FALSE
ARCH_FUTEX_TRUE
+ARCH_X86_AVX_FALSE
+ARCH_X86_AVX_TRUE
ARCH_X86_FALSE
ARCH_X86_TRUE
link_itm
@@ -11714,7 +11716,7 @@ else
lt_dlunknown=0; lt_dlno_uscore=1; lt_dlneed_uscore=2
lt_status=$lt_dlunknown
cat > conftest.$ac_ext <<_LT_EOF
-#line 11717 "configure"
+#line 11719 "configure"
#include "confdefs.h"
#if HAVE_DLFCN_H
@@ -11820,7 +11822,7 @@ else
lt_dlunknown=0; lt_dlno_uscore=1; lt_dlneed_uscore=2
lt_status=$lt_dlunknown
cat > conftest.$ac_ext <<_LT_EOF
-#line 11823 "configure"
+#line 11825 "configure"
#include "confdefs.h"
#if HAVE_DLFCN_H
@@ -17265,6 +17267,14 @@ else
ARCH_X86_FALSE=
fi
+ if test "$libitm_cv_as_avx" = yes; then
+ ARCH_X86_AVX_TRUE=
+ ARCH_X86_AVX_FALSE='#'
+else
+ ARCH_X86_AVX_TRUE='#'
+ ARCH_X86_AVX_FALSE=
+fi
+
if test $enable_linux_futex = yes; then
ARCH_FUTEX_TRUE=
ARCH_FUTEX_FALSE='#'
@@ -17418,6 +17428,10 @@ if test -z "${ARCH_X86_TRUE}" && test -z
"${ARCH_X86_FALSE}"; then
as_fn_error "conditional \"ARCH_X86\" was never defined.
Usually this means the macro was only invoked conditionally." "$LINENO" 5
fi
+if test -z "${ARCH_X86_AVX_TRUE}" && test -z "${ARCH_X86_AVX_FALSE}"; then
+ as_fn_error "conditional \"ARCH_X86_AVX\" was never defined.
+Usually this means the macro was only invoked conditionally." "$LINENO" 5
+fi
if test -z "${ARCH_FUTEX_TRUE}" && test -z "${ARCH_FUTEX_FALSE}"; then
as_fn_error "conditional \"ARCH_FUTEX\" was never defined.
Usually this means the macro was only invoked conditionally." "$LINENO" 5
diff --git a/libitm/configure.ac b/libitm/configure.ac
index 7de5cbe..672b712 100644
--- a/libitm/configure.ac
+++ b/libitm/configure.ac
@@ -272,6 +272,7 @@ fi
AC_SUBST(link_itm)
AM_CONDITIONAL([ARCH_X86], [test "$ARCH" = x86])
+AM_CONDITIONAL([ARCH_X86_AVX], [test "$libitm_cv_as_avx" = yes])
AM_CONDITIONAL([ARCH_FUTEX], [test $enable_linux_futex = yes])
AC_CONFIG_FILES(Makefile testsuite/Makefile libitm.spec)
