Message ID | 20180626105223.GO7166@tucnak |
---|---|
State | New |
Headers | show |
Series | Fix x86 setcc + movzbl peephole2s (PR target/86314) | expand |
On Tue, Jun 26, 2018 at 12:52 PM, Jakub Jelinek <jakub@redhat.com> wrote: > Hi! > > These peephole2s assume that when matching insns like: > [(parallel [(set (reg FLAGS_REG) (match_operand 0)) > (match_operand 4)]) > that operands[4] must be a set of a register with operands[0] > as SET_SRC, but that isn't the case e.g. for: > (insn 9 8 10 2 (parallel [ > (set (reg:CCC 17 flags) > (compare:CCC (unspec_volatile:DI [ > (mem/v:DI (reg/v/f:DI 5 di [orig:94 p ] [94]) [-1 S8 A64]) > (const_int 5 [0x5]) > ] UNSPECV_XCHG) > (const_int 0 [0]))) > (set (zero_extract:DI (mem/v:DI (reg/v/f:DI 5 di [orig:94 p ] [94]) [-1 S8 A64]) > (const_int 1 [0x1]) > (reg:DI 0 ax [96])) > (const_int 1 [0x1])) > ]) "pr86314.C":7 5268 {atomic_bit_test_and_setdi_1} > (expr_list:REG_DEAD (reg/v/f:DI 5 di [orig:94 p ] [94]) > (expr_list:REG_DEAD (reg:DI 0 ax [96]) > (nil)))) > > As we want to clear operands[3] before this instruction, we need to > verify that it isn't used in operands[0] (we check that) and that > operands[4] does not set it (also checked), but also need to check that > operands[4] doesn't use it (which we don't do). In the usual case > when SET_DEST of operands[4] is a REG and SET_SRC is operands[0] it > makes no difference, but if SET_DEST sets something that uses operands[3] > in it, or if SET_SRC is different from operands[0] and uses operands[3], > then this results in a wrong peephole2 transformation. > > Fixed thusly, bootstrapped/regtested on x86_64-linux and i686-linux, ok for > trunk and release branches? > > 2018-06-26 Jakub Jelinek <jakub@redhat.com> > > PR target/86314 > * config/i386/i386.md (setcc + movzbl to xor + setcc peephole2s): > Check reg_overlap_mentioned_p in addition to reg_set_p with the same > operands. > > * gcc.dg/pr86314.c: New test. OK everywhere. Thanks, Uros. > --- gcc/config/i386/i386.md.jj 2018-06-25 14:51:24.878990827 +0200 > +++ gcc/config/i386/i386.md 2018-06-26 10:01:43.042907384 +0200 > @@ -12761,6 +12761,7 @@ (define_peephole2 > "(peep2_reg_dead_p (3, operands[1]) > || operands_match_p (operands[1], operands[3])) > && ! reg_overlap_mentioned_p (operands[3], operands[0]) > + && ! reg_overlap_mentioned_p (operands[3], operands[4]) > && ! reg_set_p (operands[3], operands[4]) > && peep2_regno_dead_p (0, FLAGS_REG)" > [(parallel [(set (match_dup 5) (match_dup 0)) > @@ -12786,6 +12787,7 @@ (define_peephole2 > || operands_match_p (operands[2], operands[4])) > && ! reg_overlap_mentioned_p (operands[4], operands[0]) > && ! reg_overlap_mentioned_p (operands[4], operands[1]) > + && ! reg_overlap_mentioned_p (operands[4], operands[5]) > && ! reg_set_p (operands[4], operands[5]) > && refers_to_regno_p (FLAGS_REG, operands[1], (rtx *)NULL) > && peep2_regno_dead_p (0, FLAGS_REG)" > @@ -12835,6 +12837,7 @@ (define_peephole2 > "(peep2_reg_dead_p (3, operands[1]) > || operands_match_p (operands[1], operands[3])) > && ! reg_overlap_mentioned_p (operands[3], operands[0]) > + && ! reg_overlap_mentioned_p (operands[3], operands[4]) > && ! reg_set_p (operands[3], operands[4]) > && peep2_regno_dead_p (0, FLAGS_REG)" > [(parallel [(set (match_dup 5) (match_dup 0)) > @@ -12861,6 +12864,7 @@ (define_peephole2 > || operands_match_p (operands[2], operands[4])) > && ! reg_overlap_mentioned_p (operands[4], operands[0]) > && ! reg_overlap_mentioned_p (operands[4], operands[1]) > + && ! reg_overlap_mentioned_p (operands[4], operands[5]) > && ! reg_set_p (operands[4], operands[5]) > && refers_to_regno_p (FLAGS_REG, operands[1], (rtx *)NULL) > && peep2_regno_dead_p (0, FLAGS_REG)" > --- gcc/testsuite/gcc.dg/pr86314.c.jj 2018-06-26 10:25:35.190160477 +0200 > +++ gcc/testsuite/gcc.dg/pr86314.c 2018-06-26 10:24:42.310077331 +0200 > @@ -0,0 +1,20 @@ > +// PR target/86314 > +// { dg-do run { target sync_int_long } } > +// { dg-options "-O2" } > + > +__attribute__((noinline, noclone)) unsigned long > +foo (unsigned long *p) > +{ > + unsigned long m = 1UL << ((*p & 1) ? 1 : 0); > + unsigned long n = __atomic_fetch_or (p, m, __ATOMIC_SEQ_CST); > + return (n & m) == 0; > +} > + > +int > +main () > +{ > + unsigned long v = 1; > + if (foo (&v) != 1) > + __builtin_abort (); > + return 0; > +} > > Jakub
--- gcc/config/i386/i386.md.jj 2018-06-25 14:51:24.878990827 +0200 +++ gcc/config/i386/i386.md 2018-06-26 10:01:43.042907384 +0200 @@ -12761,6 +12761,7 @@ (define_peephole2 "(peep2_reg_dead_p (3, operands[1]) || operands_match_p (operands[1], operands[3])) && ! reg_overlap_mentioned_p (operands[3], operands[0]) + && ! reg_overlap_mentioned_p (operands[3], operands[4]) && ! reg_set_p (operands[3], operands[4]) && peep2_regno_dead_p (0, FLAGS_REG)" [(parallel [(set (match_dup 5) (match_dup 0)) @@ -12786,6 +12787,7 @@ (define_peephole2 || operands_match_p (operands[2], operands[4])) && ! reg_overlap_mentioned_p (operands[4], operands[0]) && ! reg_overlap_mentioned_p (operands[4], operands[1]) + && ! reg_overlap_mentioned_p (operands[4], operands[5]) && ! reg_set_p (operands[4], operands[5]) && refers_to_regno_p (FLAGS_REG, operands[1], (rtx *)NULL) && peep2_regno_dead_p (0, FLAGS_REG)" @@ -12835,6 +12837,7 @@ (define_peephole2 "(peep2_reg_dead_p (3, operands[1]) || operands_match_p (operands[1], operands[3])) && ! reg_overlap_mentioned_p (operands[3], operands[0]) + && ! reg_overlap_mentioned_p (operands[3], operands[4]) && ! reg_set_p (operands[3], operands[4]) && peep2_regno_dead_p (0, FLAGS_REG)" [(parallel [(set (match_dup 5) (match_dup 0)) @@ -12861,6 +12864,7 @@ (define_peephole2 || operands_match_p (operands[2], operands[4])) && ! reg_overlap_mentioned_p (operands[4], operands[0]) && ! reg_overlap_mentioned_p (operands[4], operands[1]) + && ! reg_overlap_mentioned_p (operands[4], operands[5]) && ! reg_set_p (operands[4], operands[5]) && refers_to_regno_p (FLAGS_REG, operands[1], (rtx *)NULL) && peep2_regno_dead_p (0, FLAGS_REG)" --- gcc/testsuite/gcc.dg/pr86314.c.jj 2018-06-26 10:25:35.190160477 +0200 +++ gcc/testsuite/gcc.dg/pr86314.c 2018-06-26 10:24:42.310077331 +0200 @@ -0,0 +1,20 @@ +// PR target/86314 +// { dg-do run { target sync_int_long } } +// { dg-options "-O2" } + +__attribute__((noinline, noclone)) unsigned long +foo (unsigned long *p) +{ + unsigned long m = 1UL << ((*p & 1) ? 1 : 0); + unsigned long n = __atomic_fetch_or (p, m, __ATOMIC_SEQ_CST); + return (n & m) == 0; +} + +int +main () +{ + unsigned long v = 1; + if (foo (&v) != 1) + __builtin_abort (); + return 0; +}