| Message ID | 20210728084510.GS2380545@tucnak |
|---|---|
| State | New |
| Headers | show |
| Series | match.pd: Fix up recent __builtin_bswap16 simplifications [PR101642] | expand |
On Wed, 28 Jul 2021, Jakub Jelinek wrote: > Hi! > > The following testcase ICEs. The problem is that for __builtin_bswap16 > (and only that, others are fine) the argument of the builtin is promoted > to int while the patterns assume it is not and is the same as that of > the return type. > For the bswap simplifications before these new ones it just means we > fail to optimize stuff like __builtin_bswap16 (__builtin_bswap16 (x)) > because there are casts in between, but the last one, equality comparison > of __builtin_bswap16 with integer constant results in ICE, because > we create comparison with incompatible types of the operands, and the > other might be fine because usually we bit and the operand before promoting, > but I think it is too dangerous to rely on it, one day we find out that > because it is operand to such a built in, we can throw away any changes > that affect the upper bits and all of sudden it would misbehave. > > So, this patch introduces converts that shouldn't do anything for > bswap{32,64,128} and should fix these issues for bswap16. > > Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk? OK. Richard. > 2021-07-28 Jakub Jelinek <jakub@redhat.com> > > PR middle-end/101642 > * match.pd (bswap16 (x) == bswap16 (y)): Cast both operands > to type of bswap16 for comparison. > (bswap16 (x) == cst): Cast bswap16 operand to type of cst. > > * gcc.c-torture/compile/pr101642.c: New test. > > --- gcc/match.pd.jj 2021-07-27 09:47:44.000000000 +0200 > +++ gcc/match.pd 2021-07-27 16:16:19.867757153 +0200 > @@ -3641,11 +3641,13 @@ (define_operator_list COND_TERNARY > (bitop @0 (bswap @1)))) > (for cmp (eq ne) > (simplify > - (cmp (bswap @0) (bswap @1)) > - (cmp @0 @1)) > + (cmp (bswap@2 @0) (bswap @1)) > + (with { tree ctype = TREE_TYPE (@2); } > + (cmp (convert:ctype @0) (convert:ctype @1)))) > (simplify > (cmp (bswap @0) INTEGER_CST@1) > - (cmp @0 (bswap @1)))) > + (with { tree ctype = TREE_TYPE (@1); } > + (cmp (convert:ctype @0) (bswap @1))))) > /* (bswap(x) >> C1) & C2 can sometimes be simplified to (x >> C3) & C2. */ > (simplify > (bit_and (convert1? (rshift@0 (convert2? (bswap@4 @1)) INTEGER_CST@2)) > --- gcc/testsuite/gcc.c-torture/compile/pr101642.c.jj 2021-07-27 16:34:44.910039793 +0200 > +++ gcc/testsuite/gcc.c-torture/compile/pr101642.c 2021-07-27 16:32:24.661907592 +0200 > @@ -0,0 +1,17 @@ > +/* PR middle-end/101642 */ > + > +int x; > + > +unsigned short > +foo (void) > +{ > + return __builtin_bswap16 (x) ? : 0; > +} > + > +int > +bar (int x, int y) > +{ > + unsigned short a = __builtin_bswap16 ((unsigned short) x); > + unsigned short b = __builtin_bswap16 ((unsigned short) y); > + return a == b; > +} > > Jakub > >
--- gcc/match.pd.jj 2021-07-27 09:47:44.000000000 +0200 +++ gcc/match.pd 2021-07-27 16:16:19.867757153 +0200 @@ -3641,11 +3641,13 @@ (define_operator_list COND_TERNARY (bitop @0 (bswap @1)))) (for cmp (eq ne) (simplify - (cmp (bswap @0) (bswap @1)) - (cmp @0 @1)) + (cmp (bswap@2 @0) (bswap @1)) + (with { tree ctype = TREE_TYPE (@2); } + (cmp (convert:ctype @0) (convert:ctype @1)))) (simplify (cmp (bswap @0) INTEGER_CST@1) - (cmp @0 (bswap @1)))) + (with { tree ctype = TREE_TYPE (@1); } + (cmp (convert:ctype @0) (bswap @1))))) /* (bswap(x) >> C1) & C2 can sometimes be simplified to (x >> C3) & C2. */ (simplify (bit_and (convert1? (rshift@0 (convert2? (bswap@4 @1)) INTEGER_CST@2)) --- gcc/testsuite/gcc.c-torture/compile/pr101642.c.jj 2021-07-27 16:34:44.910039793 +0200 +++ gcc/testsuite/gcc.c-torture/compile/pr101642.c 2021-07-27 16:32:24.661907592 +0200 @@ -0,0 +1,17 @@ +/* PR middle-end/101642 */ + +int x; + +unsigned short +foo (void) +{ + return __builtin_bswap16 (x) ? : 0; +} + +int +bar (int x, int y) +{ + unsigned short a = __builtin_bswap16 ((unsigned short) x); + unsigned short b = __builtin_bswap16 ((unsigned short) y); + return a == b; +}
Hi! The following testcase ICEs. The problem is that for __builtin_bswap16 (and only that, others are fine) the argument of the builtin is promoted to int while the patterns assume it is not and is the same as that of the return type. For the bswap simplifications before these new ones it just means we fail to optimize stuff like __builtin_bswap16 (__builtin_bswap16 (x)) because there are casts in between, but the last one, equality comparison of __builtin_bswap16 with integer constant results in ICE, because we create comparison with incompatible types of the operands, and the other might be fine because usually we bit and the operand before promoting, but I think it is too dangerous to rely on it, one day we find out that because it is operand to such a built in, we can throw away any changes that affect the upper bits and all of sudden it would misbehave. So, this patch introduces converts that shouldn't do anything for bswap{32,64,128} and should fix these issues for bswap16. Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk? 2021-07-28 Jakub Jelinek <jakub@redhat.com> PR middle-end/101642 * match.pd (bswap16 (x) == bswap16 (y)): Cast both operands to type of bswap16 for comparison. (bswap16 (x) == cst): Cast bswap16 operand to type of cst. * gcc.c-torture/compile/pr101642.c: New test. Jakub