On Fri, Sep 22, 2017 at 12:28 PM, Daniel Santos <daniel.san...@pobox.com> wrote:
> On 09/22/2017 03:28 AM, Rainer Orth wrote:
>> Hi Daniel,
>>
>>> On 09/22/2017 02:18 AM, Rainer Orth wrote:
>>>> Hi Daniel,
>>>>
>>>>> On 09/21/2017 05:18 PM, Daniel Santos wrote:
>>>>>> So libgcc doesn't use a config.in. :(
>>>>> Scratch that, I forgot that we're using gcc/config.in via auto-host.h.
>>>>> So I only have to add this to gcc/configure.ac and it will be available
>>>>> for my libgcc header -- this is what I used to sniff out support for the
>>>>> .hidden directive.
>>>> Please don't go that route: it's totally the wrong direction.  There's
>>>> work going on to further decouple libgcc from gcc-private headers and
>>>> configure results.  libgcc already has its own configure tests for
>>>> assembler features, and its own config.in.  What's wrong with adapting
>>>> libitm's avx test in libitm/acinclude.m4 (LIBITM_CHECK_AS_AVX) for
>>>> libgcc?  Should be trivial...
>>>>
>>>>     Rainer
>>>>
>>> Oops, I just saw your email after submitting my other patch.  Yes, I am
>>> mistaken about config.in, sorry about that.  I didn't see a config.h
>>> file, but examining further it looks like it outputs to auto-target.h.
>>> Also, I was looking for some HAVE_AS* macros, but they are named
>>> differently.
>> Right: though some are for assembler features, the macros are named
>> differently.
>>
>>> I had previously included gcc's auto-host.h since it was in the include
>>> path in order to use HAVE_AS_HIDDEN, so in order to decouple this I'll
>> HAVE_GAS_HIDDEN actually ;-)
>>
>>> need to add that check into libgcc/configure.ac as well.  Again,
>>> shouldn't be that much code.  Sound sane to you?
>> You could do that, but it was already used before your patches, so
>> please separate it from the current issue if you go down that route.
>> libgcc is still full of cleanup possibilities :-)
>>
>>       Rainer
>
> OK, so I'm just adding HAVE_AS_AVX mostly as-is from libitm (we don't
> have $target_cpu so I'm using $target).  I do have minor concerns about
> how this test will work on a cross-build -- I'm not an autotools expert
> and I don't understand which assembler it will invoke, but the results
> of the test failing only means we use .byte instead of the real
> mnemonic, so it really shouldn't be a problem.
>
> I've got tests started again, so presuming that *this* one passes, is it
> OK for the trunk?
>
> gcc/testsuite:
>         * gcc.target/i386/pr82196-1.c: Simplify so that it doesn't break
>         on Solaris or with -mno-omit-frame-pointer.

No need to explain the change in the ChangeLog. Just say "(b): Remove
volatile asm."

>         * gcc.target/i386/pr82196-2.c: Likewise.
>
> libgcc:
>         * configure.ac: Add check for HAVE_AS_AVX.
>         * config.in: Regenerate.
>         * configure: Likewise.
>         * config/i386/i386-asm.h: Include auto-target.h from libgcc.
>         (SSE_SAVE, SSE_RESTORE): Sniff HAVE_AS_AVX and directly emit raw
>         .byte code when assembler doesn't support avx, correct
>         out-of-date comments.

(SSE_SAVE, SSE_RESTORE): Emit .byte sequence for !HAVE_AS_AVX.
Correct out-of-date comments.

>
>  gcc/testsuite/gcc.target/i386/pr82196-1.c |  5 ++-
>  gcc/testsuite/gcc.target/i386/pr82196-2.c |  5 ++-
>  libgcc/config.in                          |  3 ++
>  libgcc/config/i386/i386-asm.h             | 45 ++++++++++++++++++++++-----
>  libgcc/configure                          | 39 +++++++++++++++++++++++
>  libgcc/configure.ac                       | 16 ++++++++++
>  6 files changed, 100 insertions(+), 13 deletions(-)


>

 #ifdef MS2SYSV_STUB_AVX
 # define MS2SYSV_STUB_PREFIX __avx_
-# define MOVAPS vmovaps
+# ifdef HAVE_AS_AVX
+#  define MOVAPS vmovaps
+# endif

This is unecessarily complex. Please define MOVAPS unconditionaly, and ...

 #elif defined(MS2SYSV_STUB_SSE)
 # define MS2SYSV_STUB_PREFIX __sse_
 # define MOVAPS movaps
 #endif

-#if defined (MS2SYSV_STUB_PREFIX) && defined (MOVAPS)
+#if defined (MS2SYSV_STUB_PREFIX)

 # define MS2SYSV_STUB_BEGIN(base_name) \
  HIDDEN_FUNC(PASTE2(MS2SYSV_STUB_PREFIX, base_name))
@@ -83,8 +86,10 @@ ASMNAME(fn):
 # define MS2SYSV_STUB_END(base_name) \
  FUNC_END(PASTE2(MS2SYSV_STUB_PREFIX, base_name))

-/* Save SSE registers 6-15. off is the offset of rax to get to xmm6.  */
-# define SSE_SAVE   \
+/* If expanding for sse or avx and we have assembler support.  */
+# ifdef MOVAPS

... check HAVE_AS_AVX here.

+/* Save SSE registers 6-15 using rax as the base address.  */
+#  define SSE_SAVE   \
  MOVAPS %xmm15,-0x30(%rax); \


+#  define BYTE .byte
+#  define SSE_SAVE    \
+ BYTE 0xc5, 0x78, 0x29, 0x78, 0xd0; /* vmovaps %xmm15,-0x30(%rax) */ \

Is there a reason for BYTE definition? Every known assembler supports
.byte directive.

Uros.

Reply via email to