On Mon, May 14, 2018 at 03:07:54PM +0200, Jakub Jelinek wrote:
> On Mon, May 14, 2018 at 02:54:18PM +0200, Uros Bizjak wrote:
> > > --- gcc/config/i386/i386.md.jj 2018-05-11 18:44:32.000000000 +0200
> > > +++ gcc/config/i386/i386.md 2018-05-14 13:50:28.100482520 +0200
> > > @@ -19397,11 +19397,11 @@
> > > (set (match_dup 0) (match_dup 2))])
> > > (set (match_dup 1) (match_dup 0))]
> > > "(TARGET_READ_MODIFY_WRITE || optimize_insn_for_size_p ())
> > > + && GET_CODE (operands[2]) != MINUS
> >
> > && COMMUTATIVE_ARITH_P (operands[2])
>
> That works, but then we should change the peephole2 above it as well.
> MINUS is the only non-commutative plusminuslogic_operator, so it doesn't
> change any behavior.
>
> BTW, wonder if we shouldn't allow giving peephole2's a name, it is too hard
> to refer to them...
It might be too late but I've regtested/bootstrapped this patch on x86_64-linux.
> 2018-05-14 Jakub Jelinek <ja...@redhat.com>
>
> PR target/85756
> * config/i386/i386.md: Disallow non-commutative arithmetics in
> last twpeephole for mem {+,-,&,|,^}= x; mem != 0 after cmpelim
> optimization. Use COMMUTATIVE_ARITH_P test rather than != MINUS
> in the peephole2 before it.
>
> * gcc.c-torture/execute/pr85756.c: New test.
>
> --- gcc/config/i386/i386.md.jj 2018-05-11 18:44:32.000000000 +0200
> +++ gcc/config/i386/i386.md 2018-05-14 15:02:48.363662960 +0200
> @@ -19367,7 +19367,7 @@ (define_peephole2
> (set (match_dup 1) (match_dup 0))
> (set (reg FLAGS_REG) (compare (match_dup 0) (const_int 0)))]
> "(TARGET_READ_MODIFY_WRITE || optimize_insn_for_size_p ())
> - && GET_CODE (operands[2]) != MINUS
> + && COMMUTATIVE_ARITH_P (operands[2])
> && peep2_reg_dead_p (3, operands[0])
> && !reg_overlap_mentioned_p (operands[0], operands[1])
> && ix86_match_ccmode (peep2_next_insn (2),
> @@ -19397,11 +19397,11 @@ (define_peephole2
> (set (match_dup 0) (match_dup 2))])
> (set (match_dup 1) (match_dup 0))]
> "(TARGET_READ_MODIFY_WRITE || optimize_insn_for_size_p ())
> + && COMMUTATIVE_ARITH_P (operands[2])
> && peep2_reg_dead_p (2, operands[0])
> && !reg_overlap_mentioned_p (operands[0], operands[1])
> && ix86_match_ccmode (peep2_next_insn (0),
> - (GET_CODE (operands[2]) == PLUS
> - || GET_CODE (operands[2]) == MINUS)
> + GET_CODE (operands[2]) == PLUS
> ? CCGOCmode : CCNOmode)"
> [(parallel [(set (match_dup 3) (match_dup 5))
> (set (match_dup 1) (match_dup 4))])]
> --- gcc/testsuite/gcc.c-torture/execute/pr85756.c.jj 2018-05-14
> 13:50:44.384307289 +0200
> +++ gcc/testsuite/gcc.c-torture/execute/pr85756.c 2018-05-14
> 13:48:17.000000000 +0200
> @@ -0,0 +1,50 @@
> +/* PR target/85756 */
> +
> +#if __CHAR_BIT__ == 8 && __SIZEOF_SHORT__ == 2 && __SIZEOF_INT__ == 4
> +int a, c, *e, f, h = 10;
> +short b;
> +unsigned int p;
> +
> +__attribute__((noipa)) void
> +bar (int a)
> +{
> + asm volatile ("" : : "r" (a) : "memory");
> +}
> +
> +void
> +foo ()
> +{
> + unsigned j = 1, m = 430523;
> + int k, n = 1, *l = &h;
> +lab:
> + p = m;
> + m = -((~65535U | j) - n);
> + f = b << ~(n - 8);
> + n = (m || b) ^ f;
> + j = p;
> + if (p < m)
> + *l = k < 3;
> + if (!n)
> + l = &k;
> + if (c)
> + {
> + bar (a);
> + goto lab;
> + }
> + if (!*l)
> + *e = 1;
> +}
> +
> +int
> +main ()
> +{
> + foo ();
> + return 0;
> +}
> +#else
> +int
> +main ()
> +{
> + return 0;
> +}
> +#endif
>
>
> Jakub
Marek
