| Message ID | mcrk4l5twd4.fsf@google.com |
|---|---|
| State | New |
| Headers | show |
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. > > >
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
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
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); +}