public inbox for java@gcc.gnu.org
 help / color / mirror / Atom feed
* Re: [PATCH] Remove bogus PLUS_EXPR -> POINTER_PLUS_EXPR folding
       [not found] <alpine.LNX.2.00.0901161453350.24314@zhemvz.fhfr.qr>
@ 2009-01-16 16:45 ` Richard Guenther
  2009-01-16 18:47   ` Andrew Haley
  0 siblings, 1 reply; 3+ messages in thread
From: Richard Guenther @ 2009-01-16 16:45 UTC (permalink / raw)
  To: gcc-patches; +Cc: java, tromey, aph

On Fri, 16 Jan 2009, Richard Guenther wrote:

> 
> This removes a bogus folding that causes miscompiles of libstdc++
> testcases with the ext_pointer class (PR38835) and non-field-sensitive 
> PTA (field-sensitive PTA is too dumb).  I probably anticipated the
> problem in PR36227 which should be fixed with that patch as well.
> 
> Fold changes
> 
>    return <retval> = (const struct _Fwd_list_node_base *) ((long unsigned int) this + (long unsigned int) ((const struct _Relative_pointer_impl *) this)->_M_diff);
> 
> to
> 
>    (const struct _Fwd_list_node_base *) this p+ (long unsigned int) ((const struct _Relative_pointer_impl *) this)->_M_diff
> 
> which makes PTA correctly conclude that 'this' and the result of the
> POINTER_PLUS_EXPR point to the same object.  Which they do not - the source
> used addition in (long unsigned int) to avoid this issue and the resulting
> pointer points somewhere else.
> 
> Fixed by removing the folding.  This also fixes -Wsystem-header warnings
> and miscompiles for the libstdc++ testsuite.
> 
> Bootstrap and regtest on x86_64-unknown-linux-gnu ongoing.

Needs one extra hunk in the Java frontend.  Bootstrapped and tested
on x86_64-unknown-linux-gnu, is that ok for trunk?

Also needs one C++ testcase XFAILed, the FE passes
(long int) &16B->y - 16 into fold and expects us to perform offsetof
magic.  I will try to address both XFAILs later.

Thanks,
Richard.

2009-01-16  Richard Guenther  <rguenther@suse.de>

	PR tree-optimization/38835
	PR middle-end/36227

	java/
	* builtins.c (build_addr_sum): Use POINTER_PLUS_EXPR.

Index: gcc/java/builtins.c
===================================================================
*** gcc/java/builtins.c	(revision 143429)
--- gcc/java/builtins.c	(working copy)
*************** static tree
*** 265,273 ****
  build_addr_sum (tree type, tree addr, tree offset)
  {
    tree ptr_type = build_pointer_type (type);
!   return  fold_build2 (PLUS_EXPR, 
! 		       ptr_type, 
! 		       fold_convert (ptr_type, addr), offset);
  }
  
  /* Make sure that this-arg is non-NULL.  This is a security check.  */
--- 265,274 ----
  build_addr_sum (tree type, tree addr, tree offset)
  {
    tree ptr_type = build_pointer_type (type);
!   return fold_build2 (POINTER_PLUS_EXPR,
! 		      ptr_type,
! 		      fold_convert (ptr_type, addr),
! 		      fold_convert (sizetype, offset));
  }
  
  /* Make sure that this-arg is non-NULL.  This is a security check.  */

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

* Re: [PATCH] Remove bogus PLUS_EXPR -> POINTER_PLUS_EXPR folding
  2009-01-16 16:45 ` [PATCH] Remove bogus PLUS_EXPR -> POINTER_PLUS_EXPR folding Richard Guenther
@ 2009-01-16 18:47   ` Andrew Haley
  2009-01-16 19:25     ` Richard Guenther
  0 siblings, 1 reply; 3+ messages in thread
From: Andrew Haley @ 2009-01-16 18:47 UTC (permalink / raw)
  To: Richard Guenther; +Cc: gcc-patches, java, tromey

Richard Guenther wrote:

> 2009-01-16  Richard Guenther  <rguenther@suse.de>
> 
> 	PR tree-optimization/38835
> 	PR middle-end/36227
> 
> 	java/
> 	* builtins.c (build_addr_sum): Use POINTER_PLUS_EXPR.
> 
> Index: gcc/java/builtins.c
> ===================================================================
> *** gcc/java/builtins.c	(revision 143429)
> --- gcc/java/builtins.c	(working copy)
> *************** static tree
> *** 265,273 ****
>   build_addr_sum (tree type, tree addr, tree offset)
>   {
>     tree ptr_type = build_pointer_type (type);
> !   return  fold_build2 (PLUS_EXPR, 
> ! 		       ptr_type, 
> ! 		       fold_convert (ptr_type, addr), offset);
>   }
>   
>   /* Make sure that this-arg is non-NULL.  This is a security check.  */
> --- 265,274 ----
>   build_addr_sum (tree type, tree addr, tree offset)
>   {
>     tree ptr_type = build_pointer_type (type);
> !   return fold_build2 (POINTER_PLUS_EXPR,
> ! 		      ptr_type,
> ! 		      fold_convert (ptr_type, addr),
> ! 		      fold_convert (sizetype, offset));
>   }
>   
>   /* Make sure that this-arg is non-NULL.  This is a security check.  */

OK.

Andrew.

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

* Re: [PATCH] Remove bogus PLUS_EXPR -> POINTER_PLUS_EXPR folding
  2009-01-16 18:47   ` Andrew Haley
