Re: [PATCH] x86-64: Compile branred.c with -mprefer-vector-width=128 [BZ #24603]

[Jeff Law <law at redhat dot com>]

On 6/17/19 1:06 PM, H.J. Lu wrote:
> On Fri, Jun 14, 2019 at 9:43 AM Jeff Law <law@redhat.com> wrote:
>>
>> On 6/14/19 7:58 AM, Florian Weimer wrote:
>>> * Florian Weimer:
>>>
>>>> * H. J. Lu:
>>>>
>>>>> On Tue, Jun 11, 2019 at 9:07 AM H.J. Lu <hjl.tools@gmail.com> wrote:
>>>>>>
>>>>>> On Tue, Jun 11, 2019 at 9:02 AM Florian Weimer <fweimer@redhat.com> wrote:
>>>>>>>
>>>>>>> * H. J. Lu:
>>>>>>>
>>>>>>>> Fixed.  I also added check for -mprefer-vector-width=128 which is new in GCC 8.
>>>>>>>> GCC 7 doesn't have this problem since it doesn't vectorize the second loop.
>>>>>>>>
>>>>>>>> Here is the updated patch.
>>>>>>>>
>>>>>>>> OK for master?
>>>>>>>
>>>>>>> I still don't understand why we need a workaround for a GCC performance
>>>>>>> bug in glibc.  Is there any precedent for this?
>>>>>>>
>>>>>>
>>>>>> I don't know if we ever investigated performance impacts of different GCC
>>>>>> optimizations on glibc.
>>>>>>
>>>>>
>>>>> I found:
>>>>>
>>>>> sysdeps/powerpc/powerpc64/power4/Makefile:CFLAGS-memmove.c += --param
>>>>> max-variable-expansions-in-unroller=2 --param max-unroll-times=2
>>>>> -funroll-loops -fpeel-loops
>>>>
>>>> That's been there since before 2012.  This reflects my main concern
>>>> about such tweaks: That they will never be reviewed and removed when
>>>> they are no longer necessary (and probably have negative impact).
>>>
>>> For the record, this is not a strong objection to your patch, H.J.
>> So one way to address this would be to somehow conditionalize the bits
>> on the GCC version where they were an issue.  If the version number is
>> higher then issue an error of some kind which would force someone to
>> look at the problem again.
>>
> We know GCC 8 and 9 have this issue.  But we don't know when this issue
> will be fixed in GCC and we should allow building glibc with GCC newer than
> the current minimum version, including GCC 10.  How should GCC version
> be checked here?
ISTM that 8, 9, 10 would use the new flag.  11 would issue an error
which would trigger a reinvestigation roughly a year from now.

The glibc maintainers would have the final say about this -- it's just
an idea off the top of my head.

jeff
>