Andrew Stubbs <a...@codesourcery.com> writes:
> On 17/09/18 09:40, Richard Sandiford wrote:
>> <a...@codesourcery.com> writes:
>>> This is an update of the patch posted to PR82089 long ago. We ran into the
>>> same bug on GCN, so we need this fixed as part of this series.
>>>
>>> 2018-09-05 Andrew Stubbs <a...@codesourcery.com>
>>> Tom de Vries <t...@codesourcery.com>
>>>
>>> PR82089
>>>
>>> gcc/
>>> * expmed.c (emit_cstore): Fix handling of result_mode == BImode and
>>> STORE_FLAG_VALUE == 1.
>>> ---
>>> gcc/expmed.c | 15 +++++++++++----
>>> 1 file changed, 11 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/gcc/expmed.c b/gcc/expmed.c
>>> index 29ce10b..0b87fdc 100644
>>> --- a/gcc/expmed.c
>>> +++ b/gcc/expmed.c
>>> @@ -5464,11 +5464,18 @@ emit_cstore (rtx target, enum insn_code icode, enum
>>> rtx_code code,
>>> If STORE_FLAG_VALUE does not have the sign bit set when
>>> interpreted in MODE, we can do this conversion as unsigned, which
>>> is usually more efficient. */
>>> - if (GET_MODE_SIZE (int_target_mode) > GET_MODE_SIZE (result_mode))
>>> + if (GET_MODE_SIZE (int_target_mode) > GET_MODE_SIZE (result_mode)
>>> + || (result_mode == BImode && int_target_mode != BImode))
>>
>> Would be better to test GET_MODE_PRECISION instead of GET_MODE_SIZE,
>> if that works, instead of treating BImode as a special case.
>>
>>> {
>>> - convert_move (target, subtarget,
>>> - val_signbit_known_clear_p (result_mode,
>>> - STORE_FLAG_VALUE));
>>> + gcc_assert (GET_MODE_SIZE (result_mode) != 1
>>> + || STORE_FLAG_VALUE == 1 || STORE_FLAG_VALUE == -1);
>>> + bool unsignedp
>>> + = (GET_MODE_SIZE (result_mode) == 1
>>> + ? STORE_FLAG_VALUE == 1
>>> + : val_signbit_known_clear_p (result_mode, STORE_FLAG_VALUE));
>>> +
>>> + convert_move (target, subtarget, unsignedp);
>>> +
>>
>> GET_MODE_SIZE == 1 would also trigger for QImode, which shouldn't be treated
>> differently from HImode etc.
>>
>> The original val_signbit_known_clear_p test seems like it might be an
>> abstraction too far. In practice STORE_FLAG_VALUE has to fit within
>> the mode of a natural (unextended) condition result, so I think we can
>> simply test STORE_FLAG_VALUE >= 0 for all modes to see whether the target
>> wants the result to be treated as signed or unsigned.
>
> How about the attached?
>
> I think I addressed all your comments, and it tests fine on GCN with no
> regressions.
>
> Andrew
>
> [pr82089] Don't sign-extend SFV 1 in BImode
>
> This is an update of the patch posted to PR82089 long ago. We ran into the
> same bug on GCN, so we need this fixed as part of this series.
>
> 2018-09-26 Andrew Stubbs <a...@codesourcery.com>
> Tom de Vries <t...@codesourcery.com>
>
> PR82089
>
> gcc/
> * expmed.c (emit_cstore): Fix handling of result_mode == BImode and
> STORE_FLAG_VALUE == 1.
OK, thanks.
Richard