@ 2009-01-16 19:25     ` Richard Guenther
  0 siblings, 0 replies; 3+ messages in thread
From: Richard Guenther @ 2009-01-16 19:25 UTC (permalink / raw)
  To: Andrew Haley; +Cc: gcc-patches, java, tromey

On Fri, 16 Jan 2009, Andrew Haley wrote:

> OK.
> 
> Andrew.

Thanks.  This is what I applied.

Richard.

2009-01-16  Richard Guenther  <rguenther@suse.de>

	PR tree-optimization/38835
	PR middle-end/36227
	* fold-const.c (fold_binary): Remove PTR + INT -> (INT)(PTR p+ INT)
	and INT + PTR -> (INT)(PTR p+ INT) folding.
	* tree-ssa-address.c (create_mem_ref): Properly use POINTER_PLUS_EXPR.

	java/
	* builtins.c (build_addr_sum): Use POINTER_PLUS_EXPR.

	* gcc.c-torture/execute/pr36227.c: New testcase.
	* gcc.dg/tree-ssa/foldaddr-1.c: XFAIL.
	* g++.dg/init/const7.C: Likewise.

Index: gcc/fold-const.c
===================================================================
*** gcc/fold-const.c.orig	2009-01-16 18:07:26.000000000 +0100
--- gcc/fold-const.c	2009-01-16 20:11:49.000000000 +0100
*************** fold_binary (enum tree_code code, tree t
*** 9864,9883 ****
        return NULL_TREE;
  
      case PLUS_EXPR:
-       /* PTR + INT -> (INT)(PTR p+ INT) */
-       if (POINTER_TYPE_P (TREE_TYPE (arg0))
- 	  && INTEGRAL_TYPE_P (TREE_TYPE (arg1)))
- 	return fold_convert (type, fold_build2 (POINTER_PLUS_EXPR,
- 						TREE_TYPE (arg0),
- 						arg0,
- 						fold_convert (sizetype, arg1)));
-       /* INT + PTR -> (INT)(PTR p+ INT) */
-       if (POINTER_TYPE_P (TREE_TYPE (arg1))
- 	  && INTEGRAL_TYPE_P (TREE_TYPE (arg0)))
- 	return fold_convert (type, fold_build2 (POINTER_PLUS_EXPR,
- 						TREE_TYPE (arg1),
- 						arg1,
- 						fold_convert (sizetype, arg0)));
        /* A + (-B) -> A - B */
        if (TREE_CODE (arg1) == NEGATE_EXPR)
  	return fold_build2 (MINUS_EXPR, type,
--- 9864,9869 ----
Index: gcc/testsuite/gcc.dg/tree-ssa/foldaddr-1.c
===================================================================
*** gcc/testsuite/gcc.dg/tree-ssa/foldaddr-1.c.orig	2009-01-16 18:06:51.000000000 +0100
--- gcc/testsuite/gcc.dg/tree-ssa/foldaddr-1.c	2009-01-16 20:11:49.000000000 +0100
*************** int foo(char *b)
*** 11,16 ****
  /* Folding should have determined that the two addresses were
     not identical and thus collapsed the function into a trivial
     "return 0".  */
! /* { dg-final { scan-tree-dump-times "return 0" 1 "original"} } */
  /* { dg-final { cleanup-tree-dump "original" } } */
  
--- 11,16 ----
  /* Folding should have determined that the two addresses were
     not identical and thus collapsed the function into a trivial
     "return 0".  */
! /* { dg-final { scan-tree-dump-times "return 0" 1 "original" { xfail *-*-* } } */
  /* { dg-final { cleanup-tree-dump "original" } } */
  
Index: gcc/tree-ssa-address.c
===================================================================
*** gcc/tree-ssa-address.c.orig	2009-01-16 18:06:51.000000000 +0100
--- gcc/tree-ssa-address.c	2009-01-16 20:11:49.000000000 +0100
*************** create_mem_ref (gimple_stmt_iterator *gs
*** 619,627 ****
  	    {
  	      atype = TREE_TYPE (tmp);
  	      parts.base = force_gimple_operand_gsi (gsi,
! 			fold_build2 (PLUS_EXPR, atype,
! 				     fold_convert (atype, parts.base),
! 				     tmp),
  			true, NULL_TREE, true, GSI_SAME_STMT);
  	    }
  	  else
--- 619,627 ----
  	    {
  	      atype = TREE_TYPE (tmp);
  	      parts.base = force_gimple_operand_gsi (gsi,
! 			fold_build2 (POINTER_PLUS_EXPR, atype,
! 				     tmp,
! 				     fold_convert (sizetype, parts.base)),
  			true, NULL_TREE, true, GSI_SAME_STMT);
  	    }
  	  else
Index: gcc/java/builtins.c
===================================================================
*** gcc/java/builtins.c.orig	2009-01-16 18:06:51.000000000 +0100
--- gcc/java/builtins.c	2009-01-16 20:11:49.000000000 +0100
*************** static tree
*** 265,273 ****
  build_addr_sum (tree type, tree addr, tree offset)
  {
    tree ptr_type = build_pointer_type (type);
!   return  fold_build2 (PLUS_EXPR, 
! 		       ptr_type, 
! 		       fold_convert (ptr_type, addr), offset);
  }
  
  /* Make sure that this-arg is non-NULL.  This is a security check.  */
--- 265,274 ----
  build_addr_sum (tree type, tree addr, tree offset)
  {
    tree ptr_type = build_pointer_type (type);
!   return fold_build2 (POINTER_PLUS_EXPR,
! 		      ptr_type,
! 		      fold_convert (ptr_type, addr),
! 		      fold_convert (sizetype, offset));
  }
  
  /* Make sure that this-arg is non-NULL.  This is a security check.  */
Index: gcc/testsuite/g++.dg/init/const7.C
===================================================================
*** gcc/testsuite/g++.dg/init/const7.C.orig	2009-01-16 18:06:51.000000000 +0100
--- gcc/testsuite/g++.dg/init/const7.C	2009-01-16 20:11:49.000000000 +0100
*************** short offsets[1] = {
*** 9,13 ****
  // This ensures that we get a dump whether or not the bug is present.
  void fn() { }
  
! // { dg-final { scan-tree-dump-not "initialization"  "gimple" } }
  // { dg-final { cleanup-tree-dump "gimple" } }
--- 9,13 ----
  // This ensures that we get a dump whether or not the bug is present.
  void fn() { }
  
! // { dg-final { scan-tree-dump-not "initialization"  "gimple" { xfail *-*-* } } }
  // { dg-final { cleanup-tree-dump "gimple" } }
Index: gcc/testsuite/gcc.c-torture/execute/pr36227.c
===================================================================
*** /dev/null	1970-01-01 00:00:00.000000000 +0000
--- gcc/testsuite/gcc.c-torture/execute/pr36227.c	2009-01-16 20:12:24.000000000 +0100
***************
*** 0 ****
--- 1,15 ----
+ #include <stdint.h>
+ extern void abort (void);
+ int main()
+ {
+   int i = 1;
+   int *p = &i;
+   uintptr_t iptr;
+ 
+   iptr = (uintptr_t)p - (uintptr_t)&iptr;
+   p = (int *)((uintptr_t)&iptr + iptr);
+   if (*p != 1)
+     abort ();
+   return 0;
+ }
+ 

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

end of thread, other threads:[~2009-01-16 19:25 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <alpine.LNX.2.00.0901161453350.24314@zhemvz.fhfr.qr>
2009-01-16 16:45 ` [PATCH] Remove bogus PLUS_EXPR -> POINTER_PLUS_EXPR folding Richard Guenther
2009-01-16 18:47   ` Andrew Haley
2009-01-16 19:25     ` Richard Guenther

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