Patchwork PATCH RFA: Fix PR 45687

login
register
mail settings
Submitter Ian Taylor
Date Oct. 26, 2010, 4:24 a.m.
Message ID <mcrk4l5twd4.fsf@google.com>
Download mbox | patch
Permalink /patch/69169/
State New
Headers show

Comments

Ian Taylor - Oct. 26, 2010, 4:24 a.m.
This patch implements Jakub's suggestion for fixing PR 45687.  The patch
is mainly by inspection.  The test case fails before the patch and
succeeds afterward, but the test case only tests the first hunk of the
patch, not the third (the second is just a formatting fix).

Bootstrapped and tested on x86_64-unknown-linux-gnu.  OK for mainline?

Ian


gcc/ChangeLog:

2010-10-25  Ian Lance Taylor  <iant@google.com>

	PR middle-end/45687
	* ipa-prop.c (ipa_modify_call_arguments): Correct type of MEM_REF
	offset.

gcc/testsuite/ChangeLog:

2010-10-25  Ian Lance Taylor  <iant@google.com>

	PR middle-end/45687
	* gcc.c-torture/execute/20101025-1.c: New test.
Richard Guenther - Oct. 26, 2010, 1:30 p.m.
On Tue, Oct 26, 2010 at 12:24 AM, Ian Lance Taylor <iant@google.com> wrote:
> This patch implements Jakub's suggestion for fixing PR 45687.  The patch
> is mainly by inspection.  The test case fails before the patch and
> succeeds afterward, but the test case only tests the first hunk of the
> patch, not the third (the second is just a formatting fix).
>
> Bootstrapped and tested on x86_64-unknown-linux-gnu.  OK for mainline?

The first hunk is not necessary to fix the bug, right?  I agree the current
code doesn't make sense there.

The patch is ok.  I'm still somewhat confused by this function (and think
that we do the wrong thing wrt preserving TBAA information), but the
patch seems to be a strict improvement over the current situation.

Thanks,
Richard.

> Ian
>
>
> gcc/ChangeLog:
>
> 2010-10-25  Ian Lance Taylor  <iant@google.com>
>
>        PR middle-end/45687
>        * ipa-prop.c (ipa_modify_call_arguments): Correct type of MEM_REF
>        offset.
>
> gcc/testsuite/ChangeLog:
>
> 2010-10-25  Ian Lance Taylor  <iant@google.com>
>
>        PR middle-end/45687
>        * gcc.c-torture/execute/20101025-1.c: New test.
>
>
>
Ian Taylor - Oct. 26, 2010, 1:40 p.m.
Richard Guenther <richard.guenther@gmail.com> writes:

> On Tue, Oct 26, 2010 at 12:24 AM, Ian Lance Taylor <iant@google.com> wrote:
>> This patch implements Jakub's suggestion for fixing PR 45687.  The patch
>> is mainly by inspection.  The test case fails before the patch and
>> succeeds afterward, but the test case only tests the first hunk of the
>> patch, not the third (the second is just a formatting fix).
>>
>> Bootstrapped and tested on x86_64-unknown-linux-gnu.  OK for mainline?
>
> The first hunk is not necessary to fix the bug, right?  I agree the current
> code doesn't make sense there.

The first hunk is necessary to fix the bug.  The third hunk is not
necessary.

> The patch is ok.  I'm still somewhat confused by this function (and think
> that we do the wrong thing wrt preserving TBAA information), but the
> patch seems to be a strict improvement over the current situation.

Committed.  Thanks.

Ian
H.J. Lu - May 14, 2011, 11:46 p.m.
On Mon, Oct 25, 2010 at 9:24 PM, Ian Lance Taylor <iant@google.com> wrote:
> This patch implements Jakub's suggestion for fixing PR 45687.  The patch
> is mainly by inspection.  The test case fails before the patch and
> succeeds afterward, but the test case only tests the first hunk of the
> patch, not the third (the second is just a formatting fix).
>
> Bootstrapped and tested on x86_64-unknown-linux-gnu.  OK for mainline?
>
> Ian
>
>
> gcc/ChangeLog:
>
> 2010-10-25  Ian Lance Taylor  <iant@google.com>
>
>        PR middle-end/45687
>        * ipa-prop.c (ipa_modify_call_arguments): Correct type of MEM_REF
>        offset.
>

This caused:

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=49000

Patch

Index: ipa-prop.c
===================================================================
--- ipa-prop.c	(revision 165928)
+++ ipa-prop.c	(working copy)
@@ -2186,7 +2186,7 @@  ipa_modify_call_arguments (struct cgraph
 
 	  if (TREE_CODE (base) == ADDR_EXPR
 	      && DECL_P (TREE_OPERAND (base, 0)))
-	    off = build_int_cst (reference_alias_ptr_type (base),
+	    off = build_int_cst (TREE_TYPE (base),
 				 adj->offset / BITS_PER_UNIT);
 	  else if (TREE_CODE (base) != ADDR_EXPR
 		   && POINTER_TYPE_P (TREE_TYPE (base)))
@@ -2209,7 +2209,7 @@  ipa_modify_call_arguments (struct cgraph
 		}
 	      else if (TREE_CODE (base) == MEM_REF)
 		{
-		  off = build_int_cst (TREE_TYPE (TREE_OPERAND (base,1)),
+		  off = build_int_cst (TREE_TYPE (TREE_OPERAND (base, 1)),
 				       base_offset
 				       + adj->offset / BITS_PER_UNIT);
 		  off = int_const_binop (PLUS_EXPR, TREE_OPERAND (base, 1),
@@ -2218,7 +2218,7 @@  ipa_modify_call_arguments (struct cgraph
 		}
 	      else
 		{
-		  off = build_int_cst (reference_alias_ptr_type (base),
+		  off = build_int_cst (reference_alias_ptr_type (prev_base),
 				       base_offset
 				       + adj->offset / BITS_PER_UNIT);
 		  base = build_fold_addr_expr (base);
Index: testsuite/gcc.c-torture/execute/20101025-1.c
===================================================================
--- testsuite/gcc.c-torture/execute/20101025-1.c	(revision 0)
+++ testsuite/gcc.c-torture/execute/20101025-1.c	(revision 0)
@@ -0,0 +1,30 @@ 
+static int g_7;
+static int *volatile g_6 = &g_7;
+int g_3;
+
+static int f1 (int *p_58)
+{
+    return *p_58;
+}
+
+void f2 (int i) __attribute__ ((noinline));
+void f2 (int i)
+{
+  g_3 = i;
+}
+
+int f3 (void) __attribute__ ((noinline));
+int f3 (void)
+{
+    *g_6 = 1;
+    f2 (f1 (&g_7));
+    return 0;
+}
+
+int main ()
+{
+  f3 ();
+  if (g_3 != 1)
+    abort ();
+  exit (0);
+}