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

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

On Thu, Aug 7, 2014 at 7:24 AM, Kugan <kugan.vivekanandarajah@linaro.org> wrote:
> On 06/08/14 23:29, Richard Biener wrote:
>> On Wed, Aug 6, 2014 at 3:21 PM, Kugan <kugan.vivekanandarajah@linaro.org> wrote:
>>> On 06/08/14 22:09, Richard Biener wrote:
>>>> On Tue, Aug 5, 2014 at 4:21 PM, Jakub Jelinek <jakub@redhat.com> wrote:
>>>>> On Tue, Aug 05, 2014 at 04:17:41PM +0200, Richard Biener wrote:
>>>>>> what's the semantic of setting SRP_SIGNED_AND_UNSIGNED
>>>>>> on the subreg?  That is, for the created (subreg:lhs_mode
>>>>>> (reg:<PROMOTE_MODE of ssa> N))?
>>>>>
>>>>> SRP_SIGNED_AND_UNSIGNED on a subreg should mean that
>>>>> the subreg is both zero and sign extended, which means
>>>>> that the topmost bit of the narrower mode is known to be zero,
>>>>> and all bits above it in the wider mode are known to be zero too.
>>>>> SRP_SIGNED means that the topmost bit of the narrower mode is
>>>>> either 0 or 1 and depending on that the above wider mode bits
>>>>> are either all 0 or all 1.
>>>>> SRP_UNSIGNED means that regardless of the topmost bit value,
>>>>> all above wider mode bits are 0.
>>>>
>>>> Ok, then from the context of the patch we already know that
>>>> either SRP_UNSIGNED or SRP_SIGNED is true which means
>>>> that the value is sign- or zero-extended.
>>>>
>>>> I suppose inside promoted_for_type_p
>>>> TYPE_MODE (TREE_TYPE (ssa)) == lhs_mode, I'm not sure
>>>> why you pass !unsignedp as lhs_uns.
>>>
>>> In expand_expr_real_1, it is already known that it is promoted for
>>> unsigned_p and we are setting SUBREG_PROMOTED_SET (temp, unsignedp).
>>>
>>> If we can prove that it is also promoted for !unsignedp, we can set
>>> SUBREG_PROMOTED_SET (temp, SRP_SIGNED_AND_UNSIGNED).
>>>
>>> promoted_for_type_p should prove this based on the value range info.
>>>
>>>>
>>>> Now, from 'ssa' alone we can't tell anything about a larger mode
>>>> registers value if that is either zero- or sign-extended.  But we
>>>> know that those bits are properly zero-extended if unsignedp
>>>> and properly sign-extended if !unsignedp?
>>>>
>>>> So what the predicate tries to prove is that sign- and zero-extending
>>>> results in the same larger-mode value.  This is true if the
>>>> MSB of the smaller mode is not set.
>>>>
>>>> Let's assume that smaller mode is that of 'ssa' then the test
>>>> is just
>>>>
>>>>   return (!tree_int_cst_sign_bit (min) && !tree_int_cst_sign_bit (max));
>>>>
>>>> no?
>>>
>>> hmm,  is this because we will never have a call to promoted_for_type_p
>>> with same sign (ignoring PROMOTE_MODE) for 'ssa' and the larger mode.
>>> The case with larger mode signed and 'ssa' unsigned will not work.
>>> Therefore larger mode unsigned and 'ssa' signed will be the only case
>>> that we should consider.
>>>
>>> However, with PROMOTE_MODE, isnt that we will miss some cases with this.
>>
>> No, PROMOTE_MODE will still either sign- or zero-extend.  If either
>> results in zeros in the upper bits then PROMOTE_MODE doesn't matter.
>>
>
> Thanks for the explanation. Please find the attached patch that
> implements this. I have updated the comments and predicate to match this.
>
> Bootstrap tested on x86_64-unknown-linux-gnu and regression tested on
> x86_64-unknown-linux-gnu and arm-none-linux-gnueabi with no new
> regressions. Is this OK?

Ok with changing

+      || (TYPE_PRECISION (TREE_TYPE (ssa)) > GET_MODE_PRECISION (mode)))
+    return false;

to check with != (does that even happen?)

Thanks,
Richard.

> Thanks,
> Kugan
>
> gcc/
> 2014-08-07  Kugan Vivekanandarajah  <kuganv@linaro.org>
>
>         * 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.
>
>
> gcc/testsuite
> 2014-08-07  Kugan Vivekanandarajah  <kuganv@linaro.org>
>
>         * gcc.dg/zero_sign_ext_test.c: New test.
>
>