| Message ID | 20180502074246.GR8577@tucnak |
|---|---|
| State | New |
| Headers | show |
| Series | Fix the x86 *_doubleword_mask* patterns (PR target/85582) | expand |
On Wed, May 2, 2018 at 9:42 AM, Jakub Jelinek <jakub@redhat.com> wrote: > Hi! > > operands[2] is an input operand in these define_insn_and_split patterns, > so when the masking needs to be still performed, we shouldn't modify that > input register; all the patterns are guarded with can_create_pseudo_p, > so we can create new pseudos there. > > Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk? > > 2018-05-02 Jakub Jelinek <jakub@redhat.com> > > PR target/85582 > * config/i386/i386.md (*ashl<dwi>3_doubleword_mask, > *ashl<dwi>3_doubleword_mask_1, *<shift_insn><dwi>3_doubleword_mask, > *<shift_insn><dwi>3_doubleword_mask_1): If and[sq]i3 is needed, don't > clobber operands[2], instead use a new pseudo. Formatting fixes. > > * gcc.c-torture/execute/pr85582-1.c: New test. > * gcc.c-torture/execute/pr85582-2.c: New test. O. Thanks, Uros. > --- gcc/config/i386/i386.md.jj 2018-05-01 12:18:15.566832552 +0200 > +++ gcc/config/i386/i386.md 2018-05-01 13:09:36.635164943 +0200 > @@ -10366,7 +10366,7 @@ (define_insn_and_split "*ashl<dwi>3_doub > (match_operand:SI 2 "register_operand" "c") > (match_operand:SI 3 "const_int_operand")) 0))) > (clobber (reg:CC FLAGS_REG))] > - "INTVAL (operands[3]) <= (<MODE_SIZE> * BITS_PER_UNIT)-1 > + "INTVAL (operands[3]) <= (<MODE_SIZE> * BITS_PER_UNIT) - 1 > && can_create_pseudo_p ()" > "#" > "&& 1" > @@ -10385,8 +10385,12 @@ (define_insn_and_split "*ashl<dwi>3_doub > > operands[8] = GEN_INT (<MODE_SIZE> * BITS_PER_UNIT); > > - if (INTVAL (operands[3]) < (<MODE_SIZE> * BITS_PER_UNIT)-1) > - emit_insn (gen_andsi3 (operands[2], operands[2], operands[3])); > + if (INTVAL (operands[3]) < (<MODE_SIZE> * BITS_PER_UNIT) - 1) > + { > + rtx tem = gen_reg_rtx (SImode); > + emit_insn (gen_andsi3 (tem, operands[2], operands[3])); > + operands[2] = tem; > + } > > operands[2] = gen_lowpart (QImode, operands[2]); > > @@ -10402,7 +10406,7 @@ (define_insn_and_split "*ashl<dwi>3_doub > (match_operand:QI 2 "register_operand" "c") > (match_operand:QI 3 "const_int_operand")))) > (clobber (reg:CC FLAGS_REG))] > - "INTVAL (operands[3]) <= (<MODE_SIZE> * BITS_PER_UNIT)-1 > + "INTVAL (operands[3]) <= (<MODE_SIZE> * BITS_PER_UNIT) - 1 > && can_create_pseudo_p ()" > "#" > "&& 1" > @@ -10421,8 +10425,12 @@ (define_insn_and_split "*ashl<dwi>3_doub > > operands[8] = GEN_INT (<MODE_SIZE> * BITS_PER_UNIT); > > - if (INTVAL (operands[3]) < (<MODE_SIZE> * BITS_PER_UNIT)-1) > - emit_insn (gen_andqi3 (operands[2], operands[2], operands[3])); > + if (INTVAL (operands[3]) < (<MODE_SIZE> * BITS_PER_UNIT) - 1) > + { > + rtx tem = gen_reg_rtx (QImode); > + emit_insn (gen_andqi3 (tem, operands[2], operands[3])); > + operands[2] = tem; > + } > > if (!rtx_equal_p (operands[6], operands[7])) > emit_move_insn (operands[6], operands[7]); > @@ -11118,7 +11126,7 @@ (define_insn_and_split "*<shift_insn><dw > (match_operand:SI 2 "register_operand" "c") > (match_operand:SI 3 "const_int_operand")) 0))) > (clobber (reg:CC FLAGS_REG))] > - "INTVAL (operands[3]) <= (<MODE_SIZE> * BITS_PER_UNIT)-1 > + "INTVAL (operands[3]) <= (<MODE_SIZE> * BITS_PER_UNIT) - 1 > && can_create_pseudo_p ()" > "#" > "&& 1" > @@ -11138,7 +11146,11 @@ (define_insn_and_split "*<shift_insn><dw > operands[8] = GEN_INT (<MODE_SIZE> * BITS_PER_UNIT); > > if (INTVAL (operands[3]) < (<MODE_SIZE> * BITS_PER_UNIT)-1) > - emit_insn (gen_andsi3 (operands[2], operands[2], operands[3])); > + { > + rtx tem = gen_reg_rtx (SImode); > + emit_insn (gen_andsi3 (tem, operands[2], operands[3])); > + operands[2] = tem; > + } > > operands[2] = gen_lowpart (QImode, operands[2]); > > @@ -11154,7 +11166,7 @@ (define_insn_and_split "*<shift_insn><dw > (match_operand:QI 2 "register_operand" "c") > (match_operand:QI 3 "const_int_operand")))) > (clobber (reg:CC FLAGS_REG))] > - "INTVAL (operands[3]) <= (<MODE_SIZE> * BITS_PER_UNIT)-1 > + "INTVAL (operands[3]) <= (<MODE_SIZE> * BITS_PER_UNIT) - 1 > && can_create_pseudo_p ()" > "#" > "&& 1" > @@ -11173,8 +11185,12 @@ (define_insn_and_split "*<shift_insn><dw > > operands[8] = GEN_INT (<MODE_SIZE> * BITS_PER_UNIT); > > - if (INTVAL (operands[3]) < (<MODE_SIZE> * BITS_PER_UNIT)-1) > - emit_insn (gen_andqi3 (operands[2], operands[2], operands[3])); > + if (INTVAL (operands[3]) < (<MODE_SIZE> * BITS_PER_UNIT) - 1) > + { > + rtx tem = gen_reg_rtx (QImode); > + emit_insn (gen_andqi3 (tem, operands[2], operands[3])); > + operands[2] = tem; > + } > > if (!rtx_equal_p (operands[4], operands[5])) > emit_move_insn (operands[4], operands[5]); > --- gcc/testsuite/gcc.c-torture/execute/pr85582-1.c.jj 2018-05-01 13:28:07.547933727 +0200 > +++ gcc/testsuite/gcc.c-torture/execute/pr85582-1.c 2018-05-01 13:26:32.270868649 +0200 > @@ -0,0 +1,21 @@ > +/* PR target/85582 */ > + > +int a, b, d = 2, e; > +long long c = 1; > + > +int > +main () > +{ > + int g = 6; > +L1: > + e = d; > + if (a) > + goto L1; > + g--; > + int i = c >> ~(~e | ~g); > +L2: > + c = (b % c) * i; > + if (!e) > + goto L2; > + return 0; > +} > --- gcc/testsuite/gcc.c-torture/execute/pr85582-2.c.jj 2018-05-01 13:28:10.332935636 +0200 > +++ gcc/testsuite/gcc.c-torture/execute/pr85582-2.c 2018-05-01 13:28:00.615929000 +0200 > @@ -0,0 +1,51 @@ > +/* PR target/85582 */ > + > +#ifdef __SIZEOF_INT128__ > +typedef __int128 S; > +typedef unsigned __int128 U; > +#else > +typedef long long S; > +typedef unsigned long long U; > +#endif > + > +__attribute__((noipa)) S > +f1 (S x, int y) > +{ > + x = x << (y & 5); > + x += y; > + return x; > +} > + > +__attribute__((noipa)) S > +f2 (S x, int y) > +{ > + x = x >> (y & 5); > + x += y; > + return x; > +} > + > +__attribute__((noipa)) U > +f3 (U x, int y) > +{ > + x = x >> (y & 5); > + x += y; > + return x; > +} > + > +int > +main () > +{ > + S a = (S) 1 << (sizeof (S) * __CHAR_BIT__ - 7); > + S b = f1 (a, 12); > + if (b != ((S) 1 << (sizeof (S) * __CHAR_BIT__ - 3)) + 12) > + __builtin_abort (); > + S c = (U) 1 << (sizeof (S) * __CHAR_BIT__ - 1); > + S d = f2 (c, 12); > + if ((U) d != ((U) 0x1f << (sizeof (S) * __CHAR_BIT__ - 5)) + 12) > + __builtin_abort (); > + U e = (U) 1 << (sizeof (U) * __CHAR_BIT__ - 1); > + U f = f3 (c, 12); > + if (f != ((U) 1 << (sizeof (U) * __CHAR_BIT__ - 5)) + 12) > + __builtin_abort (); > + return 0; > +} > > Jakub
On Wed, May 02, 2018 at 09:50:07AM +0200, Uros Bizjak wrote: > > 2018-05-02 Jakub Jelinek <jakub@redhat.com> > > > > PR target/85582 > > * config/i386/i386.md (*ashl<dwi>3_doubleword_mask, > > *ashl<dwi>3_doubleword_mask_1, *<shift_insn><dwi>3_doubleword_mask, > > *<shift_insn><dwi>3_doubleword_mask_1): If and[sq]i3 is needed, don't > > clobber operands[2], instead use a new pseudo. Formatting fixes. > > > > * gcc.c-torture/execute/pr85582-1.c: New test. > > * gcc.c-torture/execute/pr85582-2.c: New test. > > O. Thanks, committed. BTW, thinking about these some more, isn't INTVAL (operands[3]) <= (<MODE_SIZE> * BITS_PER_UNIT) - 1 incorrect? Say for <MODE_SIZE> being 4 this is INTVAL (operands[3]) <= 31 so it will accept & 0 to & 31 (that is correct), or e.g. & -2 (that is incorrect). Isn't the right guarding condition that (INTVAL (operands[3]) & (<MODE_SIZE> * BITS_PER_UNIT)) == 0 i.e. that we have guarantee the shift count doesn't have the topmost relevant bit set and we can ignore bits above it (UB)? And for the decision if we should use a masking or not perhaps if ((INTVAL (operands[3]) & ((<MODE_SIZE> * BITS_PER_UNIT) - 1)) != (<MODE_SIZE> * BITS_PER_UNIT) - 1) , again ignoring bits we don't care about. > > --- gcc/config/i386/i386.md.jj 2018-05-01 12:18:15.566832552 +0200 > > +++ gcc/config/i386/i386.md 2018-05-01 13:09:36.635164943 +0200 > > @@ -10366,7 +10366,7 @@ (define_insn_and_split "*ashl<dwi>3_doub > > (match_operand:SI 2 "register_operand" "c") > > (match_operand:SI 3 "const_int_operand")) 0))) > > (clobber (reg:CC FLAGS_REG))] > > - "INTVAL (operands[3]) <= (<MODE_SIZE> * BITS_PER_UNIT)-1 > > + "INTVAL (operands[3]) <= (<MODE_SIZE> * BITS_PER_UNIT) - 1 > > && can_create_pseudo_p ()" > > "#" > > "&& 1" > > @@ -10385,8 +10385,12 @@ (define_insn_and_split "*ashl<dwi>3_doub > > > > operands[8] = GEN_INT (<MODE_SIZE> * BITS_PER_UNIT); > > > > - if (INTVAL (operands[3]) < (<MODE_SIZE> * BITS_PER_UNIT)-1) > > - emit_insn (gen_andsi3 (operands[2], operands[2], operands[3])); > > + if (INTVAL (operands[3]) < (<MODE_SIZE> * BITS_PER_UNIT) - 1) > > + { > > + rtx tem = gen_reg_rtx (SImode); > > + emit_insn (gen_andsi3 (tem, operands[2], operands[3])); > > + operands[2] = tem; > > + } > > > > operands[2] = gen_lowpart (QImode, operands[2]); > > Jakub
On Wed, May 2, 2018 at 10:02 AM, Jakub Jelinek <jakub@redhat.com> wrote: > On Wed, May 02, 2018 at 09:50:07AM +0200, Uros Bizjak wrote: >> > 2018-05-02 Jakub Jelinek <jakub@redhat.com> >> > >> > PR target/85582 >> > * config/i386/i386.md (*ashl<dwi>3_doubleword_mask, >> > *ashl<dwi>3_doubleword_mask_1, *<shift_insn><dwi>3_doubleword_mask, >> > *<shift_insn><dwi>3_doubleword_mask_1): If and[sq]i3 is needed, don't >> > clobber operands[2], instead use a new pseudo. Formatting fixes. >> > >> > * gcc.c-torture/execute/pr85582-1.c: New test. >> > * gcc.c-torture/execute/pr85582-2.c: New test. >> >> O. > > Thanks, committed. BTW, thinking about these some more, > isn't INTVAL (operands[3]) <= (<MODE_SIZE> * BITS_PER_UNIT) - 1 > incorrect? Say for <MODE_SIZE> being 4 this is INTVAL (operands[3]) <= 31 > so it will accept & 0 to & 31 (that is correct), or e.g. & -2 (that is > incorrect). Hm, I wasn't thinking about undefined cases here. > Isn't the right guarding condition that > (INTVAL (operands[3]) & (<MODE_SIZE> * BITS_PER_UNIT)) == 0 > i.e. that we have guarantee the shift count doesn't have the topmost > relevant bit set and we can ignore bits above it (UB)? Yes, based on above, it looks more robust, indeed. > And for the decision if we should use a masking or not perhaps > if ((INTVAL (operands[3]) & ((<MODE_SIZE> * BITS_PER_UNIT) - 1)) > != (<MODE_SIZE> * BITS_PER_UNIT) - 1) > , again ignoring bits we don't care about. Yes, looks more robust vs. UB, too (but updated guarding condition won't allow UB values here). Let's also change this, for consistency. The patch that changes the condition to your approach is pre-approved. Thanks, Uros. >> > --- gcc/config/i386/i386.md.jj 2018-05-01 12:18:15.566832552 +0200 >> > +++ gcc/config/i386/i386.md 2018-05-01 13:09:36.635164943 +0200 >> > @@ -10366,7 +10366,7 @@ (define_insn_and_split "*ashl<dwi>3_doub >> > (match_operand:SI 2 "register_operand" "c") >> > (match_operand:SI 3 "const_int_operand")) 0))) >> > (clobber (reg:CC FLAGS_REG))] >> > - "INTVAL (operands[3]) <= (<MODE_SIZE> * BITS_PER_UNIT)-1 >> > + "INTVAL (operands[3]) <= (<MODE_SIZE> * BITS_PER_UNIT) - 1 >> > && can_create_pseudo_p ()" >> > "#" >> > "&& 1" >> > @@ -10385,8 +10385,12 @@ (define_insn_and_split "*ashl<dwi>3_doub >> > >> > operands[8] = GEN_INT (<MODE_SIZE> * BITS_PER_UNIT); >> > >> > - if (INTVAL (operands[3]) < (<MODE_SIZE> * BITS_PER_UNIT)-1) >> > - emit_insn (gen_andsi3 (operands[2], operands[2], operands[3])); >> > + if (INTVAL (operands[3]) < (<MODE_SIZE> * BITS_PER_UNIT) - 1) >> > + { >> > + rtx tem = gen_reg_rtx (SImode); >> > + emit_insn (gen_andsi3 (tem, operands[2], operands[3])); >> > + operands[2] = tem; >> > + } >> > >> > operands[2] = gen_lowpart (QImode, operands[2]); >> > > > Jakub
On Wed, May 02, 2018 at 10:09:22AM +0200, Uros Bizjak wrote: > On Wed, May 2, 2018 at 10:02 AM, Jakub Jelinek <jakub@redhat.com> wrote: > > On Wed, May 02, 2018 at 09:50:07AM +0200, Uros Bizjak wrote: > >> > 2018-05-02 Jakub Jelinek <jakub@redhat.com> > >> > > >> > PR target/85582 > >> > * config/i386/i386.md (*ashl<dwi>3_doubleword_mask, > >> > *ashl<dwi>3_doubleword_mask_1, *<shift_insn><dwi>3_doubleword_mask, > >> > *<shift_insn><dwi>3_doubleword_mask_1): If and[sq]i3 is needed, don't > >> > clobber operands[2], instead use a new pseudo. Formatting fixes. > >> > > >> > * gcc.c-torture/execute/pr85582-1.c: New test. > >> > * gcc.c-torture/execute/pr85582-2.c: New test. > >> > >> O. > > > > Thanks, committed. BTW, thinking about these some more, > > isn't INTVAL (operands[3]) <= (<MODE_SIZE> * BITS_PER_UNIT) - 1 > > incorrect? Say for <MODE_SIZE> being 4 this is INTVAL (operands[3]) <= 31 > > so it will accept & 0 to & 31 (that is correct), or e.g. & -2 (that is > > incorrect). > > Hm, I wasn't thinking about undefined cases here. It isn't just about undefined cases, see the testcase below which is miscompiled because of these checks. Whether some bit in the mask is set or not still doesn't imply anything on whether the other operand of the bit and will have those bits set or clear. > > Isn't the right guarding condition that > > (INTVAL (operands[3]) & (<MODE_SIZE> * BITS_PER_UNIT)) == 0 > > i.e. that we have guarantee the shift count doesn't have the topmost > > relevant bit set and we can ignore bits above it (UB)? > > Yes, based on above, it looks more robust, indeed. > > > And for the decision if we should use a masking or not perhaps > > if ((INTVAL (operands[3]) & ((<MODE_SIZE> * BITS_PER_UNIT) - 1)) > > != (<MODE_SIZE> * BITS_PER_UNIT) - 1) > > , again ignoring bits we don't care about. > > Yes, looks more robust vs. UB, too (but updated guarding condition > won't allow UB values here). Let's also change this, for consistency. > > The patch that changes the condition to your approach is pre-approved. This is what I'll bootstrap/regtest tonight. 2018-05-03 Jakub Jelinek <jakub@redhat.com> PR target/85582 * config/i386/i386.md (*ashl<dwi>3_doubleword_mask, *ashl<dwi>3_doubleword_mask_1, *<shift_insn><dwi>3_doubleword_mask, *<shift_insn><dwi>3_doubleword_mask_1): In condition require that the highest significant bit of the shift count mask is clear. In check whether and[sq]i3 is needed verify that all significant bits of the shift count other than the highest are set. * gcc.c-torture/execute/pr85582-3.c: New test. --- gcc/config/i386/i386.md.jj 2018-05-02 09:51:24.898661931 +0200 +++ gcc/config/i386/i386.md 2018-05-02 14:58:58.581494664 +0200 @@ -10366,7 +10366,7 @@ (define_insn_and_split "*ashl<dwi>3_doub (match_operand:SI 2 "register_operand" "c") (match_operand:SI 3 "const_int_operand")) 0))) (clobber (reg:CC FLAGS_REG))] - "INTVAL (operands[3]) <= (<MODE_SIZE> * BITS_PER_UNIT) - 1 + "(INTVAL (operands[3]) & (<MODE_SIZE> * BITS_PER_UNIT)) == 0 && can_create_pseudo_p ()" "#" "&& 1" @@ -10385,7 +10385,8 @@ (define_insn_and_split "*ashl<dwi>3_doub operands[8] = GEN_INT (<MODE_SIZE> * BITS_PER_UNIT); - if (INTVAL (operands[3]) < (<MODE_SIZE> * BITS_PER_UNIT) - 1) + if ((INTVAL (operands[3]) & ((<MODE_SIZE> * BITS_PER_UNIT) - 1)) + != ((<MODE_SIZE> * BITS_PER_UNIT) - 1)) { rtx tem = gen_reg_rtx (SImode); emit_insn (gen_andsi3 (tem, operands[2], operands[3])); @@ -10406,7 +10407,7 @@ (define_insn_and_split "*ashl<dwi>3_doub (match_operand:QI 2 "register_operand" "c") (match_operand:QI 3 "const_int_operand")))) (clobber (reg:CC FLAGS_REG))] - "INTVAL (operands[3]) <= (<MODE_SIZE> * BITS_PER_UNIT) - 1 + "(INTVAL (operands[3]) & (<MODE_SIZE> * BITS_PER_UNIT)) == 0 && can_create_pseudo_p ()" "#" "&& 1" @@ -10425,7 +10426,8 @@ (define_insn_and_split "*ashl<dwi>3_doub operands[8] = GEN_INT (<MODE_SIZE> * BITS_PER_UNIT); - if (INTVAL (operands[3]) < (<MODE_SIZE> * BITS_PER_UNIT) - 1) + if ((INTVAL (operands[3]) & ((<MODE_SIZE> * BITS_PER_UNIT) - 1)) + != ((<MODE_SIZE> * BITS_PER_UNIT) - 1)) { rtx tem = gen_reg_rtx (QImode); emit_insn (gen_andqi3 (tem, operands[2], operands[3])); @@ -11126,7 +11128,7 @@ (define_insn_and_split "*<shift_insn><dw (match_operand:SI 2 "register_operand" "c") (match_operand:SI 3 "const_int_operand")) 0))) (clobber (reg:CC FLAGS_REG))] - "INTVAL (operands[3]) <= (<MODE_SIZE> * BITS_PER_UNIT) - 1 + "(INTVAL (operands[3]) & (<MODE_SIZE> * BITS_PER_UNIT)) == 0 && can_create_pseudo_p ()" "#" "&& 1" @@ -11145,7 +11147,8 @@ (define_insn_and_split "*<shift_insn><dw operands[8] = GEN_INT (<MODE_SIZE> * BITS_PER_UNIT); - if (INTVAL (operands[3]) < (<MODE_SIZE> * BITS_PER_UNIT)-1) + if ((INTVAL (operands[3]) & ((<MODE_SIZE> * BITS_PER_UNIT) - 1)) + != ((<MODE_SIZE> * BITS_PER_UNIT) - 1)) { rtx tem = gen_reg_rtx (SImode); emit_insn (gen_andsi3 (tem, operands[2], operands[3])); @@ -11166,7 +11169,7 @@ (define_insn_and_split "*<shift_insn><dw (match_operand:QI 2 "register_operand" "c") (match_operand:QI 3 "const_int_operand")))) (clobber (reg:CC FLAGS_REG))] - "INTVAL (operands[3]) <= (<MODE_SIZE> * BITS_PER_UNIT) - 1 + "(INTVAL (operands[3]) & (<MODE_SIZE> * BITS_PER_UNIT)) == 0 && can_create_pseudo_p ()" "#" "&& 1" @@ -11185,7 +11188,8 @@ (define_insn_and_split "*<shift_insn><dw operands[8] = GEN_INT (<MODE_SIZE> * BITS_PER_UNIT); - if (INTVAL (operands[3]) < (<MODE_SIZE> * BITS_PER_UNIT) - 1) + if ((INTVAL (operands[3]) & ((<MODE_SIZE> * BITS_PER_UNIT) - 1)) + != ((<MODE_SIZE> * BITS_PER_UNIT) - 1)) { rtx tem = gen_reg_rtx (QImode); emit_insn (gen_andqi3 (tem, operands[2], operands[3])); --- gcc/testsuite/gcc.c-torture/execute/pr85582-3.c.jj 2018-05-02 15:02:23.655654507 +0200 +++ gcc/testsuite/gcc.c-torture/execute/pr85582-3.c 2018-05-02 14:55:56.953353103 +0200 @@ -0,0 +1,55 @@ +/* PR target/85582 */ + +#ifdef __SIZEOF_INT128__ +typedef __int128 S; +typedef unsigned __int128 U; +#else +typedef long long S; +typedef unsigned long long U; +#endif + +__attribute__((noipa)) U +f1 (U x, int y) +{ + return x << (y & -2); +} + +__attribute__((noipa)) S +f2 (S x, int y) +{ + return x >> (y & -2); +} + +__attribute__((noipa)) U +f3 (U x, int y) +{ + return x >> (y & -2); +} + +int +main () +{ + U a = (U) 1 << (sizeof (U) * __CHAR_BIT__ - 7); + if (f1 (a, 5) != ((U) 1 << (sizeof (S) * __CHAR_BIT__ - 3))) + __builtin_abort (); + S b = (U) 0x101 << (sizeof (S) * __CHAR_BIT__ / 2 - 7); + if (f1 (b, sizeof (S) * __CHAR_BIT__ / 2) != (U) 0x101 << (sizeof (S) * __CHAR_BIT__ - 7)) + __builtin_abort (); + if (f1 (b, sizeof (S) * __CHAR_BIT__ / 2 + 2) != (U) 0x101 << (sizeof (S) * __CHAR_BIT__ - 5)) + __builtin_abort (); + S c = (U) 1 << (sizeof (S) * __CHAR_BIT__ - 1); + if ((U) f2 (c, 5) != ((U) 0x1f << (sizeof (S) * __CHAR_BIT__ - 5))) + __builtin_abort (); + if ((U) f2 (c, sizeof (S) * __CHAR_BIT__ / 2) != ((U) -1 << (sizeof (S) * __CHAR_BIT__ / 2 - 1))) + __builtin_abort (); + if ((U) f2 (c, sizeof (S) * __CHAR_BIT__ / 2 + 2) != ((U) -1 << (sizeof (S) * __CHAR_BIT__ / 2 - 3))) + __builtin_abort (); + U d = (U) 1 << (sizeof (S) * __CHAR_BIT__ - 1); + if (f3 (c, 5) != ((U) 0x1 << (sizeof (S) * __CHAR_BIT__ - 5))) + __builtin_abort (); + if (f3 (c, sizeof (S) * __CHAR_BIT__ / 2) != ((U) 1 << (sizeof (S) * __CHAR_BIT__ / 2 - 1))) + __builtin_abort (); + if (f3 (c, sizeof (S) * __CHAR_BIT__ / 2 + 2) != ((U) 1 << (sizeof (S) * __CHAR_BIT__ / 2 - 3))) + __builtin_abort (); + return 0; +} Jakub
--- gcc/config/i386/i386.md.jj 2018-05-01 12:18:15.566832552 +0200 +++ gcc/config/i386/i386.md 2018-05-01 13:09:36.635164943 +0200 @@ -10366,7 +10366,7 @@ (define_insn_and_split "*ashl<dwi>3_doub (match_operand:SI 2 "register_operand" "c") (match_operand:SI 3 "const_int_operand")) 0))) (clobber (reg:CC FLAGS_REG))] - "INTVAL (operands[3]) <= (<MODE_SIZE> * BITS_PER_UNIT)-1 + "INTVAL (operands[3]) <= (<MODE_SIZE> * BITS_PER_UNIT) - 1 && can_create_pseudo_p ()" "#" "&& 1" @@ -10385,8 +10385,12 @@ (define_insn_and_split "*ashl<dwi>3_doub operands[8] = GEN_INT (<MODE_SIZE> * BITS_PER_UNIT); - if (INTVAL (operands[3]) < (<MODE_SIZE> * BITS_PER_UNIT)-1) - emit_insn (gen_andsi3 (operands[2], operands[2], operands[3])); + if (INTVAL (operands[3]) < (<MODE_SIZE> * BITS_PER_UNIT) - 1) + { + rtx tem = gen_reg_rtx (SImode); + emit_insn (gen_andsi3 (tem, operands[2], operands[3])); + operands[2] = tem; + } operands[2] = gen_lowpart (QImode, operands[2]); @@ -10402,7 +10406,7 @@ (define_insn_and_split "*ashl<dwi>3_doub (match_operand:QI 2 "register_operand" "c") (match_operand:QI 3 "const_int_operand")))) (clobber (reg:CC FLAGS_REG))] - "INTVAL (operands[3]) <= (<MODE_SIZE> * BITS_PER_UNIT)-1 + "INTVAL (operands[3]) <= (<MODE_SIZE> * BITS_PER_UNIT) - 1 && can_create_pseudo_p ()" "#" "&& 1" @@ -10421,8 +10425,12 @@ (define_insn_and_split "*ashl<dwi>3_doub operands[8] = GEN_INT (<MODE_SIZE> * BITS_PER_UNIT); - if (INTVAL (operands[3]) < (<MODE_SIZE> * BITS_PER_UNIT)-1) - emit_insn (gen_andqi3 (operands[2], operands[2], operands[3])); + if (INTVAL (operands[3]) < (<MODE_SIZE> * BITS_PER_UNIT) - 1) + { + rtx tem = gen_reg_rtx (QImode); + emit_insn (gen_andqi3 (tem, operands[2], operands[3])); + operands[2] = tem; + } if (!rtx_equal_p (operands[6], operands[7])) emit_move_insn (operands[6], operands[7]); @@ -11118,7 +11126,7 @@ (define_insn_and_split "*<shift_insn><dw (match_operand:SI 2 "register_operand" "c") (match_operand:SI 3 "const_int_operand")) 0))) (clobber (reg:CC FLAGS_REG))] - "INTVAL (operands[3]) <= (<MODE_SIZE> * BITS_PER_UNIT)-1 + "INTVAL (operands[3]) <= (<MODE_SIZE> * BITS_PER_UNIT) - 1 && can_create_pseudo_p ()" "#" "&& 1" @@ -11138,7 +11146,11 @@ (define_insn_and_split "*<shift_insn><dw operands[8] = GEN_INT (<MODE_SIZE> * BITS_PER_UNIT); if (INTVAL (operands[3]) < (<MODE_SIZE> * BITS_PER_UNIT)-1) - emit_insn (gen_andsi3 (operands[2], operands[2], operands[3])); + { + rtx tem = gen_reg_rtx (SImode); + emit_insn (gen_andsi3 (tem, operands[2], operands[3])); + operands[2] = tem; + } operands[2] = gen_lowpart (QImode, operands[2]); @@ -11154,7 +11166,7 @@ (define_insn_and_split "*<shift_insn><dw (match_operand:QI 2 "register_operand" "c") (match_operand:QI 3 "const_int_operand")))) (clobber (reg:CC FLAGS_REG))] - "INTVAL (operands[3]) <= (<MODE_SIZE> * BITS_PER_UNIT)-1 + "INTVAL (operands[3]) <= (<MODE_SIZE> * BITS_PER_UNIT) - 1 && can_create_pseudo_p ()" "#" "&& 1" @@ -11173,8 +11185,12 @@ (define_insn_and_split "*<shift_insn><dw operands[8] = GEN_INT (<MODE_SIZE> * BITS_PER_UNIT); - if (INTVAL (operands[3]) < (<MODE_SIZE> * BITS_PER_UNIT)-1) - emit_insn (gen_andqi3 (operands[2], operands[2], operands[3])); + if (INTVAL (operands[3]) < (<MODE_SIZE> * BITS_PER_UNIT) - 1) + { + rtx tem = gen_reg_rtx (QImode); + emit_insn (gen_andqi3 (tem, operands[2], operands[3])); + operands[2] = tem; + } if (!rtx_equal_p (operands[4], operands[5])) emit_move_insn (operands[4], operands[5]); --- gcc/testsuite/gcc.c-torture/execute/pr85582-1.c.jj 2018-05-01 13:28:07.547933727 +0200 +++ gcc/testsuite/gcc.c-torture/execute/pr85582-1.c 2018-05-01 13:26:32.270868649 +0200 @@ -0,0 +1,21 @@ +/* PR target/85582 */ + +int a, b, d = 2, e; +long long c = 1; + +int +main () +{ + int g = 6; +L1: + e = d; + if (a) + goto L1; + g--; + int i = c >> ~(~e | ~g); +L2: + c = (b % c) * i; + if (!e) + goto L2; + return 0; +} --- gcc/testsuite/gcc.c-torture/execute/pr85582-2.c.jj 2018-05-01 13:28:10.332935636 +0200 +++ gcc/testsuite/gcc.c-torture/execute/pr85582-2.c 2018-05-01 13:28:00.615929000 +0200 @@ -0,0 +1,51 @@ +/* PR target/85582 */ + +#ifdef __SIZEOF_INT128__ +typedef __int128 S; +typedef unsigned __int128 U; +#else +typedef long long S; +typedef unsigned long long U; +#endif + +__attribute__((noipa)) S +f1 (S x, int y) +{ + x = x << (y & 5); + x += y; + return x; +} + +__attribute__((noipa)) S +f2 (S x, int y) +{ + x = x >> (y & 5); + x += y; + return x; +} + +__attribute__((noipa)) U +f3 (U x, int y) +{ + x = x >> (y & 5); + x += y; + return x; +} + +int +main () +{ + S a = (S) 1 << (sizeof (S) * __CHAR_BIT__ - 7); + S b = f1 (a, 12); + if (b != ((S) 1 << (sizeof (S) * __CHAR_BIT__ - 3)) + 12) + __builtin_abort (); + S c = (U) 1 << (sizeof (S) * __CHAR_BIT__ - 1); + S d = f2 (c, 12); + if ((U) d != ((U) 0x1f << (sizeof (S) * __CHAR_BIT__ - 5)) + 12) + __builtin_abort (); + U e = (U) 1 << (sizeof (U) * __CHAR_BIT__ - 1); + U f = f3 (c, 12); + if (f != ((U) 1 << (sizeof (U) * __CHAR_BIT__ - 5)) + 12) + __builtin_abort (); + return 0; +}