diff mbox

[2/2] Enable elimination of zext/sext

Message ID 53C34734.2080103@linaro.org
State New
Headers show

Commit Message

Kugan Vivekanandarajah July 14, 2014, 2:57 a.m. UTC
On 11/07/14 22:47, Richard Biener wrote:
> On Fri, Jul 11, 2014 at 1:52 PM, Kugan
> <kugan.vivekanandarajah@linaro.org> wrote:
>> Thanks foe the review and suggestions.
>>
>> On 10/07/14 22:15, Richard Biener wrote:
>>> On Mon, Jul 7, 2014 at 8:55 AM, Kugan <kugan.vivekanandarajah@linaro.org> wrote:
>>
>> [...]
>>
>>>>
>>>> For -fwrapv, it is due to how PROMOTE_MODE is defined in arm back-end.
>>>> In the test-case, a function (which has signed char return type) returns
>>>> -1 in one of the paths. ARM PROMOTE_MODE changes that to 255 and relies
>>>> on zero/sign extension generated by RTL again for the correct value. I
>>>> saw some other targets also defining similar think. I am therefore
>>>> skipping removing zero/sign extension if the ssa variable can be set to
>>>> negative integer constants.
>>>
>>> Hm?  I think you should rather check that you are removing a
>>> sign-/zero-extension - PROMOTE_MODE tells you if it will sign- or
>>> zero-extend.  Definitely
>>>
>>> +  /* In some architectures, negative integer constants are truncated and
>>> +     sign changed with target defined PROMOTE_MODE macro. This will impact
>>> +     the value range seen here and produce wrong code if zero/sign extensions
>>> +     are eliminated. Therefore, return false if this SSA can have negative
>>> +     integers.  */
>>> +  if (is_gimple_assign (stmt)
>>> +      && (TREE_CODE_CLASS (gimple_assign_rhs_code (stmt)) == tcc_unary))
>>> +    {
>>> +      tree rhs1 = gimple_assign_rhs1 (stmt);
>>> +      if (TREE_CODE (rhs1) == INTEGER_CST
>>> +         && !TYPE_UNSIGNED (TREE_TYPE (ssa))
>>> +         && tree_int_cst_compare (rhs1, integer_zero_node) == -1)
>>> +       return false;
>>>
>>> looks completely bogus ... (an unary op with a constant operand?)
>>> instead you want to do sth like
>>
>> I see that unary op with a constant operand is not possible in gimple.
>> What I wanted to check here is any sort of constant loads; but seems
>> that will not happen in gimple. Is PHI statements the only possible
>> statements where we will end up with such constants.
> 
> No, in theory you can have
> 
>   ssa_1 = -1;
> 
> but that's not unary but a GIMPLE_SINGLE_RHS and thus
> gimple_assign_rhs_code (stmt) == INTEGER_CST.
> 
>>>   mode = TYPE_MODE (TREE_TYPE (ssa));
>>>   rhs_uns = TYPE_UNSIGNED (TREE_TYPE (ssa));
>>>   PROMOTE_MODE (mode, rhs_uns, TREE_TYPE (ssa));
>>>
>>> instead of initializing rhs_uns from ssas type.  That is, if
>>> PROMOTE_MODE tells you to promote _not_ according to ssas sign then
>>> honor that.
>>
>> This is triggered in pr43017.c in function foo for arm-none-linux-gnueabi.
>>
>> where, the gimple statement that cause this looks like:
>> .....
>>   # _3 = PHI <_17(7), -1(2)>
>> bb43:
>>   return _3;
>>
>> ARM PROMOTE_MODE changes the sign for integer constants only and hence
>> looking at the variable with PROMOTE_MODE is not changing the sign in
>> this case.
>>
>> #define PROMOTE_MODE(MODE, UNSIGNEDP, TYPE)     \
>>   if (GET_MODE_CLASS (MODE) == MODE_INT         \
>>       && GET_MODE_SIZE (MODE) < 4)              \
>>     {                                           \
>>       if (MODE == QImode)                       \
>>         UNSIGNEDP = 1;                          \
>>       else if (MODE == HImode)                  \
>>         UNSIGNEDP = 1;                          \
>>       (MODE) = SImode;                          \
>>     }
> 
> Where does it only apply for "constants"?  It applies to all QImode and
> HImode entities.

oops, sorry. I don’t know what I was thinking or looking at when I wrote
that :( It indeed fixes my problems. Thanks for that.

Here is the modified patch. Bootstrapped and regression tested for
86_64-unknown-linux-gnu and arm-none-linux-gnueabi with no new regressions.


Is this OK?

Thanks,
Kugan


gcc/

2014-07-14  Kugan Vivekanandarajah  <kuganv@linaro.org>

	* calls.c (precompute_arguments): Check is_promoted_for_type
	and set the promoted mode.
	(is_promoted_for_type): New function.
	(expand_expr_real_1): Check is_promoted_for_type
	and set the promoted mode.
	* expr.h (is_promoted_for_type): 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-07-14  Kugan Vivekanandarajah  <kuganv@linaro.org>

	* gcc.dg/zero_sign_ext_test.c: New test.

Comments

Bernhard Reutner-Fischer July 14, 2014, 8:10 p.m. UTC | #1
On 14 July 2014 04:58:17 Kugan <kugan.vivekanandarajah@linaro.org> wrote:

> On 11/07/14 22:47, Richard Biener wrote:
> > On Fri, Jul 11, 2014 at 1:52 PM, Kugan
> > <kugan.vivekanandarajah@linaro.org> wrote:
> >> Thanks foe the review and suggestions.
> >>
> >> On 10/07/14 22:15, Richard Biener wrote:
> >>> On Mon, Jul 7, 2014 at 8:55 AM, Kugan 
> <kugan.vivekanandarajah@linaro.org> wrote:
> >>
> >> [...]
> >>
> >>>>
> >>>> For -fwrapv, it is due to how PROMOTE_MODE is defined in arm back-end.
> >>>> In the test-case, a function (which has signed char return type) returns
> >>>> -1 in one of the paths. ARM PROMOTE_MODE changes that to 255 and relies
> >>>> on zero/sign extension generated by RTL again for the correct value. I
> >>>> saw some other targets also defining similar think. I am therefore
> >>>> skipping removing zero/sign extension if the ssa variable can be set to
> >>>> negative integer constants.
> >>>
> >>> Hm?  I think you should rather check that you are removing a
> >>> sign-/zero-extension - PROMOTE_MODE tells you if it will sign- or
> >>> zero-extend.  Definitely
> >>>
> >>> +  /* In some architectures, negative integer constants are truncated and
> >>> +     sign changed with target defined PROMOTE_MODE macro. This will impact
> >>> +     the value range seen here and produce wrong code if zero/sign 
> extensions
> >>> +     are eliminated. Therefore, return false if this SSA can have negative
> >>> +     integers.  */
> >>> +  if (is_gimple_assign (stmt)
> >>> +      && (TREE_CODE_CLASS (gimple_assign_rhs_code (stmt)) == tcc_unary))
> >>> +    {
> >>> +      tree rhs1 = gimple_assign_rhs1 (stmt);
> >>> +      if (TREE_CODE (rhs1) == INTEGER_CST
> >>> +         && !TYPE_UNSIGNED (TREE_TYPE (ssa))
> >>> +         && tree_int_cst_compare (rhs1, integer_zero_node) == -1)
> >>> +       return false;
> >>>
> >>> looks completely bogus ... (an unary op with a constant operand?)
> >>> instead you want to do sth like
> >>
> >> I see that unary op with a constant operand is not possible in gimple.
> >> What I wanted to check here is any sort of constant loads; but seems
> >> that will not happen in gimple. Is PHI statements the only possible
> >> statements where we will end up with such constants.
> >
> > No, in theory you can have
> >
> >   ssa_1 = -1;
> >
> > but that's not unary but a GIMPLE_SINGLE_RHS and thus
> > gimple_assign_rhs_code (stmt) == INTEGER_CST.
> >
> >>>   mode = TYPE_MODE (TREE_TYPE (ssa));
> >>>   rhs_uns = TYPE_UNSIGNED (TREE_TYPE (ssa));
> >>>   PROMOTE_MODE (mode, rhs_uns, TREE_TYPE (ssa));
> >>>
> >>> instead of initializing rhs_uns from ssas type.  That is, if
> >>> PROMOTE_MODE tells you to promote _not_ according to ssas sign then
> >>> honor that.
> >>
> >> This is triggered in pr43017.c in function foo for arm-none-linux-gnueabi.
> >>
> >> where, the gimple statement that cause this looks like:
> >> .....
> >>   # _3 = PHI <_17(7), -1(2)>
> >> bb43:
> >>   return _3;
> >>
> >> ARM PROMOTE_MODE changes the sign for integer constants only and hence
> >> looking at the variable with PROMOTE_MODE is not changing the sign in
> >> this case.
> >>
> >> #define PROMOTE_MODE(MODE, UNSIGNEDP, TYPE)     \
> >>   if (GET_MODE_CLASS (MODE) == MODE_INT         \
> >>       && GET_MODE_SIZE (MODE) < 4)              \
> >>     {                                           \
> >>       if (MODE == QImode)                       \
> >>         UNSIGNEDP = 1;                          \
> >>       else if (MODE == HImode)                  \
> >>         UNSIGNEDP = 1;                          \
> >>       (MODE) = SImode;                          \
> >>     }
> >
> > Where does it only apply for "constants"?  It applies to all QImode and
> > HImode entities.
>
> oops, sorry. I don’t know what I was thinking or looking at when I wrote
> that :( It indeed fixes my problems. Thanks for that.
>
> Here is the modified patch. Bootstrapped and regression tested for
> 86_64-unknown-linux-gnu and arm-none-linux-gnueabi with no new regressions.
>
>
> Is this OK?
>
> Thanks,
> Kugan
>
>
> gcc/
>
> 2014-07-14  Kugan Vivekanandarajah  <kuganv@linaro.org>
>
> 	* calls.c (precompute_arguments): Check is_promoted_for_type
> 	and set the promoted mode.
> 	(is_promoted_for_type): New function.

Don't we name predicates more like promoted_for_type_p?

Thanks,
> 	(expand_expr_real_1): Check is_promoted_for_type
> 	and set the promoted mode.
> 	* expr.h (is_promoted_for_type): 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-07-14  Kugan Vivekanandarajah  <kuganv@linaro.org>
>
> 	* gcc.dg/zero_sign_ext_test.c: New test.



Sent with AquaMail for Android
http://www.aqua-mail.com
Richard Biener July 23, 2014, 2:18 p.m. UTC | #2
On Mon, Jul 14, 2014 at 4:57 AM, Kugan
<kugan.vivekanandarajah@linaro.org> wrote:
> On 11/07/14 22:47, Richard Biener wrote:
>> On Fri, Jul 11, 2014 at 1:52 PM, Kugan
>> <kugan.vivekanandarajah@linaro.org> wrote:
>>> Thanks foe the review and suggestions.
>>>
>>> On 10/07/14 22:15, Richard Biener wrote:
>>>> On Mon, Jul 7, 2014 at 8:55 AM, Kugan <kugan.vivekanandarajah@linaro.org> wrote:
>>>
>>> [...]
>>>
>>>>>
>>>>> For -fwrapv, it is due to how PROMOTE_MODE is defined in arm back-end.
>>>>> In the test-case, a function (which has signed char return type) returns
>>>>> -1 in one of the paths. ARM PROMOTE_MODE changes that to 255 and relies
>>>>> on zero/sign extension generated by RTL again for the correct value. I
>>>>> saw some other targets also defining similar think. I am therefore
>>>>> skipping removing zero/sign extension if the ssa variable can be set to
>>>>> negative integer constants.
>>>>
>>>> Hm?  I think you should rather check that you are removing a
>>>> sign-/zero-extension - PROMOTE_MODE tells you if it will sign- or
>>>> zero-extend.  Definitely
>>>>
>>>> +  /* In some architectures, negative integer constants are truncated and
>>>> +     sign changed with target defined PROMOTE_MODE macro. This will impact
>>>> +     the value range seen here and produce wrong code if zero/sign extensions
>>>> +     are eliminated. Therefore, return false if this SSA can have negative
>>>> +     integers.  */
>>>> +  if (is_gimple_assign (stmt)
>>>> +      && (TREE_CODE_CLASS (gimple_assign_rhs_code (stmt)) == tcc_unary))
>>>> +    {
>>>> +      tree rhs1 = gimple_assign_rhs1 (stmt);
>>>> +      if (TREE_CODE (rhs1) == INTEGER_CST
>>>> +         && !TYPE_UNSIGNED (TREE_TYPE (ssa))
>>>> +         && tree_int_cst_compare (rhs1, integer_zero_node) == -1)
>>>> +       return false;
>>>>
>>>> looks completely bogus ... (an unary op with a constant operand?)
>>>> instead you want to do sth like
>>>
>>> I see that unary op with a constant operand is not possible in gimple.
>>> What I wanted to check here is any sort of constant loads; but seems
>>> that will not happen in gimple. Is PHI statements the only possible
>>> statements where we will end up with such constants.
>>
>> No, in theory you can have
>>
>>   ssa_1 = -1;
>>
>> but that's not unary but a GIMPLE_SINGLE_RHS and thus
>> gimple_assign_rhs_code (stmt) == INTEGER_CST.
>>
>>>>   mode = TYPE_MODE (TREE_TYPE (ssa));
>>>>   rhs_uns = TYPE_UNSIGNED (TREE_TYPE (ssa));
>>>>   PROMOTE_MODE (mode, rhs_uns, TREE_TYPE (ssa));
>>>>
>>>> instead of initializing rhs_uns from ssas type.  That is, if
>>>> PROMOTE_MODE tells you to promote _not_ according to ssas sign then
>>>> honor that.
>>>
>>> This is triggered in pr43017.c in function foo for arm-none-linux-gnueabi.
>>>
>>> where, the gimple statement that cause this looks like:
>>> .....
>>>   # _3 = PHI <_17(7), -1(2)>
>>> bb43:
>>>   return _3;
>>>
>>> ARM PROMOTE_MODE changes the sign for integer constants only and hence
>>> looking at the variable with PROMOTE_MODE is not changing the sign in
>>> this case.
>>>
>>> #define PROMOTE_MODE(MODE, UNSIGNEDP, TYPE)     \
>>>   if (GET_MODE_CLASS (MODE) == MODE_INT         \
>>>       && GET_MODE_SIZE (MODE) < 4)              \
>>>     {                                           \
>>>       if (MODE == QImode)                       \
>>>         UNSIGNEDP = 1;                          \
>>>       else if (MODE == HImode)                  \
>>>         UNSIGNEDP = 1;                          \
>>>       (MODE) = SImode;                          \
>>>     }
>>
>> Where does it only apply for "constants"?  It applies to all QImode and
>> HImode entities.
>
> oops, sorry. I don’t know what I was thinking or looking at when I wrote
> that :( It indeed fixes my problems. Thanks for that.
>
> Here is the modified patch. Bootstrapped and regression tested for
> 86_64-unknown-linux-gnu and arm-none-linux-gnueabi with no new regressions.
>
>
> Is this OK?

+  lhs_type = lang_hooks.types.type_for_mode (lhs_mode, lhs_uns);
...
+      && ((!lhs_uns && !wi::neg_p (min, TYPE_SIGN (lhs_type)))
...
+  type_min = wide_int::from (TYPE_MIN_VALUE (lhs_type), prec,
+                            TYPE_SIGN (TREE_TYPE (ssa)));
+  type_max = wide_int::from (TYPE_MAX_VALUE (lhs_type), prec,
+                            TYPE_SIGN (TREE_TYPE (ssa)));

you shouldn't try getting at lhs_type.  Btw, do you want to constrain
lhs_mode to MODE_INTs somewhere?

For TYPE_SIGN use lhs_uns instead, for the min/max value you
should use wi::min_value () and wi::max_value () instead.

You are still using TYPE_SIGN (TREE_TYPE (ssa)) here and later,
but we computed rhs_uns "properly" using PROMOTE_MODE.
I think  the code with re-setting lhs_uns if rhs_uns != lhs_uns
and later using TYPE_SIGN again is pretty hard to follow.

Btw, it seems you need to conditionalize the call to PROMOTE_MODE
on its availability.

Isn't it simply about choosing a proper range we need to restrict
ssa to?  That is, dependent on rhs_uns computed by PROMOTE_MODE,
simply:

+  mode = TYPE_MODE (TREE_TYPE (ssa));
+  rhs_uns = TYPE_UNSIGNED (TREE_TYPE (ssa));
#ifdef PROMOTE_MODE
+  PROMOTE_MODE (mode, rhs_uns, TREE_TYPE (ssa));
#endif

 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

?

Thanks,
Richard.

> Thanks,
> Kugan
>
>
> gcc/
>
> 2014-07-14  Kugan Vivekanandarajah  <kuganv@linaro.org>
>
>         * calls.c (precompute_arguments): Check is_promoted_for_type
>         and set the promoted mode.
>         (is_promoted_for_type): New function.
>         (expand_expr_real_1): Check is_promoted_for_type
>         and set the promoted mode.
>         * expr.h (is_promoted_for_type): 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-07-14  Kugan Vivekanandarajah  <kuganv@linaro.org>
>
>         * gcc.dg/zero_sign_ext_test.c: New test.
Kugan Vivekanandarajah Aug. 1, 2014, 4:51 a.m. UTC | #3
> +  lhs_type = lang_hooks.types.type_for_mode (lhs_mode, lhs_uns);
> ...
> +      && ((!lhs_uns && !wi::neg_p (min, TYPE_SIGN (lhs_type)))
> ...
> +  type_min = wide_int::from (TYPE_MIN_VALUE (lhs_type), prec,
> +                            TYPE_SIGN (TREE_TYPE (ssa)));
> +  type_max = wide_int::from (TYPE_MAX_VALUE (lhs_type), prec,
> +                            TYPE_SIGN (TREE_TYPE (ssa)));
> 
> you shouldn't try getting at lhs_type.  Btw, do you want to constrain
> lhs_mode to MODE_INTs somewhere?

Is this in addition to !INTEGRAL_TYPE_P (TREE_TYPE (ssa)))? Do you mean
that I should check lhs_mode as well?

> For TYPE_SIGN use lhs_uns instead, for the min/max value you
> should use wi::min_value () and wi::max_value () instead.
> 
> You are still using TYPE_SIGN (TREE_TYPE (ssa)) here and later,
> but we computed rhs_uns "properly" using PROMOTE_MODE.
> I think  the code with re-setting lhs_uns if rhs_uns != lhs_uns
> and later using TYPE_SIGN again is pretty hard to follow.
> 
> Btw, it seems you need to conditionalize the call to PROMOTE_MODE
> on its availability.
> 
> Isn't it simply about choosing a proper range we need to restrict
> ssa to?  That is, dependent on rhs_uns computed by PROMOTE_MODE,
> simply:
> 
> +  mode = TYPE_MODE (TREE_TYPE (ssa));
> +  rhs_uns = TYPE_UNSIGNED (TREE_TYPE (ssa));
> #ifdef PROMOTE_MODE
> +  PROMOTE_MODE (mode, rhs_uns, TREE_TYPE (ssa));
> #endif
> 
>  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?

How about this?

bool
promoted_for_type_p (tree ssa, enum machine_mode lhs_mode, bool lhs_uns)
{
  wide_int min, max;
  tree lhs_type, rhs_type;
  bool rhs_uns;
  enum machine_mode rhs_mode;
  tree min_tree, max_tree;

  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));
  if (rhs_uns != lhs_uns)
    {
      /* Signedness of LHS and RHS differs and values also cannot be
	 represented in LHS range.  */
      unsigned int prec = min.get_precision ();
      if ((lhs_uns && wi::neg_p (min, rhs_uns ? UNSIGNED : SIGNED))
	  || (!lhs_uns && !wi::le_p (max,
				    wi::max_value (prec, SIGNED),
				    rhs_uns ? UNSIGNED : SIGNED)))
	return false;
    }

  /* In some architectures, modes are promoted and sign changed with
     target defined PROMOTE_MODE macro.  If PROMOTE_MODE tells you to
     promote _not_ according to ssa's sign then honour that.  */
  rhs_mode = TYPE_MODE (TREE_TYPE (ssa));
#ifdef PROMOTE_MODE
  PROMOTE_MODE (rhs_mode, rhs_uns, TREE_TYPE (ssa));
#endif

  rhs_type = lang_hooks.types.type_for_mode (rhs_mode, rhs_uns);
  lhs_type = lang_hooks.types.type_for_mode (lhs_mode, lhs_uns);
  min_tree = wide_int_to_tree (rhs_type, min);
  max_tree = wide_int_to_tree (rhs_type, max);

  /* Check if values lies in-between the type range.  */
  if (int_fits_type_p (min_tree, lhs_type)
      && int_fits_type_p (max_tree, lhs_type))
    return true;
  else
    return false;
}


Thanks,
Kugan
Richard Biener Aug. 1, 2014, 11:16 a.m. UTC | #4
On Fri, Aug 1, 2014 at 6:51 AM, Kugan <kugan.vivekanandarajah@linaro.org> wrote:
>> +  lhs_type = lang_hooks.types.type_for_mode (lhs_mode, lhs_uns);
>> ...
>> +      && ((!lhs_uns && !wi::neg_p (min, TYPE_SIGN (lhs_type)))
>> ...
>> +  type_min = wide_int::from (TYPE_MIN_VALUE (lhs_type), prec,
>> +                            TYPE_SIGN (TREE_TYPE (ssa)));
>> +  type_max = wide_int::from (TYPE_MAX_VALUE (lhs_type), prec,
>> +                            TYPE_SIGN (TREE_TYPE (ssa)));
>>
>> you shouldn't try getting at lhs_type.  Btw, do you want to constrain
>> lhs_mode to MODE_INTs somewhere?
>
> Is this in addition to !INTEGRAL_TYPE_P (TREE_TYPE (ssa)))? Do you mean
> that I should check lhs_mode as well?

No, that's probably enough.

>> For TYPE_SIGN use lhs_uns instead, for the min/max value you
>> should use wi::min_value () and wi::max_value () instead.
>>
>> You are still using TYPE_SIGN (TREE_TYPE (ssa)) here and later,
>> but we computed rhs_uns "properly" using PROMOTE_MODE.
>> I think  the code with re-setting lhs_uns if rhs_uns != lhs_uns
>> and later using TYPE_SIGN again is pretty hard to follow.
>>
>> Btw, it seems you need to conditionalize the call to PROMOTE_MODE
>> on its availability.
>>
>> Isn't it simply about choosing a proper range we need to restrict
>> ssa to?  That is, dependent on rhs_uns computed by PROMOTE_MODE,
>> simply:
>>
>> +  mode = TYPE_MODE (TREE_TYPE (ssa));
>> +  rhs_uns = TYPE_UNSIGNED (TREE_TYPE (ssa));
>> #ifdef PROMOTE_MODE
>> +  PROMOTE_MODE (mode, rhs_uns, TREE_TYPE (ssa));
>> #endif
>>
>>  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));

?

I don't like the use of int_fits_type_p you propose.

Richard.

> How about this?
>
> bool
> promoted_for_type_p (tree ssa, enum machine_mode lhs_mode, bool lhs_uns)
> {
>   wide_int min, max;
>   tree lhs_type, rhs_type;
>   bool rhs_uns;
>   enum machine_mode rhs_mode;
>   tree min_tree, max_tree;
>
>   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));
>   if (rhs_uns != lhs_uns)
>     {
>       /* Signedness of LHS and RHS differs and values also cannot be
>          represented in LHS range.  */
>       unsigned int prec = min.get_precision ();
>       if ((lhs_uns && wi::neg_p (min, rhs_uns ? UNSIGNED : SIGNED))
>           || (!lhs_uns && !wi::le_p (max,
>                                     wi::max_value (prec, SIGNED),
>                                     rhs_uns ? UNSIGNED : SIGNED)))
>         return false;
>     }
>
>   /* In some architectures, modes are promoted and sign changed with
>      target defined PROMOTE_MODE macro.  If PROMOTE_MODE tells you to
>      promote _not_ according to ssa's sign then honour that.  */
>   rhs_mode = TYPE_MODE (TREE_TYPE (ssa));
> #ifdef PROMOTE_MODE
>   PROMOTE_MODE (rhs_mode, rhs_uns, TREE_TYPE (ssa));
> #endif
>
>   rhs_type = lang_hooks.types.type_for_mode (rhs_mode, rhs_uns);
>   lhs_type = lang_hooks.types.type_for_mode (lhs_mode, lhs_uns);
>   min_tree = wide_int_to_tree (rhs_type, min);
>   max_tree = wide_int_to_tree (rhs_type, max);
>
>   /* Check if values lies in-between the type range.  */
>   if (int_fits_type_p (min_tree, lhs_type)
>       && int_fits_type_p (max_tree, lhs_type))
>     return true;
>   else
>     return false;
> }
>
>
> Thanks,
> Kugan
Kugan Vivekanandarajah Aug. 1, 2014, 4:03 p.m. UTC | #5
>>>  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.

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?

/* 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
Kugan Vivekanandarajah Aug. 3, 2014, 11:55 p.m. UTC | #6
On 02/08/14 02:03, Kugan 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));
>>
>> ?

Looking at your comments again, I think we have to consider three things
here.

To be able assign to LHS (of lhs_uns and lhs_mode) without conversion of
RHS (tree SSA)

* If we ignore the mode changes (i.e. LHS_mode can be different in terms
of precision) and ignore PROMOTE_MODE and consider only the sign of LHS
and RHS
  if (lhs_uns)
   return wi::ge_p (min, 0, rhs_signop);  // if min >= 0 then range
contains positive values
 else
   if (rhs_uns)
     // if max <= signed-max-of-type then range doesn't need sign-extension
     return wi::le_p (max, wi::max_value (TYPE_PRECISION (TREE_TYPE
(ssa)), SIGNED);
   else
     return true;


* However, if we consider the PROMOTE_MODE might change the RHS sign
  if (lhs_uns)
    {
      return wi::ge_p (min, 0, rhs_signop);
    }
  else
    {
      signed_max = wide_int::from (TYPE_MAX_VALUE (lhs_type),
				   TYPE_PRECISION (TREE_TYPE (ssa)), rhs_signop);
      if (rhs_uns)
	/* If PROMOTE_MODE changed an RHS signed to unsigned and
	   SSA contains negative value range, we still have to do sign-extend.  */
	return wi::ge_p (min, 0, TYPE_SIGN (TREE_TYPE (ssa)))
	  && wi::le_p (max, signed_max, rhs_signop);
      else
	/* If PROMOTE_MODE changed an RHS unsigned to signed and SSA contains value
	   range more than signed-max-of-type, we still have to do sign-extend.  */
	return wi::le_p (max, signed_max, TYPE_SIGN (TREE_TYPE (ssa)));
    }

