On 6/13/24 9:26 PM, Kewen.Lin wrote:
> on 2024/6/13 21:24, Peter Bergner wrote:
>> On 6/13/24 12:35 AM, Kewen.Lin wrote:
>>>> @@ -826,7 +826,14 @@ rs6000_stack_info (void)
>>>> info->ehrd_offset -= info->rop_hash_size;
>>>> }
>>>> else
>>>> - info->ehrd_offset = info->gp_save_offset - ehrd_size;
>>>> + {
>>>> + info->ehrd_offset = info->gp_save_offset - ehrd_size;
>>>> +
>>>> + /* Adjust for ROP protection. */
>>>> + info->rop_hash_save_offset
>>>> + = info->gp_save_offset - info->rop_hash_size;
>>>> + info->ehrd_offset -= info->rop_hash_size;
>>>> + }
>>>
>>> I understand this is just copied from the if arm, but if I read this right,
>>> it can be
>>> simplified as:
>>
>> Ok, I'll retest with that simplification.
So I retested a normal powerpc64le-linux build (ie, we default to Power8
with Altivec) and it bootstrapped and regtested with no regressions.
I then attempted a --with-cpu=power5 build to test the non-altivec path,
but both the unpatched and patched builds died building libgfortran with
the following error: "error: ‘_Float128’ is not supported on this target".
I believe that is related to PR113652. I'll kick off the build again,
this time disabling Fortran and seeing if the build completes.
>>>> +/* { dg-do assemble } */
>>>> +/* { dg-options "-O2 -mdejagnu-cpu=power10 -mrop-protect -mno-vsx
>>>> -mno-altivec -mabi=no-altivec -save-temps" } */
>>>
>>> I'd expect -mabi=no-altivec is default for -mno-altivec, but specifying it
>>> explicitly
>>> looks fine to me. :)
>>
>> That's what I expected too! :-) However, I was surprised to learn that
>> -mno-altivec
>> does *not* disable TARGET_ALTIVEC_ABI. I had to explicitly use the -mabi=
>> option to
>> expose the bug.
>
> oh, it's surprising, I learn something today! :) I guess it's not intentional
> but just no
> one noticed it, as it seems nonsense to have altivec ABI extension but not
> using any altivec
> features.
Agreed!
Peter
