On Thu, 2 Sep 2021, Jakub Jelinek wrote:

> Hi!
> 
> The overflow builtins work on infinite precision integers and then convert
> to the result type's precision, so any argument promotions are useless.
> The expand_arith_overflow expansion is able to demote the arguments itself
> through get_range_pos_neg and get_min_precision calls and if needed promote
> to whatever mode it decides to perform the operations in, but if there are
> any promotions it demoted, those are already expanded.  Normally combine
> would remove the useless sign or zero extensions when it sees the result
> of those is only used in a lowpart subreg, but typically those lowpart
> subregs appear multiple times in the pattern so that they describe properly
> the overflow behavior and combine gives up, so we end up with e.g.
>         movswl  %si, %esi
>         movswl  %di, %edi
>         imulw   %si, %di
>         seto    %al
> where both movswl insns are useless.
> 
> The following patch fixes it by demoting operands of the ifns (only gets
> rid of integral to integral conversions that increase precision).
> While IFN_{ADD,MUL}_OVERFLOW are commutative and just one simplify would be
> enough, IFN_SUB_OVERFLOW is not, therefore two simplifications.
> 
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

It appears we're careful to accept different typed operands on
those IFNs at least in the few places I found, thus OK.

Thanks,
Richard.

> 2021-09-02  Jakub Jelinek  <ja...@redhat.com>
> 
>       PR tree-optimization/99591
>       * match.pd: Demote operands of IFN_{ADD,SUB,MUL}_OVERFLOW if they
>       were promoted.
> 
>       * gcc.target/i386/pr99591.c: New test.
>       * gcc.target/i386/pr97950.c: Match or reject setb or jn?b instructions
>       together with seta or jn?a.
> 
> --- gcc/match.pd.jj   2021-08-30 08:36:11.226516509 +0200
> +++ gcc/match.pd      2021-09-01 10:43:15.072908430 +0200
> @@ -5587,6 +5587,21 @@ (define_operator_list COND_TERNARY
>     (with { tree t = TREE_TYPE (@0), cpx = build_complex_type (t); }
>      (cmp (imagpart (IFN_MUL_OVERFLOW:cpx @0 @1)) { build_zero_cst (t); })))))
>  
> +/* Demote operands of IFN_{ADD,SUB,MUL}_OVERFLOW.  */
> +(for ovf (IFN_ADD_OVERFLOW IFN_SUB_OVERFLOW IFN_MUL_OVERFLOW)
> + (simplify
> +  (ovf (convert@2 @0) @1)
> +  (if (INTEGRAL_TYPE_P (TREE_TYPE (@0))
> +       && INTEGRAL_TYPE_P (TREE_TYPE (@2))
> +       && TYPE_PRECISION (TREE_TYPE (@2)) > TYPE_PRECISION (TREE_TYPE (@0)))
> +   (ovf @0 @1)))
> + (simplify
> +  (ovf @1 (convert@2 @0))
> +  (if (INTEGRAL_TYPE_P (TREE_TYPE (@0))
> +       && INTEGRAL_TYPE_P (TREE_TYPE (@2))
> +       && TYPE_PRECISION (TREE_TYPE (@2)) > TYPE_PRECISION (TREE_TYPE (@0)))
> +   (ovf @1 @0))))
> +
>  /* Simplification of math builtins.  These rules must all be optimizations
>     as well as IL simplifications.  If there is a possibility that the new
>     form could be a pessimization, the rule should go in the canonicalization
> --- gcc/testsuite/gcc.target/i386/pr99591.c.jj        2021-09-01 
> 10:49:32.286556087 +0200
> +++ gcc/testsuite/gcc.target/i386/pr99591.c   2021-09-01 10:49:17.450766597 
> +0200
> @@ -0,0 +1,32 @@
> +/* PR tree-optimization/99591 */
> +/* { dg-do compile } */
> +/* { dg-options "-O2" } */
> +/* { dg-final { scan-assembler-not "\tmovs\[bw]l\t" } } */
> +
> +int
> +foo (signed char a, signed char b)
> +{
> +  signed char r;
> +  return __builtin_add_overflow (a, b, &r);
> +}
> +
> +int
> +bar (short a, short b)
> +{
> +  short r;
> +  return __builtin_add_overflow (a, b, &r);
> +}
> +
> +int
> +baz (signed char a, signed char b)
> +{
> +  signed char r;
> +  return __builtin_add_overflow ((int) a, (int) b, &r);
> +}
> +
> +int
> +qux (short a, short b)
> +{
> +  short r;
> +  return __builtin_add_overflow ((int) a, (int) b, &r);
> +}
> --- gcc/testsuite/gcc.target/i386/pr97950.c.jj        2020-11-24 
> 23:16:27.601004905 +0100
> +++ gcc/testsuite/gcc.target/i386/pr97950.c   2021-09-02 09:28:06.934382216 
> +0200
> @@ -1,10 +1,10 @@
>  /* PR target/95950 */
>  /* { dg-do compile } */
>  /* { dg-options "-O2 -mtune=generic" } */
> -/* { dg-final { scan-assembler-times "\tseta\t" 4 } } */
> +/* { dg-final { scan-assembler-times "\tset\[ab]\t" 4 } } */
>  /* { dg-final { scan-assembler-times "\tseto\t" 16 } } */
>  /* { dg-final { scan-assembler-times "\tsetc\t" 4 } } */
> -/* { dg-final { scan-assembler-not "\tjn?a\t" } } */
> +/* { dg-final { scan-assembler-not "\tjn?\[ab]\t" } } */
>  /* { dg-final { scan-assembler-not "\tjn?o\t" } } */
>  /* { dg-final { scan-assembler-not "\tjn?c\t" } } */
>  
> 
>       Jakub
> 
> 

-- 
Richard Biener <rguent...@suse.de>
SUSE Software Solutions Germany GmbH, Maxfeldstrasse 5, 90409 Nuernberg,
Germany; GF: Felix Imendörffer; HRB 36809 (AG Nuernberg)

Reply via email to