* If we also consider that LHS mode and RHS mode precision can be different
  if (lhs_uns)
    {
      unsigned_max = wide_int::from (TYPE_MAX_VALUE (lhs_type),
				     TYPE_PRECISION (TREE_TYPE (ssa)), rhs_signop);
      /* If min >= 0 then range contains positive values and doesnt need
	 zero-extension.  If max <= unsigned-max-of-type, then value fits type.  */
      return wi::ge_p (min, 0, rhs_signop)
	&& wi::le_p (max, unsigned_max, rhs_signop);
    }
  else
    {
      signed_max = wide_int::from (TYPE_MAX_VALUE (lhs_type),
				   TYPE_PRECISION (TREE_TYPE (ssa)), rhs_signop);
      signed_min = wide_int::from (TYPE_MIN_VALUE (lhs_type),
				   TYPE_PRECISION (TREE_TYPE (ssa)), rhs_signop);
      if (rhs_uns)
	/* If PROMOTE_MODE changed an RHS signed to unsigned and
	   SSA contains negative value range, we still have to do sign-extend.  */
	return wi::ge_p (min, 0, TYPE_SIGN (TREE_TYPE (ssa)))
	  && wi::le_p (max, signed_max, rhs_signop);
      else
	/* If PROMOTE_MODE changed an RHS unsigned to signed and SSA contains value
	   range more than signed-max-of-type, we still have to do sign-extend.  */
	return wi::le_p (max, signed_max, TYPE_SIGN (TREE_TYPE (ssa)))
	  && wi::ge_p (min, signed_min, rhs_signop);
    }
}


