[2/2] Enable elimination of zext/sext
diff mbox

Message ID CAFiYyc1Gh6teQWgc44j+U1w8S6oyXHigPj8N0-MqTOVg1Xbn5Q@mail.gmail.com
State New
Headers show

Commit Message

Richard Biener Aug. 5, 2014, 2:17 p.m. UTC
On Fri, Aug 1, 2014 at 6:03 PM, Kugan <kugan.vivekanandarajah@linaro.org> wrote:
>>>>  if (rhs_uns)
>>>>    return wi::ge_p (min, 0);  // if min >= 0 then range contains positive values
>>>>  else
>>>>    return wi::le_p (max, wi::max_value (TYPE_PRECISION (TREE_TYPE
>>>> (ssa)), SIGNED);  // if max <= signed-max-of-type then range doesn't
>>>> need sign-extension
>>>
>>> I think we will have to check that ssa has necessary sign/zero extension
>>> when assigned to lhs_type. If PROMOTE_MODE tells us that ssa's type will
>>> be interpreted differently, the value range of ssa also will have
>>> corresponding range.  In this cases, shouldn’t we have to check for
>>> upper and lower limit for both min and max?
>>
>> Hmm?  That's exactly what the check is testing...  we know that
>> min <= max thus if min >= 0 then max >= 0.
>>
>> zero_extension will never do anything on [0, INF]
>>
>> If max < MAX-SIGNED then sign-extension will not do anything.  Ok,
>> sign-extension will do sth for negative values still.  So rather
>>
>>   if (rhs_uns)
>>     return wi::geu_p (min, 0);
>>   else
>>     return wi::ges_p (min, 0) && wi::les_p (max, wi::max_value
>> (TYPE_PRECISION (TREE_TYPE (ssa)), SIGNED));
>>
>> ?
>
> Thanks for the explanation. I agree. Don’t we have to however check this
> on lhs_uns as this function is checking if ssa is promoted for lhs_sign
> and lhs_mode?
>
> Here is an attempt based on this. I ran regression testing with
> arm-none-linux-gnueabi on qemu-arm without any new regressions.
>
> Sine I am not comparing value ranges to see if it can be represented in
> lhs_sigh, I can now skip the PROMOTED_MODE check.

Now I'm lost.  You call this function from two contexts:


and

@@ -9527,7 +9587,10 @@ expand_expr_real_1 (tree exp, rtx target, enum
machine_mode tmode,

          temp = gen_lowpart_SUBREG (mode, decl_rtl);
          SUBREG_PROMOTED_VAR_P (temp) = 1;
-         SUBREG_PROMOTED_SET (temp, unsignedp);
+         if (is_promoted_for_type (ssa_name, mode, !unsignedp))
+           SUBREG_PROMOTED_SET (temp, SRP_SIGNED_AND_UNSIGNED);
+         else
+           SUBREG_PROMOTED_SET (temp, unsignedp);
          return temp;
        }

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))?

it seems that we need to verify that 'ssa', when promoted,
does not have bits set above the target modes MSB when
we know it is zero-extended (according to PROMOTE_MODE)?
Or has all bits set to one and is sign-extended (according to
PROMOTE_MODE)?

Now it seems that the promotion is according to
promote_{function,decl}_mode in expand_expr_real_1
and according to promote_mode in calls.c.

The function comment above promoted_for_type_p needs to be
more elaborate on what invariant it checks.  As you pass in
the subreg mode but you need to verify the larger mode is
properly extended.

> I am still using wide_int::from (instead of wi::max_value) to get the
> limit as I have to match the precision with min, max precision.
> otherwise wide_int comparisons will not work. Is there a better way for
> this?

I don't understand.  wi::max_value takes a precision argument.

>
> /* Return TRUE if value in SSA is already zero/sign extended for lhs type
>    (type here is the combination of LHS_MODE and LHS_UNS) using value range
>    information stored.  Return FALSE otherwise.  */
> bool
> promoted_for_type_p (tree ssa, enum machine_mode lhs_mode, bool lhs_uns)
> {
>   wide_int min, max, limit;
>   tree lhs_type;
>   bool rhs_uns;
>   signop rhs_signop;
>
>   if (ssa == NULL_TREE
>       || TREE_CODE (ssa) != SSA_NAME
>       || !INTEGRAL_TYPE_P (TREE_TYPE (ssa)))
>     return false;
>
>   /* Return FALSE if value_range is not recorded for SSA.  */
>   if (get_range_info (ssa, &min, &max) != VR_RANGE)
>     return false;
>
>   rhs_uns = TYPE_UNSIGNED (TREE_TYPE (ssa));
>   rhs_signop = rhs_uns ? UNSIGNED : SIGNED;
>   lhs_type = lang_hooks.types.type_for_mode (lhs_mode, lhs_uns);
>   limit = wide_int::from (TYPE_MAX_VALUE (lhs_type),
>                           TYPE_PRECISION (TREE_TYPE (ssa)), SIGNED);
>
>   if (lhs_uns)
>     /* If min >= 0 then range contains positive values and doesnt need
>        zero-extension.  */
>     return wi::ge_p (min, 0, rhs_signop);
>   else
>     /* If min >= 0 and max <= signed-max-of-type then range doesn't need
>        sign-extension.  */
>     return wi::ge_p (min, 0, rhs_signop) && wi::le_p (max, limit,
> rhs_signop);
> }
>
> Thanks,
> Kugan

Comments

Jakub Jelinek Aug. 5, 2014, 2:21 p.m. UTC | #1
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.

	Jakub
Richard Biener Aug. 6, 2014, 12:09 p.m. UTC | #2
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.

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?

Thanks,
Richard.

>         Jakub
Kugan Vivekanandarajah Aug. 6, 2014, 1:21 p.m. UTC | #3
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.

Thanks,
Kugan
Richard Biener Aug. 6, 2014, 1:29 p.m. UTC | #4
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.

Richard.

> Thanks,
> Kugan
>
>

Patch
diff mbox

diff --git a/gcc/calls.c b/gcc/calls.c
index a3e6faa..eac512f 100644
--- a/gcc/calls.c
+++ b/gcc/calls.c
@@ -1484,7 +1484,10 @@  precompute_arguments (int num_actuals, struct
arg_data *args)
              args[i].initial_value
                = gen_lowpart_SUBREG (mode, args[i].value);
              SUBREG_PROMOTED_VAR_P (args[i].initial_value) = 1;
-             SUBREG_PROMOTED_SET (args[i].initial_value, args[i].unsignedp);
+             if (is_promoted_for_type (args[i].tree_value, mode,
!args[i].unsignedp))
+               SUBREG_PROMOTED_SET (args[i].initial_value,
SRP_SIGNED_AND_UNSIGNED);
+             else
+               SUBREG_PROMOTED_SET (args[i].initial_value, args[i].unsignedp);