| Message ID | alpine.LSU.2.20.1803151353300.18265@zhemvz.fhfr.qr |
|---|---|
| State | New |
| Headers | show |
| Series | Fix PR84873 | expand |
On Thu, Mar 15, 2018 at 01:56:16PM +0100, Richard Biener wrote: > The following fixes the C familiy gimplification langhook to not > introduce tree sharing which isn't valid during gimplification. > For the specific case the tree sharing is introduced by > fold_binary_op_with_cond and is reached via convert () eventually > folding something. I've kept folding constants here but for the > rest defer folding to GIMPLE (the gimplifier already folds most > generated stmts). > > Bootstrapped and tested on x86_64-unknown-linux-gnu, ok for trunk and > branches? > > Thanks, > Richard. > > 2018-03-15 Richard Biener <rguenther@suse.de> > > PR c/84873 > * c-gimplify.c (c_gimplify_expr): Do not fold expressions. > > * c-c++-common/pr84873.c: New testcase. Ok, thanks. Jakub
On Thu, Mar 15, 2018 at 1:07 PM, Jakub Jelinek <jakub@redhat.com> wrote: > On Thu, Mar 15, 2018 at 01:56:16PM +0100, Richard Biener wrote: >> The following fixes the C familiy gimplification langhook to not >> introduce tree sharing which isn't valid during gimplification. >> For the specific case the tree sharing is introduced by >> fold_binary_op_with_cond and is reached via convert () eventually >> folding something. I've kept folding constants here but for the >> rest defer folding to GIMPLE (the gimplifier already folds most >> generated stmts). >> >> Bootstrapped and tested on x86_64-unknown-linux-gnu, ok for trunk and >> branches? Hi, FYI, this causes below failure. Failures: gcc.target/aarch64/var_shift_mask_1.c Bisected to: commit 676d61f64d05af5833ddd471cc99229cedbd59b4 Author: rguenth <rguenth@138bc75d-0d04-0410-961f-82ee72b054a4> Date: Thu Mar 15 13:10:24 2018 +0000 2018-03-15 Richard Biener <rguenther@suse.de> PR c/84873 * c-gimplify.c (c_gimplify_expr): Do not fold expressions. * c-c++-common/pr84873.c: New testcase. git-svn-id: svn+ssh://gcc.gnu.org/svn/gcc/trunk@258556 138bc75d-0d04-0410-961f-82ee72b054a4 I will get more information about the failure. Thanks, bin >> >> Thanks, >> Richard. >> >> 2018-03-15 Richard Biener <rguenther@suse.de> >> >> PR c/84873 >> * c-gimplify.c (c_gimplify_expr): Do not fold expressions. >> >> * c-c++-common/pr84873.c: New testcase. > > Ok, thanks. > > Jakub
On Thu, 15 Mar 2018, Bin.Cheng wrote: > On Thu, Mar 15, 2018 at 1:07 PM, Jakub Jelinek <jakub@redhat.com> wrote: > > On Thu, Mar 15, 2018 at 01:56:16PM +0100, Richard Biener wrote: > >> The following fixes the C familiy gimplification langhook to not > >> introduce tree sharing which isn't valid during gimplification. > >> For the specific case the tree sharing is introduced by > >> fold_binary_op_with_cond and is reached via convert () eventually > >> folding something. I've kept folding constants here but for the > >> rest defer folding to GIMPLE (the gimplifier already folds most > >> generated stmts). > >> > >> Bootstrapped and tested on x86_64-unknown-linux-gnu, ok for trunk and > >> branches? > Hi, > FYI, this causes below failure. > > Failures: > gcc.target/aarch64/var_shift_mask_1.c Ok, these look like narrowing only done via convert () and neither fold nor gimple folding. An alternative slightly more expensive patch is the following which I'm now testing on x86_64-unknown-linux-gnu, the above testcase is fixed with it (verified with a cross). Richard. 2018-03-16 Richard Biener <rguenther@suse.de> PR c/84873 * c-gimplify.c (c_gimplify_expr): Revert previous change. Instead unshare the possibly folded expression. Index: gcc/c-family/c-gimplify.c =================================================================== --- gcc/c-family/c-gimplify.c (revision 258584) +++ gcc/c-family/c-gimplify.c (working copy) @@ -245,15 +245,9 @@ c_gimplify_expr (tree *expr_p, gimple_se unsigned_type_node) && !types_compatible_p (TYPE_MAIN_VARIANT (TREE_TYPE (*op1_p)), integer_type_node)) - { - /* ??? Do not use convert () here or fold arbitrary trees - since folding can introduce tree sharing which is not - allowed during gimplification. */ - if (TREE_CODE (*op1_p) == INTEGER_CST) - *op1_p = fold_convert (unsigned_type_node, *op1_p); - else - *op1_p = build1 (NOP_EXPR, unsigned_type_node, *op1_p); - } + /* Make sure to unshare the result, tree sharing is invalid + during gimplification. */ + *op1_p = unshare_expr (convert (unsigned_type_node, *op1_p)); break; } > Bisected to: > > > commit 676d61f64d05af5833ddd471cc99229cedbd59b4 > Author: rguenth <rguenth@138bc75d-0d04-0410-961f-82ee72b054a4> > Date: Thu Mar 15 13:10:24 2018 +0000 > > 2018-03-15 Richard Biener <rguenther@suse.de> > > PR c/84873 > * c-gimplify.c (c_gimplify_expr): Do not fold expressions. > > * c-c++-common/pr84873.c: New testcase. > > > git-svn-id: svn+ssh://gcc.gnu.org/svn/gcc/trunk@258556 > 138bc75d-0d04-0410-961f-82ee72b054a4 > > I will get more information about the failure. > > Thanks, > bin > >> > >> Thanks, > >> Richard. > >> > >> 2018-03-15 Richard Biener <rguenther@suse.de> > >> > >> PR c/84873 > >> * c-gimplify.c (c_gimplify_expr): Do not fold expressions. > >> > >> * c-c++-common/pr84873.c: New testcase. > > > > Ok, thanks. > > > > Jakub > >
On Fri, 16 Mar 2018, Richard Biener wrote: > On Thu, 15 Mar 2018, Bin.Cheng wrote: > > > On Thu, Mar 15, 2018 at 1:07 PM, Jakub Jelinek <jakub@redhat.com> wrote: > > > On Thu, Mar 15, 2018 at 01:56:16PM +0100, Richard Biener wrote: > > >> The following fixes the C familiy gimplification langhook to not > > >> introduce tree sharing which isn't valid during gimplification. > > >> For the specific case the tree sharing is introduced by > > >> fold_binary_op_with_cond and is reached via convert () eventually > > >> folding something. I've kept folding constants here but for the > > >> rest defer folding to GIMPLE (the gimplifier already folds most > > >> generated stmts). > > >> > > >> Bootstrapped and tested on x86_64-unknown-linux-gnu, ok for trunk and > > >> branches? > > Hi, > > FYI, this causes below failure. > > > > Failures: > > gcc.target/aarch64/var_shift_mask_1.c > > Ok, these look like narrowing only done via convert () and neither > fold nor gimple folding. > > An alternative slightly more expensive patch is the following which > I'm now testing on x86_64-unknown-linux-gnu, the above testcase > is fixed with it (verified with a cross). Bootstrapped and tested on x86_64-unknown-linux-gnu, ok? Thanks, Richard. > Richard. > > 2018-03-16 Richard Biener <rguenther@suse.de> > > PR c/84873 > * c-gimplify.c (c_gimplify_expr): Revert previous change. Instead > unshare the possibly folded expression. > > Index: gcc/c-family/c-gimplify.c > =================================================================== > --- gcc/c-family/c-gimplify.c (revision 258584) > +++ gcc/c-family/c-gimplify.c (working copy) > @@ -245,15 +245,9 @@ c_gimplify_expr (tree *expr_p, gimple_se > unsigned_type_node) > && !types_compatible_p (TYPE_MAIN_VARIANT (TREE_TYPE > (*op1_p)), > integer_type_node)) > - { > - /* ??? Do not use convert () here or fold arbitrary trees > - since folding can introduce tree sharing which is not > - allowed during gimplification. */ > - if (TREE_CODE (*op1_p) == INTEGER_CST) > - *op1_p = fold_convert (unsigned_type_node, *op1_p); > - else > - *op1_p = build1 (NOP_EXPR, unsigned_type_node, *op1_p); > - } > + /* Make sure to unshare the result, tree sharing is invalid > + during gimplification. */ > + *op1_p = unshare_expr (convert (unsigned_type_node, *op1_p)); > break; > } > > > > > > Bisected to: > > > > > > commit 676d61f64d05af5833ddd471cc99229cedbd59b4 > > Author: rguenth <rguenth@138bc75d-0d04-0410-961f-82ee72b054a4> > > Date: Thu Mar 15 13:10:24 2018 +0000 > > > > 2018-03-15 Richard Biener <rguenther@suse.de> > > > > PR c/84873 > > * c-gimplify.c (c_gimplify_expr): Do not fold expressions. > > > > * c-c++-common/pr84873.c: New testcase. > > > > > > git-svn-id: svn+ssh://gcc.gnu.org/svn/gcc/trunk@258556 > > 138bc75d-0d04-0410-961f-82ee72b054a4 > > > > I will get more information about the failure. > > > > Thanks, > > bin > > >> > > >> Thanks, > > >> Richard. > > >> > > >> 2018-03-15 Richard Biener <rguenther@suse.de> > > >> > > >> PR c/84873 > > >> * c-gimplify.c (c_gimplify_expr): Do not fold expressions. > > >> > > >> * c-c++-common/pr84873.c: New testcase. > > > > > > Ok, thanks. > > > > > > Jakub > > > > > >
On Fri, Mar 16, 2018 at 12:53:29PM +0100, Richard Biener wrote: > > An alternative slightly more expensive patch is the following which > > I'm now testing on x86_64-unknown-linux-gnu, the above testcase > > is fixed with it (verified with a cross). > > Bootstrapped and tested on x86_64-unknown-linux-gnu, ok? > > > 2018-03-16 Richard Biener <rguenther@suse.de> > > > > PR c/84873 > > * c-gimplify.c (c_gimplify_expr): Revert previous change. Instead > > unshare the possibly folded expression. > > > > + /* Make sure to unshare the result, tree sharing is invalid > > + during gimplification. */ > > + *op1_p = unshare_expr (convert (unsigned_type_node, *op1_p)); Ok, thanks. Jakub
Index: gcc/c-family/c-gimplify.c =================================================================== --- gcc/c-family/c-gimplify.c (revision 258552) +++ gcc/c-family/c-gimplify.c (working copy) @@ -245,7 +245,15 @@ c_gimplify_expr (tree *expr_p, gimple_se unsigned_type_node) && !types_compatible_p (TYPE_MAIN_VARIANT (TREE_TYPE (*op1_p)), integer_type_node)) - *op1_p = convert (unsigned_type_node, *op1_p); + { + /* ??? Do not use convert () here or fold arbitrary trees + since folding can introduce tree sharing which is not + allowed during gimplification. */ + if (TREE_CODE (*op1_p) == INTEGER_CST) + *op1_p = fold_convert (unsigned_type_node, *op1_p); + else + *op1_p = build1 (NOP_EXPR, unsigned_type_node, *op1_p); + } break; } Index: gcc/testsuite/c-c++-common/pr84873.c =================================================================== --- gcc/testsuite/c-c++-common/pr84873.c (nonexistent) +++ gcc/testsuite/c-c++-common/pr84873.c (working copy) @@ -0,0 +1,8 @@ +/* { dg-do compile } */ +/* { dg-additional-options "-frounding-math" } */ + +int +i1 (int w3, int n9) +{ + return w3 >> ((long int)(1 + 0.1) + -!n9); +}