This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH try 2] [i386,testsuite,libgcc] Fix build breakage on Mac and test FAILS on Solaris caused by PR82196 patch
On Sat, Sep 23, 2017 at 11:03 AM, Daniel Santos <daniel.santos@pobox.com> wrote:
> On 09/22/2017 06:50 AM, Uros Bizjak wrote:
>> On Fri, Sep 22, 2017 at 1:27 PM, Uros Bizjak <ubizjak@gmail.com> wrote:
>>> On Fri, Sep 22, 2017 at 12:28 PM, Daniel Santos <daniel.santos@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 ...
>> Please disregard the above... It is OK, since the code also handles sse.
>
> Also, a little bit of the complexity is due to the header being intended
> for use other than for these stubs. So I only define SSE_SAVE and
> SSE_RESTORE if (in essence) one of MS2SYSV_STUB_AVX or MS2SYSV_STUB_SSE
> are defined prior to the #include. I try to balance thinking ahead and
> genericity with trying not to make things overly complex for an
> imaginary, would-be future user -- I may not always succeed at striking
> that balance though. :)
>
>>
>>> +# 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.
>> + BYTE 0xc5, 0xf8, 0x28, 0x76, 0x60; /* vmovaps 0x60(%rsi),%xmm6 */
>> +# endif /* MOVAPS */
>> #endif /* defined (MS2SYSV_STUB_ISA) && defined (MOVAPS) */
>> #endif /* I386_ASM_H */
>>
>> Please update the #endif comment.
>>
>> Uros.
>
> Thanks, more uglies that I won't have to correct later!
The patch is OK for mainline, with the above comment fixed. Let's
unbreak Darwin build.
Thanks,
Uros.
- References:
- Re: [PATCH] [i386, libgcc] PR 82196 -mcall-ms2sysv-xlogues emits wrong AVX/SSE MOV
- From: Dominique d'Humières
- Re: [PATCH] [i386, libgcc] PR 82196 -mcall-ms2sysv-xlogues emits wrong AVX/SSE MOV
- Re: [PATCH] [i386, libgcc] PR 82196 -mcall-ms2sysv-xlogues emits wrong AVX/SSE MOV
- Re: [PATCH] [i386, libgcc] PR 82196 -mcall-ms2sysv-xlogues emits wrong AVX/SSE MOV
- Re: [PATCH] [i386, libgcc] PR 82196 -mcall-ms2sysv-xlogues emits wrong AVX/SSE MOV
- Re: [PATCH] [i386, libgcc] PR 82196 -mcall-ms2sysv-xlogues emits wrong AVX/SSE MOV
- Re: [PATCH] [i386, libgcc] PR 82196 -mcall-ms2sysv-xlogues emits wrong AVX/SSE MOV
- Re: [PATCH] [i386, libgcc] PR 82196 -mcall-ms2sysv-xlogues emits wrong AVX/SSE MOV
- Re: [PATCH] [i386, libgcc] PR 82196 -mcall-ms2sysv-xlogues emits wrong AVX/SSE MOV
- Re: [PATCH] [i386, libgcc] PR 82196 -mcall-ms2sysv-xlogues emits wrong AVX/SSE MOV
- [PATCH try 2] [i386,testsuite,libgcc] Fix build breakage on Mac and test FAILS on Solaris caused by PR82196 patch
- Re: [PATCH try 2] [i386,testsuite,libgcc] Fix build breakage on Mac and test FAILS on Solaris caused by PR82196 patch
- Re: [PATCH try 2] [i386,testsuite,libgcc] Fix build breakage on Mac and test FAILS on Solaris caused by PR82196 patch
- Re: [PATCH try 2] [i386,testsuite,libgcc] Fix build breakage on Mac and test FAILS on Solaris caused by PR82196 patch