public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH] Fix PR84873
@ 2018-03-15 12:56 Richard Biener
  2018-03-15 15:43 ` Jakub Jelinek
  0 siblings, 1 reply; 6+ messages in thread
From: Richard Biener @ 2018-03-15 12:56 UTC (permalink / raw)
  To: gcc-patches; +Cc: Jakub Jelinek


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.

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);
+}

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] Fix PR84873
  2018-03-15 12:56 [PATCH] Fix PR84873 Richard Biener
@ 2018-03-15 15:43 ` Jakub Jelinek
  2018-03-15 16:51   ` Bin.Cheng
  0 siblings, 1 reply; 6+ messages in thread
From: Jakub Jelinek @ 2018-03-15 15:43 UTC (permalink / raw)
  To: Richard Biener; +Cc: gcc-patches

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] Fix PR84873
  2018-03-15 15:43 ` Jakub Jelinek
@ 2018-03-15 16:51   ` Bin.Cheng
  2018-03-16  7:56     ` Richard Biener
  0 siblings, 1 reply; 6+ messages in thread
From: Bin.Cheng @ 2018-03-15 16:51 UTC (permalink / raw)
  To: Jakub Jelinek; +Cc: Richard Biener, gcc-patches List

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] Fix PR84873
  2018-03-15 16:51   ` Bin.Cheng
@ 2018-03-16  7:56     ` Richard Biener
  2018-03-16 11:55       ` Richard Biener
  0 siblings, 1 reply; 6+ messages in thread
From: Richard Biener @ 2018-03-16  7:56 UTC (permalink / raw)
  To: Bin.Cheng; +Cc: Jakub Jelinek, gcc-patches List

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
> 
> 

-- 
Richard Biener <rguenther@suse.de>
SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nuernberg)

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] Fix PR84873
  2018-03-16  7:56     ` Richard Biener
@ 2018-03-16 11:55       ` Richard Biener
  2018-03-16 20:57         ` Jakub Jelinek
  0 siblings, 1 reply; 6+ messages in thread
From: Richard Biener @ 2018-03-16 11:55 UTC (permalink / raw)
  To: Bin.Cheng; +Cc: Jakub Jelinek, gcc-patches List

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
> > 
> > 
> 
> 

-- 
Richard Biener <rguenther@suse.de>
SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nuernberg)

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] Fix PR84873
  2018-03-16 11:55       ` Richard Biener
@ 2018-03-16 20:57         ` Jakub Jelinek
  0 siblings, 0 replies; 6+ messages in thread
From: Jakub Jelinek @ 2018-03-16 20:57 UTC (permalink / raw)
  To: Richard Biener; +Cc: Bin.Cheng, gcc-patches List

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2018-03-16 20:35 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-03-15 12:56 [PATCH] Fix PR84873 Richard Biener
2018-03-15 15:43 ` Jakub Jelinek
2018-03-15 16:51   ` Bin.Cheng
2018-03-16  7:56     ` Richard Biener
2018-03-16 11:55       ` Richard Biener
2018-03-16 20:57         ` Jakub Jelinek

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).