Re: [PATCH 2/2] Enable elimination of zext/sext

[Richard Biener <richard.guenther@gmail.com>]

On Tue, Sep 9, 2014 at 12:06 PM, Kugan
<kugan.vivekanandarajah@linaro.org> wrote:
>
>
> On 08/09/14 19:48, Richard Biener wrote:
>> On Sun, Sep 7, 2014 at 11:50 AM, Kugan
>> <kugan.vivekanandarajah@linaro.org> wrote:
>>> On 05/09/14 19:50, Richard Biener wrote:
>>>
>>>> Well - the best way would be to expose the target specifics to GIMPLE
>>>> at some point in the optimization pipeline.  My guess would be that it's
>>>> appropriate after loop optimizations (but maybe before induction variable
>>>> optimization).
>>>>
>>>> That is, have a pass that applies register promotion to all SSA names
>>>> in the function, inserting appropriate truncations and extensions.  That
>>>> way you'd never see (set (subreg...) on RTL.  The VRP and DOM
>>>> passes running after that pass would then be able to aggressively
>>>> optimize redundant truncations and extensions.
>>>>
>>>> Effects on debug information are to be considered.  You can change
>>>> the type of SSA names in-place but you don't want to do that for
>>>> user DECLs (and we can't have the SSA name type and its DECL
>>>> type differ - and not sure if we might want to lift that restriction).
>>>
>>> Thanks. I will try to implement this.
>>>
>>> I still would like to keep the VRP based approach as there are some
>>> cases that I think can only be done with range info. For example:
>>>
>>> short foo(unsigned char c)
>>> {
>>>   c = c & (unsigned char)0x0F;
>>>   if( c > 7 )
>>>     return((short)(c - 5));
>>>   else
>>>     return(( short )c);
>>> }
>>>
>>>
>>> So, how about adding and setting the overflow/wrap around flag to
>>> range_info. We now set static_flag for VR_RANG/VR_ANTI_RANGE. If we go
>>> back to the max + 1, min - 1 for VR_ANTI_RANGE, we can use this
>>> static_flag to encode overflow/wrap around. Will that be something
>>> acceptable?
>>
>> You mean tracking in the VRP lattice whether a value wrapped around
>> (or was assumed not to due to undefined behavior)?  I'm not sure this
>> is easy to do correctly (VRP is large).
>>
>> Note that I don't think we'll lose the testcase you quoted if the promotion
>> pass runs before VRP2.   We'd have as input to VRP2 sth like (assuming
>> promote mode would promote to SImode)
>>
>>   SImode tem_2 = (unsigned int)c_1(D);
>>   tem_3 = tem_3 & 0xF;
>>   if (tem_3 > 7)
>>     {
>>       tem_4 = tem_3 - 5;
>>       short _5 = (short)_4;
>>       tem_5 = (unsigned int)_5;
>>      return tem_5;
>>    }
>> else
>>    {
>>      short _6 = (short)_3;
>>      return _6;
>>    }
>>
>> VRP should be able to remove the (unsigned int)(short) sign-extension
>> of tem_4.
>>
>> note that both incoming registers and return registers are "interesting".
>> For simplicity I suggest to not promote them on GIMPLE.
>>
>> What you'd lose in VRP2 is the smaller value-ranges you'd get from
>> (undefined) wrapping.  You could recover the undefinedness by
>> looking at SSA names recorded value-range and transfering that
>> in the promotion pass (but I'm not sure if you want to open the
>> can of latent signed overflow bugs in programs even more for
>> PROMOTE_MODE targets...)
>>
>
> Thanks. In the meantime I would like to revert the patch which is
> enabling zero/sign extension. I have bootstrapped it in x86_64 and
> regression testing is ongoing. Is this OK ?

Ok.

Thanks,
Richard.

> Thanks,
> Kugan
>
> gcc/ChangeLog:
>
> 2014-09-09  Kugan Vivekanandarajah  <kuganv@linaro.org>
>
>         Revert r213751:
>         * calls.c (precompute_arguments): Check
>          promoted_for_signed_and_unsigned_p and set the promoted mode.
>         (promoted_for_signed_and_unsigned_p): New function.
>         (expand_expr_real_1): Check promoted_for_signed_and_unsigned_p
>         and set the promoted mode.
>         * expr.h (promoted_for_signed_and_unsigned_p): New function definition.
>         * cfgexpand.c (expand_gimple_stmt_1): Call emit_move_insn if
>         SUBREG is promoted with SRP_SIGNED_AND_UNSIGNED.