On Fri, 23 Feb 2024, Jakub Jelinek wrote:

> Hi!
> 
> The following testcase ICEs, because the REDUCE_BIT_FIELD macro uses
> the target variable implicitly:
> #define REDUCE_BIT_FIELD(expr)  (reduce_bit_field                         \
>                                  ? reduce_to_bit_field_precision ((expr), \
>                                                                   target, \
>                                                                   type)   \
>                                  : (expr))
> and so when the code below reuses the target variable, documented to be
>    The value may be stored in TARGET if TARGET is nonzero.
>    TARGET is just a suggestion; callers must assume that
>    the rtx returned may not be the same as TARGET.
> for something unrelated (the value that should be returned), this misbehaves
> (in the testcase target is set to a CONST_INT, which has VOIDmode and
> reduce_to_bit_field_precision assert checking doesn't like that).
> Needed to say that
>    If TARGET is CONST0_RTX, it means that the value will be ignored.
> but in expand_expr_real_2 does at the start:
>   ignore = (target == const0_rtx
>             || ((CONVERT_EXPR_CODE_P (code)
>                  || code == COND_EXPR || code == VIEW_CONVERT_EXPR)
>                 && TREE_CODE (type) == VOID_TYPE));
> 
>   /* We should be called only if we need the result.  */
>   gcc_assert (!ignore);
> - so such target is mainly meant for calls and the like in other routines.
> Certainly doesn't expect that target changes from not being ignored
> initially to ignore later on and other CONST_INT results as well as anything
> which is not an object into which anything can be stored.
> 
> So, the following patch fixes that by using a more appripriate temporary
> for the result, which other code is using.
> 
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

OK.

Thanks,
Richard.

> 2024-02-23  Jakub Jelinek  <ja...@redhat.com>
> 
>       PR rtl-optimization/114054
>       * expr.cc (expand_expr_real_2) <case MULT_EXPR>: Use
>       temp variable instead of target parameter for result.
> 
>       * gcc.dg/bitint-92.c: New test.
> 
> --- gcc/expr.cc.jj    2024-02-14 14:26:19.709811397 +0100
> +++ gcc/expr.cc       2024-02-22 18:39:37.852431789 +0100
> @@ -10259,12 +10259,12 @@ expand_expr_real_2 (sepops ops, rtx targ
>                                         &algorithm, &variant, cost)
>                 : cost < mul_cost (speed, mode))
>               {
> -               target = bit0_p ? expand_and (mode, negate_rtx (mode, op0),
> -                                             op1, target)
> -                               : expand_and (mode, op0,
> -                                             negate_rtx (mode, op1),
> -                                             target);
> -               return REDUCE_BIT_FIELD (target);
> +               temp = bit0_p ? expand_and (mode, negate_rtx (mode, op0),
> +                                           op1, target)
> +                             : expand_and (mode, op0,
> +                                           negate_rtx (mode, op1),
> +                                           target);
> +               return REDUCE_BIT_FIELD (temp);
>               }
>           }
>       }
> --- gcc/testsuite/gcc.dg/bitint-92.c.jj       2024-02-22 18:43:56.433910671 
> +0100
> +++ gcc/testsuite/gcc.dg/bitint-92.c  2024-02-22 18:43:29.464277919 +0100
> @@ -0,0 +1,17 @@
> +/* PR rtl-optimization/114054 */
> +/* { dg-do compile { target bitint } } */
> +/* { dg-options "-Og -fwhole-program -fno-tree-ccp -fprofile-use 
> -fno-tree-copy-prop -w" } */
> +
> +int x;
> +
> +void
> +foo (int i, unsigned u)
> +{
> +  x = __builtin_mul_overflow_p ((unsigned _BitInt(1)) u, i, (_BitInt(33)) 0);
> +}
> +
> +int
> +main ()
> +{
> +  foo (11, 0);
> +}
> 
>       Jakub
> 
> 

-- 
Richard Biener <rguent...@suse.de>
SUSE Software Solutions Germany GmbH,
Frankenstrasse 146, 90461 Nuernberg, Germany;
GF: Ivo Totev, Andrew McDonald, Werner Knoblich; (HRB 36809, AG Nuernberg)

Reply via email to