Since we can have PROMOTE_MODE changing the sign and LHS mode and RHS
mode precision can be different, the check should be the third one. Does
that make sense or am I still missing it?

Thanks again for your time,
Kugan
diff mbox

Patch

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);
 	    }
 	}
     }
diff --git a/gcc/cfgexpand.c b/gcc/cfgexpand.c
index f98c322..b14626c 100644
--- a/gcc/cfgexpand.c
+++ b/gcc/cfgexpand.c
@@ -3309,7 +3309,13 @@  expand_gimple_stmt_1 (gimple stmt)
 					  GET_MODE (target), temp, unsignedp);
 		  }
 
-		convert_move (SUBREG_REG (target), temp, unsignedp);
+		if ((SUBREG_PROMOTED_GET (target) == SRP_SIGNED_AND_UNSIGNED)
+		    && (GET_CODE (temp) == SUBREG)
+		    && (GET_MODE (target) == GET_MODE (temp))
+		    && (GET_MODE (SUBREG_REG (target)) == GET_MODE (SUBREG_REG (temp))))
+		  emit_move_insn (SUBREG_REG (target), SUBREG_REG (temp));
+		else
+		  convert_move (SUBREG_REG (target), temp, unsignedp);
 	      }
 	    else if (nontemporal && emit_storent_insn (target, temp))
 	      ;
