Re: [PATCH] Add define_insn_and_split for combine to detect x < 123U ? -1 : 0 (PR target/88425)

[Uros Bizjak <ubizjak at gmail dot com>]

On Tue, Dec 11, 2018 at 8:27 AM Jakub Jelinek <jakub@redhat.com> wrote:
>
> Hi!
>
> For the following testcase (x < 123U ? -1U : 0) we emit worse code than for
> (x < y ? -1U : 0).  That is because the generic code canonicalizes x < 123U
> comparisons into x <= 122U.  For this particular case, we want LTU though,
> as that sets carry and we can then just sbb.
>
> The following patch adds an insn with splitter for this.
>
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
>
> 2018-12-11  Jakub Jelinek  <jakub@redhat.com>
>
>         PR target/88425
>         * config/i386/i386.md (*x86_mov<SWI48:mode>cc_0_m1_neg_leu<SWI:mode>):
>         New define_insn_and_split.
>
>         * gcc.target/i386/pr88425.c: New test.
>
> --- gcc/config/i386/i386.md.jj  2018-11-22 10:40:31.179683319 +0100
> +++ gcc/config/i386/i386.md     2018-12-10 11:24:49.785830186 +0100
> @@ -17195,6 +17195,24 @@ (define_insn "*x86_mov<mode>cc_0_m1_neg"
>     (set_attr "mode" "<MODE>")
>     (set_attr "length_immediate" "0")])
>
> +(define_insn_and_split "*x86_mov<SWI48:mode>cc_0_m1_neg_leu<SWI:mode>"
> +  [(set (match_operand:SWI48 0 "register_operand" "=r")
> +       (neg:SWI48
> +         (leu:SWI48
> +           (match_operand:SWI 1 "nonimmediate_operand" "<SWI:r>m")
> +           (match_operand:SWI 2 "<SWI:immediate_operand>" "<SWI:i>"))))

You can use const_int_operand predicate with "n" constraint here.

> +   (clobber (reg:CC FLAGS_REG))]
> +  "CONST_INT_P (operands[2])
> +   && INTVAL (operands[2]) != -1
> +   && INTVAL (operands[2]) != 2147483647"

Can UINTVAL be used here?

Uros.

> +  "#"
> +  ""
> +  [(set (reg:CC FLAGS_REG) (compare:CC (match_dup 1) (match_dup 2)))
> +   (parallel [(set (match_dup 0)
> +                  (neg:SWI48 (ltu:SWI48 (reg:CC FLAGS_REG) (const_int 0))))
> +             (clobber (reg:CC FLAGS_REG))])]
> +  "operands[2] = GEN_INT (INTVAL (operands[2]) + 1);")
> +
>  (define_insn "*mov<mode>cc_noc"
>    [(set (match_operand:SWI248 0 "register_operand" "=r,r")
>         (if_then_else:SWI248 (match_operator 1 "ix86_comparison_operator"
> --- gcc/testsuite/gcc.target/i386/pr88425.c.jj  2018-12-10 11:31:35.507196453 +0100
> +++ gcc/testsuite/gcc.target/i386/pr88425.c     2018-12-10 11:31:17.231495274 +0100
> @@ -0,0 +1,53 @@
> +/* PR target/88425 */
> +/* { dg-do compile } */
> +/* { dg-options "-O2 -masm=att" } */
> +/* { dg-final { scan-assembler-times "sbb\[lq]\[ \t]" 8 } } */
> +/* { dg-final { scan-assembler-not "setbe\[ \t]" } } */
> +
> +unsigned long
> +f1 (unsigned long x)
> +{
> +  return x < 123UL ? -1UL : 0;
> +}
> +
> +unsigned long
> +f2 (unsigned int x)
> +{
> +  return x < 12345U ? -1UL : 0;
> +}
> +
> +unsigned long
> +f3 (unsigned short *x)
> +{
> +  return x[0] < 1234U ? -1UL : 0;
> +}
> +
> +unsigned long
> +f4 (unsigned char *x)
> +{
> +  return x[0] < 123U ? -1UL : 0;
> +}
> +
> +unsigned int
> +f5 (unsigned long x)
> +{
> +  return x < 123UL ? -1U : 0;
> +}
> +
> +unsigned int
> +f6 (unsigned int x)
> +{
> +  return x < 12345U ? -1U : 0;
> +}
> +
> +unsigned int
> +f7 (unsigned short *x)
> +{
> +  return x[0] < 1234U ? -1U : 0;
> +}
> +
> +unsigned int
> +f8 (unsigned char *x)
> +{
> +  return x[0] < 123U ? -1U : 0;
> +}
>
>         Jakub