diff --git a/gcc/expr.c b/gcc/expr.c
index 7356e76..d25f506 100644
--- a/gcc/expr.c
+++ b/gcc/expr.c
@@ -68,6 +68,7 @@  along with GCC; see the file COPYING3.  If not see
 #include "tree-ssa-address.h"
 #include "cfgexpand.h"
 #include "builtins.h"
+#include "tree-ssa.h"
 
 #ifndef STACK_PUSH_CODE
 #ifdef STACK_GROWS_DOWNWARD
@@ -9224,6 +9225,65 @@  expand_expr_real_2 (sepops ops, rtx target, enum machine_mode tmode,
 }
 #undef REDUCE_BIT_FIELD
 
+/* 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
+is_promoted_for_type (tree ssa, enum machine_mode lhs_mode, bool lhs_uns)
+{
+  wide_int type_min, type_max;
+  wide_int min, max, limit;
+  unsigned int prec;
+  tree lhs_type;
+  bool rhs_uns;
+  enum machine_mode mode;
+
+  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;
+
+  /* In some architectures, modes are promoted and sign changed with
+     target defined PROMOTE_MODE macro.  If PROMOTE_MODE tells you to
+     promote _not_ according to ssa's sign then honour that.  */
+  mode = TYPE_MODE (TREE_TYPE (ssa));
+  rhs_uns = TYPE_UNSIGNED (TREE_TYPE (ssa));
+  PROMOTE_MODE (mode, rhs_uns, TREE_TYPE (ssa));
+
+  lhs_type = lang_hooks.types.type_for_mode (lhs_mode, lhs_uns);
+  prec = min.get_precision ();
+
+  /* Signed maximum value.  */
+  limit = wide_int::from (TYPE_MAX_VALUE (TREE_TYPE (ssa)), prec, SIGNED);
+
+  /* Signedness of LHS and RHS differs but values in range.  */
+  if ((rhs_uns != lhs_uns)
+      && ((!lhs_uns && !wi::neg_p (min, TYPE_SIGN (lhs_type)))
+	  || (lhs_uns && (wi::cmp (max, limit, TYPE_SIGN (TREE_TYPE (ssa))) == -1))))
+    lhs_uns = !lhs_uns;
+
+  /* Signedness of LHS and RHS should match.  */
+  if (rhs_uns != lhs_uns)
+    return false;
+
+  type_min = wide_int::from (TYPE_MIN_VALUE (lhs_type), prec,
+			     TYPE_SIGN (TREE_TYPE (ssa)));
+  type_max = wide_int::from (TYPE_MAX_VALUE (lhs_type), prec,
+			     TYPE_SIGN (TREE_TYPE (ssa)));
+
+  /* Check if values lies in-between the type range.  */
+  if ((wi::neg_p (max, TYPE_SIGN (TREE_TYPE (ssa)))
+       || (wi::cmp (max, type_max, TYPE_SIGN (TREE_TYPE (ssa))) != 1))
+      && (!wi::neg_p (min, TYPE_SIGN (TREE_TYPE (ssa)))
+	  || (wi::cmp (type_min, min, TYPE_SIGN (TREE_TYPE (ssa))) != 1)))
+    return true;
+
+  return false;
+}
 
 /* Return TRUE if expression STMT is suitable for replacement.  
    Never consider memory loads as replaceable, because those don't ever lead 
@@ -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;
 	}
 
diff --git a/gcc/expr.h b/gcc/expr.h
index 6a1d3ab..e99d000 100644
--- a/gcc/expr.h
+++ b/gcc/expr.h
@@ -440,6 +440,7 @@  extern rtx expand_expr_real_1 (tree, rtx, enum machine_mode,
 			       enum expand_modifier, rtx *, bool);
 extern rtx expand_expr_real_2 (sepops, rtx, enum machine_mode,
 			       enum expand_modifier);
+extern bool is_promoted_for_type (tree, enum machine_mode, bool);
 
 /* Generate code for computing expression EXP.
    An rtx for the computed value is returned.  The value is never null.
diff --git a/gcc/testsuite/gcc.dg/zero_sign_ext_test.c b/gcc/testsuite/gcc.dg/zero_sign_ext_test.c
index e69de29..6a52678 100644
--- a/gcc/testsuite/gcc.dg/zero_sign_ext_test.c
+++ b/gcc/testsuite/gcc.dg/zero_sign_ext_test.c
@@ -0,0 +1,136 @@ 
+extern void abort (void);
+
+/* { dg-options "-O2" } */
+/* { dg-do run } */
+
+#define TYPE_MAX(type, sign)	\
+  ((!sign) ? ((1 << (sizeof (type) * 8 - 1)) - 1) :	\
+   ((1 << (sizeof (type) * 8)) - 1))
+#define TYPE_MIN(type, sign)	\
+  ((!sign) ? -(1 << (sizeof (type) * 8 - 1)) : 0)
+
+#define TEST_FN(NAME, ARG_TYPE, RET_TYPE, CAST_TYPE, VAL, VR_MIN, VR_MAX)\
+  __attribute__((noinline, noclone)) RET_TYPE				\
+      NAME (ARG_TYPE arg){						\
+      RET_TYPE ret = VAL;						\
+      if (arg + 1 < VR_MIN || arg + 1 > VR_MAX) return ret;		\
+      /* Value Range of arg at this point will be  [VR_min, VR_max].  */\
+      arg = arg + VAL;							\
+      ret = (CAST_TYPE)arg;						\
+      return arg;							\
+  }
+
+/* Signed to signed conversion with value in-range.  */
+TEST_FN (foo1, short, short, char, 1, TYPE_MIN (char, 0), TYPE_MAX (char, 0));
+TEST_FN (foo2, short, short, char, 1, TYPE_MIN (char, 0) + 1,\
+	TYPE_MAX (char, 0) - 1);
+
+/* Signed to signed conversion with value not in-range.  */
+TEST_FN (foo3, short, short, char, -1, TYPE_MIN (short, 0) + 1,  100);
+TEST_FN (foo4, short, short, char, 1, 12, TYPE_MAX (short, 0) + 1);
+
+/* Unsigned to unsigned conversion with value in-range.  */
+TEST_FN (foo5, unsigned short, unsigned short, unsigned char, 1,\
+	TYPE_MIN (char, 1) + 1, TYPE_MAX (char, 1) - 1);
+TEST_FN (foo6, unsigned short, unsigned short, unsigned char, 1,\
+	TYPE_MIN (char, 1), TYPE_MAX (char, 1));
+
+/* Unsigned to unsigned conversion with value not in-range.  */
+TEST_FN (foo7, unsigned short, unsigned short, unsigned char, 1,\
+	TYPE_MIN (short, 1) + 1, TYPE_MAX (short, 1) - 1);
+TEST_FN (foo8, unsigned short, unsigned short, unsigned char, 1,\
+	TYPE_MIN (short, 1), TYPE_MAX (short, 1));
+
+/* Signed to unsigned conversion with value range positive.  */
+TEST_FN (foo9, short, short, unsigned char, -1, 1,\
+	TYPE_MAX (char, 1) - 1);
+TEST_FN (foo10, short, short, unsigned char, 1, 0,\
+	TYPE_MAX (char, 1));
+
+/* Signed to unsigned conversion with value range negative.  */
+TEST_FN (foo11, short, short, unsigned char, 1,\
+	TYPE_MIN (char, 0) + 1, TYPE_MAX (char, 0) - 1);
+TEST_FN (foo12, short, short, unsigned char, 1,\
+	TYPE_MIN (char, 0), TYPE_MAX (char, 0));
+
+/* Unsigned to Signed conversion with value range in signed equiv range.  */
+TEST_FN (foo13, unsigned short, unsigned short, char, 1,\
+	TYPE_MIN (char, 1) + 1, TYPE_MAX (char, 0) - 1);
+TEST_FN (foo14, unsigned short, unsigned short, char, 1,\
+	TYPE_MIN (char, 1), TYPE_MAX (char, 0));
+
+/* Unsigned to Signed conversion with value range not-in signed range.  */
+TEST_FN (foo15, unsigned short, unsigned short, char, 1,\
+	TYPE_MIN (char, 1) + 1, TYPE_MAX (char, 1) - 1);
+TEST_FN (foo16, unsigned short, unsigned short, char, 1,\
+	TYPE_MIN (char, 1), TYPE_MAX (char, 1));
+
+int main ()
+{
+  /* Signed to signed conversion with value in-range.  */
+  /* arg + 1.  */
+  if (foo1 (-32) != -31)
+    abort ();
+  /* arg + 1.  */
+  if (foo2 (32) != 33)
+    abort ();
+
+  /* Signed to signed conversion with value not in-range.  */
+  /* arg - 1.  */
+  if (foo3 (-512) != -513)
+    abort ();
+  /* arg + 1.  */
+  if (foo4 (512) != 513)
+    abort ();
+
+  /* Unsigned to unsigned conversion with value in-range.  */
+  /* arg + 1.  */
+  if (foo5 (64) != 65)
+    abort ();
+  /* arg + 1.  */
+  if (foo6 (64) != 65)
+    abort ();
+
+  /* Unsigned to unsigned conversion with value not in-range.  */
+  /* arg + 1.  */
+  if (foo7 (512) != 513)
+    abort ();
+  /* arg + 1.  */
+  if (foo8 (512) != 513)
+    abort ();
+
+  /* Signed to unsigned conversion with value range positive.  */
+  /* arg - 1.  */
+  if (foo9 (2) != 1)
+    abort ();
+  /* arg + 1.  */
+  if (foo10 (2) != 3)
+    abort ();
+
+  /* Signed to unsigned conversion with value range negative.  */
+  /* arg + 1.  */
+  if (foo11 (-125) != -124)
+    abort ();
+  /* arg + 1.  */
+  if (foo12 (-125) != -124)
+    abort ();
+
+  /* Unsigned to Signed conversion with value range in signed equiv range.  */
+  /* arg + 1.  */
+  if (foo13 (125) != 126)
+    abort ();
+  /* arg + 1.  */
+  if (foo14 (125) != 126)
+    abort ();
+
+  /* Unsigned to Signed conversion with value range not-in signed range.  */
+  /* arg + 1.  */
+  if (foo15 (250) != 251)
+    abort ();
+  /* arg + 1.  */
+  if (foo16 (250) != 251)
+    abort ();
+
+  return 0;
+}
+