* PATCH [5/n]: Prepare x32: PR middle-end/48016: Inconsistency in non-local goto save area @ 2011-06-11 16:22 H.J. Lu 2011-06-15 14:37 ` Michael Matz 0 siblings, 1 reply; 11+ messages in thread From: H.J. Lu @ 2011-06-11 16:22 UTC (permalink / raw) To: gcc-patches Hi, We are very inconsistent when saving and restoring non-local goto save area. See: http://gcc.gnu.org/bugzilla/show_bug.cgi?id=48016 for detailed analysis. OK for trunk? Thanks. H.J. --- 2011-06-07 H.J. Lu <hongjiu.lu@intel.com> PR middle-end/48016 * explow.c (emit_stack_save): Adjust mode of stack save area. * function.c (expand_function_start): Properly store frame pointer for non-local goto. diff --git a/gcc/explow.c b/gcc/explow.c index 7387dad..b343bf8 100644 --- a/gcc/explow.c +++ b/gcc/explow.c @@ -1035,6 +1030,14 @@ emit_stack_save (enum save_level save_level, rtx *psave) do_pending_stack_adjust (); if (sa != 0) sa = validize_mem (sa); + /* FIXME: update_nonlocal_goto_save_area may pass SA in the wrong mode. */ + if (GET_MODE (sa) != mode) + { + gcc_assert (ptr_mode != Pmode + && GET_MODE (sa) == ptr_mode + && mode == Pmode); + sa = adjust_address (sa, mode, 0); + } emit_insn (fcn (sa, stack_pointer_rtx)); } diff --git a/gcc/function.c b/gcc/function.c index 30cc9ff..47fd5b7 100644 --- a/gcc/function.c +++ b/gcc/function.c @@ -4779,7 +4779,7 @@ expand_function_start (tree subr) cfun->nonlocal_goto_save_area, integer_zero_node, NULL_TREE, NULL_TREE); r_save = expand_expr (t_save, NULL_RTX, VOIDmode, EXPAND_WRITE); - r_save = convert_memory_address (Pmode, r_save); + r_save = adjust_address (r_save, Pmode, 0); emit_move_insn (r_save, targetm.builtin_setjmp_frame_value ()); update_nonlocal_goto_save_area (); ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: PATCH [5/n]: Prepare x32: PR middle-end/48016: Inconsistency in non-local goto save area 2011-06-11 16:22 PATCH [5/n]: Prepare x32: PR middle-end/48016: Inconsistency in non-local goto save area H.J. Lu @ 2011-06-15 14:37 ` Michael Matz 2011-06-15 15:10 ` H.J. Lu 0 siblings, 1 reply; 11+ messages in thread From: Michael Matz @ 2011-06-15 14:37 UTC (permalink / raw) To: H.J. Lu; +Cc: gcc-patches Hi, On Sat, 11 Jun 2011, H.J. Lu wrote: > We are very inconsistent when saving and restoring non-local goto save > area. See: > > http://gcc.gnu.org/bugzilla/show_bug.cgi?id=48016 > > for detailed analysis. OK for trunk? > + /* FIXME: update_nonlocal_goto_save_area may pass SA in the wrong mode. */ > + if (GET_MODE (sa) != mode) > + { > + gcc_assert (ptr_mode != Pmode > + && GET_MODE (sa) == ptr_mode > + && mode == Pmode); > + sa = adjust_address (sa, mode, 0); > + } That may be appropriate for a branch, but trunk shouldn't contain FIXMEs that explain how something should be fixed, instead that something should be carried out. I.e. just fix update_nonlocal_goto_save_area. Ciao, Michael. ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: PATCH [5/n]: Prepare x32: PR middle-end/48016: Inconsistency in non-local goto save area 2011-06-15 14:37 ` Michael Matz @ 2011-06-15 15:10 ` H.J. Lu [not found] ` <Pine.LNX.4.64.1106151659200.17115@wotan.suse.de> 0 siblings, 1 reply; 11+ messages in thread From: H.J. Lu @ 2011-06-15 15:10 UTC (permalink / raw) To: Michael Matz; +Cc: gcc-patches On Wed, Jun 15, 2011 at 7:11 AM, Michael Matz <matz@suse.de> wrote: > Hi, > > On Sat, 11 Jun 2011, H.J. Lu wrote: > >> We are very inconsistent when saving and restoring non-local goto save >> area. See: >> >> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=48016 >> >> for detailed analysis. OK for trunk? >> + /* FIXME: update_nonlocal_goto_save_area may pass SA in the wrong mode. */ >> + if (GET_MODE (sa) != mode) >> + { >> + gcc_assert (ptr_mode != Pmode >> + && GET_MODE (sa) == ptr_mode >> + && mode == Pmode); >> + sa = adjust_address (sa, mode, 0); >> + } > > That may be appropriate for a branch, but trunk shouldn't contain FIXMEs > that explain how something should be fixed, instead that something should > be carried out. I.e. just fix update_nonlocal_goto_save_area. > I don't know update_nonlocal_goto_save_area enough to fix it without breaking other targets. This patch is the lest invasive. Any suggestions how to properly fix it is appreciated. Thanks. -- H.J. ^ permalink raw reply [flat|nested] 11+ messages in thread
[parent not found: <Pine.LNX.4.64.1106151659200.17115@wotan.suse.de>]
* Re: PATCH [5/n]: Prepare x32: PR middle-end/48016: Inconsistency in non-local goto save area [not found] ` <Pine.LNX.4.64.1106151659200.17115@wotan.suse.de> @ 2011-06-16 6:41 ` H.J. Lu 2011-06-16 8:02 ` Richard Guenther 0 siblings, 1 reply; 11+ messages in thread From: H.J. Lu @ 2011-06-16 6:41 UTC (permalink / raw) To: Michael Matz; +Cc: gcc-patches [-- Attachment #1: Type: text/plain, Size: 2214 bytes --] On Wed, Jun 15, 2011 at 8:16 AM, Michael Matz <matz@suse.de> wrote: > Hi, > > On Wed, 15 Jun 2011, H.J. Lu wrote: > >> >> + /* FIXME: update_nonlocal_goto_save_area may pass SA in the wrong mode. */ >> >> + if (GET_MODE (sa) != mode) >> >> + { >> >> + gcc_assert (ptr_mode != Pmode >> >> + && GET_MODE (sa) == ptr_mode >> >> + && mode == Pmode); >> >> + sa = adjust_address (sa, mode, 0); >> >> + } >> > >> > That may be appropriate for a branch, but trunk shouldn't contain FIXMEs >> > that explain how something should be fixed, instead that something should >> > be carried out. I.e. just fix update_nonlocal_goto_save_area. >> > >> >> I don't know update_nonlocal_goto_save_area enough to fix it >> without breaking other targets. This patch is the lest invasive. >> Any suggestions how to properly fix it is appreciated. > > Well, the most obvious variant would be to move the above code right > before the call of emit_stack_save in update_nonlocal_goto_save_area > (using r_save and STACK_SAVEAREA_MODE (SAVE_NONLOCAL)). All other callers > of emit_stack_save already make sure to pass an object of correct mode, so > this one should too. > > But I think it's better to just produce a correct array_ref from the > start. get_nl_goto_field creates an array_type for the > nonlocal_goto_save_area of correct type (ptr_type_node or > lang_hooks.types.type_for_mode (Pmode, 1)), and we should use that. > > So something like this in update_nonlocal_goto_save_area: > t_save = build4 (ARRAY_REF, > TREE_TYPE (TREE_TYPE (cfun->nonlocal_goto_save_area)), > cfun->nonlocal_goto_save_area, > integer_one_node, NULL_TREE, NULL_TREE); > > instead of the current building of t_save. Then r_save also should get > the correct mode automatically. > Here is the updated patch. OK for trunk? Thanks. -- H.J. --- 2011-06-15 H.J. Lu <hongjiu.lu@intel.com> PR middle-end/48016 * explow.c (update_nonlocal_goto_save_area): Use proper mode for stack save area. * function.c (expand_function_start): Properly store frame pointer for non-local goto. [-- Attachment #2: gcc-x32-pr48016-2.patch --] [-- Type: text/x-diff, Size: 1482 bytes --] 2011-06-15 H.J. Lu <hongjiu.lu@intel.com> PR middle-end/48016 * explow.c (update_nonlocal_goto_save_area): Use proper mode for stack save area. * function.c (expand_function_start): Properly store frame pointer for non-local goto. diff --git a/gcc/explow.c b/gcc/explow.c index c7d8183..efe6c7e 100644 --- a/gcc/explow.c +++ b/gcc/explow.c @@ -1102,7 +1097,9 @@ update_nonlocal_goto_save_area (void) first one is used for the frame pointer save; the rest are sized by STACK_SAVEAREA_MODE. Create a reference to array index 1, the first of the stack save area slots. */ - t_save = build4 (ARRAY_REF, ptr_type_node, cfun->nonlocal_goto_save_area, + t_save = build4 (ARRAY_REF, + TREE_TYPE (TREE_TYPE (cfun->nonlocal_goto_save_area)), + cfun->nonlocal_goto_save_area, integer_one_node, NULL_TREE, NULL_TREE); r_save = expand_expr (t_save, NULL_RTX, VOIDmode, EXPAND_WRITE); diff --git a/gcc/function.c b/gcc/function.c index 81c4d39..131bc09 100644 --- a/gcc/function.c +++ b/gcc/function.c @@ -4780,7 +4780,7 @@ expand_function_start (tree subr) cfun->nonlocal_goto_save_area, integer_zero_node, NULL_TREE, NULL_TREE); r_save = expand_expr (t_save, NULL_RTX, VOIDmode, EXPAND_WRITE); - r_save = convert_memory_address (Pmode, r_save); + r_save = adjust_address (r_save, Pmode, 0); emit_move_insn (r_save, targetm.builtin_setjmp_frame_value ()); update_nonlocal_goto_save_area (); ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: PATCH [5/n]: Prepare x32: PR middle-end/48016: Inconsistency in non-local goto save area 2011-06-16 6:41 ` H.J. Lu @ 2011-06-16 8:02 ` Richard Guenther 2011-06-16 17:59 ` H.J. Lu 0 siblings, 1 reply; 11+ messages in thread From: Richard Guenther @ 2011-06-16 8:02 UTC (permalink / raw) To: H.J. Lu; +Cc: Michael Matz, gcc-patches On Wed, Jun 15, 2011 at 9:55 PM, H.J. Lu <hjl.tools@gmail.com> wrote: > On Wed, Jun 15, 2011 at 8:16 AM, Michael Matz <matz@suse.de> wrote: >> Hi, >> >> On Wed, 15 Jun 2011, H.J. Lu wrote: >> >>> >> + /* FIXME: update_nonlocal_goto_save_area may pass SA in the wrong mode. */ >>> >> + if (GET_MODE (sa) != mode) >>> >> + { >>> >> + gcc_assert (ptr_mode != Pmode >>> >> + && GET_MODE (sa) == ptr_mode >>> >> + && mode == Pmode); >>> >> + sa = adjust_address (sa, mode, 0); >>> >> + } >>> > >>> > That may be appropriate for a branch, but trunk shouldn't contain FIXMEs >>> > that explain how something should be fixed, instead that something should >>> > be carried out. I.e. just fix update_nonlocal_goto_save_area. >>> > >>> >>> I don't know update_nonlocal_goto_save_area enough to fix it >>> without breaking other targets. This patch is the lest invasive. >>> Any suggestions how to properly fix it is appreciated. >> >> Well, the most obvious variant would be to move the above code right >> before the call of emit_stack_save in update_nonlocal_goto_save_area >> (using r_save and STACK_SAVEAREA_MODE (SAVE_NONLOCAL)). All other callers >> of emit_stack_save already make sure to pass an object of correct mode, so >> this one should too. >> >> But I think it's better to just produce a correct array_ref from the >> start. get_nl_goto_field creates an array_type for the >> nonlocal_goto_save_area of correct type (ptr_type_node or >> lang_hooks.types.type_for_mode (Pmode, 1)), and we should use that. >> >> So something like this in update_nonlocal_goto_save_area: >> t_save = build4 (ARRAY_REF, >> TREE_TYPE (TREE_TYPE (cfun->nonlocal_goto_save_area)), >> cfun->nonlocal_goto_save_area, >> integer_one_node, NULL_TREE, NULL_TREE); >> >> instead of the current building of t_save. Then r_save also should get >> the correct mode automatically. >> > > Here is the updated patch. OK for trunk? The explow.c change is ok. For the function.c change I wonder why convert_memory_address doesn't do the right thing - from it's documentation it definitely should, so it should be fixed instead of being replaced by adjust_address with a zero offset. Richard. > Thanks. > > -- > H.J. > --- > 2011-06-15 H.J. Lu <hongjiu.lu@intel.com> > > PR middle-end/48016 > * explow.c (update_nonlocal_goto_save_area): Use proper mode > for stack save area. > > * function.c (expand_function_start): Properly store frame > pointer for non-local goto. > ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: PATCH [5/n]: Prepare x32: PR middle-end/48016: Inconsistency in non-local goto save area 2011-06-16 8:02 ` Richard Guenther @ 2011-06-16 17:59 ` H.J. Lu 2011-06-25 16:20 ` H.J. Lu 0 siblings, 1 reply; 11+ messages in thread From: H.J. Lu @ 2011-06-16 17:59 UTC (permalink / raw) To: Richard Guenther; +Cc: Michael Matz, gcc-patches On Thu, Jun 16, 2011 at 12:56 AM, Richard Guenther <richard.guenther@gmail.com> wrote: > On Wed, Jun 15, 2011 at 9:55 PM, H.J. Lu <hjl.tools@gmail.com> wrote: >> On Wed, Jun 15, 2011 at 8:16 AM, Michael Matz <matz@suse.de> wrote: >>> Hi, >>> >>> On Wed, 15 Jun 2011, H.J. Lu wrote: >>> >>>> >> + /* FIXME: update_nonlocal_goto_save_area may pass SA in the wrong mode. */ >>>> >> + if (GET_MODE (sa) != mode) >>>> >> + { >>>> >> + gcc_assert (ptr_mode != Pmode >>>> >> + && GET_MODE (sa) == ptr_mode >>>> >> + && mode == Pmode); >>>> >> + sa = adjust_address (sa, mode, 0); >>>> >> + } >>>> > >>>> > That may be appropriate for a branch, but trunk shouldn't contain FIXMEs >>>> > that explain how something should be fixed, instead that something should >>>> > be carried out. I.e. just fix update_nonlocal_goto_save_area. >>>> > >>>> >>>> I don't know update_nonlocal_goto_save_area enough to fix it >>>> without breaking other targets. This patch is the lest invasive. >>>> Any suggestions how to properly fix it is appreciated. >>> >>> Well, the most obvious variant would be to move the above code right >>> before the call of emit_stack_save in update_nonlocal_goto_save_area >>> (using r_save and STACK_SAVEAREA_MODE (SAVE_NONLOCAL)). All other callers >>> of emit_stack_save already make sure to pass an object of correct mode, so >>> this one should too. >>> >>> But I think it's better to just produce a correct array_ref from the >>> start. get_nl_goto_field creates an array_type for the >>> nonlocal_goto_save_area of correct type (ptr_type_node or >>> lang_hooks.types.type_for_mode (Pmode, 1)), and we should use that. >>> >>> So something like this in update_nonlocal_goto_save_area: >>> t_save = build4 (ARRAY_REF, >>> TREE_TYPE (TREE_TYPE (cfun->nonlocal_goto_save_area)), >>> cfun->nonlocal_goto_save_area, >>> integer_one_node, NULL_TREE, NULL_TREE); >>> >>> instead of the current building of t_save. Then r_save also should get >>> the correct mode automatically. >>> >> >> Here is the updated patch. OK for trunk? > > The explow.c change is ok. For the function.c change I wonder why > convert_memory_address doesn't do the right thing - from it's documentation > it definitely should, so it should be fixed instead of being replaced by > adjust_address with a zero offset. > convert_memory_address may return a pseudo register converted to Pmode. But here what we want is the same memory address adjusted for Pmode. I don't think the usage of convert_memory_address -- H.J. ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: PATCH [5/n]: Prepare x32: PR middle-end/48016: Inconsistency in non-local goto save area 2011-06-16 17:59 ` H.J. Lu @ 2011-06-25 16:20 ` H.J. Lu 2011-06-29 15:25 ` H.J. Lu 0 siblings, 1 reply; 11+ messages in thread From: H.J. Lu @ 2011-06-25 16:20 UTC (permalink / raw) To: Richard Guenther; +Cc: Michael Matz, gcc-patches On Thu, Jun 16, 2011 at 10:18 AM, H.J. Lu <hjl.tools@gmail.com> wrote: > On Thu, Jun 16, 2011 at 12:56 AM, Richard Guenther > <richard.guenther@gmail.com> wrote: >> On Wed, Jun 15, 2011 at 9:55 PM, H.J. Lu <hjl.tools@gmail.com> wrote: >>> On Wed, Jun 15, 2011 at 8:16 AM, Michael Matz <matz@suse.de> wrote: >>>> Hi, >>>> >>>> On Wed, 15 Jun 2011, H.J. Lu wrote: >>>> >>>>> >> + /* FIXME: update_nonlocal_goto_save_area may pass SA in the wrong mode. */ >>>>> >> + if (GET_MODE (sa) != mode) >>>>> >> + { >>>>> >> + gcc_assert (ptr_mode != Pmode >>>>> >> + && GET_MODE (sa) == ptr_mode >>>>> >> + && mode == Pmode); >>>>> >> + sa = adjust_address (sa, mode, 0); >>>>> >> + } >>>>> > >>>>> > That may be appropriate for a branch, but trunk shouldn't contain FIXMEs >>>>> > that explain how something should be fixed, instead that something should >>>>> > be carried out. I.e. just fix update_nonlocal_goto_save_area. >>>>> > >>>>> >>>>> I don't know update_nonlocal_goto_save_area enough to fix it >>>>> without breaking other targets. This patch is the lest invasive. >>>>> Any suggestions how to properly fix it is appreciated. >>>> >>>> Well, the most obvious variant would be to move the above code right >>>> before the call of emit_stack_save in update_nonlocal_goto_save_area >>>> (using r_save and STACK_SAVEAREA_MODE (SAVE_NONLOCAL)). All other callers >>>> of emit_stack_save already make sure to pass an object of correct mode, so >>>> this one should too. >>>> >>>> But I think it's better to just produce a correct array_ref from the >>>> start. get_nl_goto_field creates an array_type for the >>>> nonlocal_goto_save_area of correct type (ptr_type_node or >>>> lang_hooks.types.type_for_mode (Pmode, 1)), and we should use that. >>>> >>>> So something like this in update_nonlocal_goto_save_area: >>>> t_save = build4 (ARRAY_REF, >>>> TREE_TYPE (TREE_TYPE (cfun->nonlocal_goto_save_area)), >>>> cfun->nonlocal_goto_save_area, >>>> integer_one_node, NULL_TREE, NULL_TREE); >>>> >>>> instead of the current building of t_save. Then r_save also should get >>>> the correct mode automatically. >>>> >>> >>> Here is the updated patch. OK for trunk? >> >> The explow.c change is ok. For the function.c change I wonder why >> convert_memory_address doesn't do the right thing - from it's documentation >> it definitely should, so it should be fixed instead of being replaced by >> adjust_address with a zero offset. >> > > convert_memory_address may return a pseudo register converted > to Pmode. But here what we want is the same memory address > adjusted for Pmode. I don't think the usage of convert_memory_address > Here is the code in question: r_save = convert_memory_address (Pmode, r_save); emit_move_insn (r_save, targetm.builtin_setjmp_frame_value ()); R_SAVE must be lvalue. But return from convert_memory_address isn't. I am re-posting my patch here. OK for trunk? Thanks. -- H.J. --- 2011-06-15 H.J. Lu <hongjiu.lu@intel.com> PR middle-end/48016 * explow.c (update_nonlocal_goto_save_area): Use proper mode for stack save area. * function.c (expand_function_start): Properly store frame pointer for non-local goto. diff --git a/gcc/explow.c b/gcc/explow.c index c7d8183..efe6c7e 100644 --- a/gcc/explow.c +++ b/gcc/explow.c @@ -1102,7 +1097,9 @@ update_nonlocal_goto_save_area (void) first one is used for the frame pointer save; the rest are sized by STACK_SAVEAREA_MODE. Create a reference to array index 1, the first of the stack save area slots. */ - t_save = build4 (ARRAY_REF, ptr_type_node, cfun->nonlocal_goto_save_area, + t_save = build4 (ARRAY_REF, + TREE_TYPE (TREE_TYPE (cfun->nonlocal_goto_save_area)), + cfun->nonlocal_goto_save_area, integer_one_node, NULL_TREE, NULL_TREE); r_save = expand_expr (t_save, NULL_RTX, VOIDmode, EXPAND_WRITE); diff --git a/gcc/function.c b/gcc/function.c index 81c4d39..131bc09 100644 --- a/gcc/function.c +++ b/gcc/function.c @@ -4780,7 +4780,7 @@ expand_function_start (tree subr) cfun->nonlocal_goto_save_area, integer_zero_node, NULL_TREE, NULL_TREE); r_save = expand_expr (t_save, NULL_RTX, VOIDmode, EXPAND_WRITE); - r_save = convert_memory_address (Pmode, r_save); + r_save = adjust_address (r_save, Pmode, 0); emit_move_insn (r_save, targetm.builtin_setjmp_frame_value ()); update_nonlocal_goto_save_area (); ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: PATCH [5/n]: Prepare x32: PR middle-end/48016: Inconsistency in non-local goto save area 2011-06-25 16:20 ` H.J. Lu @ 2011-06-29 15:25 ` H.J. Lu 2011-06-29 17:03 ` Michael Matz 0 siblings, 1 reply; 11+ messages in thread From: H.J. Lu @ 2011-06-29 15:25 UTC (permalink / raw) To: Richard Guenther; +Cc: Michael Matz, gcc-patches Ping. On Sat, Jun 25, 2011 at 9:06 AM, H.J. Lu <hjl.tools@gmail.com> wrote: > On Thu, Jun 16, 2011 at 10:18 AM, H.J. Lu <hjl.tools@gmail.com> wrote: >> On Thu, Jun 16, 2011 at 12:56 AM, Richard Guenther >> <richard.guenther@gmail.com> wrote: >>> On Wed, Jun 15, 2011 at 9:55 PM, H.J. Lu <hjl.tools@gmail.com> wrote: >>>> On Wed, Jun 15, 2011 at 8:16 AM, Michael Matz <matz@suse.de> wrote: >>>>> Hi, >>>>> >>>>> On Wed, 15 Jun 2011, H.J. Lu wrote: >>>>> >>>>>> >> + /* FIXME: update_nonlocal_goto_save_area may pass SA in the wrong mode. */ >>>>>> >> + if (GET_MODE (sa) != mode) >>>>>> >> + { >>>>>> >> + gcc_assert (ptr_mode != Pmode >>>>>> >> + && GET_MODE (sa) == ptr_mode >>>>>> >> + && mode == Pmode); >>>>>> >> + sa = adjust_address (sa, mode, 0); >>>>>> >> + } >>>>>> > >>>>>> > That may be appropriate for a branch, but trunk shouldn't contain FIXMEs >>>>>> > that explain how something should be fixed, instead that something should >>>>>> > be carried out. I.e. just fix update_nonlocal_goto_save_area. >>>>>> > >>>>>> >>>>>> I don't know update_nonlocal_goto_save_area enough to fix it >>>>>> without breaking other targets. This patch is the lest invasive. >>>>>> Any suggestions how to properly fix it is appreciated. >>>>> >>>>> Well, the most obvious variant would be to move the above code right >>>>> before the call of emit_stack_save in update_nonlocal_goto_save_area >>>>> (using r_save and STACK_SAVEAREA_MODE (SAVE_NONLOCAL)). All other callers >>>>> of emit_stack_save already make sure to pass an object of correct mode, so >>>>> this one should too. >>>>> >>>>> But I think it's better to just produce a correct array_ref from the >>>>> start. get_nl_goto_field creates an array_type for the >>>>> nonlocal_goto_save_area of correct type (ptr_type_node or >>>>> lang_hooks.types.type_for_mode (Pmode, 1)), and we should use that. >>>>> >>>>> So something like this in update_nonlocal_goto_save_area: >>>>> t_save = build4 (ARRAY_REF, >>>>> TREE_TYPE (TREE_TYPE (cfun->nonlocal_goto_save_area)), >>>>> cfun->nonlocal_goto_save_area, >>>>> integer_one_node, NULL_TREE, NULL_TREE); >>>>> >>>>> instead of the current building of t_save. Then r_save also should get >>>>> the correct mode automatically. >>>>> >>>> >>>> Here is the updated patch. OK for trunk? >>> >>> The explow.c change is ok. For the function.c change I wonder why >>> convert_memory_address doesn't do the right thing - from it's documentation >>> it definitely should, so it should be fixed instead of being replaced by >>> adjust_address with a zero offset. >>> >> >> convert_memory_address may return a pseudo register converted >> to Pmode. But here what we want is the same memory address >> adjusted for Pmode. I don't think the usage of convert_memory_address >> > > Here is the code in question: > > r_save = convert_memory_address (Pmode, r_save); > > emit_move_insn (r_save, targetm.builtin_setjmp_frame_value ()); > > R_SAVE must be lvalue. But return from convert_memory_address > isn't. I am re-posting my patch here. OK for trunk? > > Thanks. > > -- > H.J. > --- > 2011-06-15 H.J. Lu <hongjiu.lu@intel.com> > > PR middle-end/48016 > * explow.c (update_nonlocal_goto_save_area): Use proper mode > for stack save area. > > * function.c (expand_function_start): Properly store frame > pointer for non-local goto. > > diff --git a/gcc/explow.c b/gcc/explow.c > index c7d8183..efe6c7e 100644 > --- a/gcc/explow.c > +++ b/gcc/explow.c > @@ -1102,7 +1097,9 @@ update_nonlocal_goto_save_area (void) > first one is used for the frame pointer save; the rest are sized by > STACK_SAVEAREA_MODE. Create a reference to array index 1, the first > of the stack save area slots. */ > - t_save = build4 (ARRAY_REF, ptr_type_node, cfun->nonlocal_goto_save_area, > + t_save = build4 (ARRAY_REF, > + TREE_TYPE (TREE_TYPE (cfun->nonlocal_goto_save_area)), > + cfun->nonlocal_goto_save_area, > integer_one_node, NULL_TREE, NULL_TREE); > r_save = expand_expr (t_save, NULL_RTX, VOIDmode, EXPAND_WRITE); > > diff --git a/gcc/function.c b/gcc/function.c > index 81c4d39..131bc09 100644 > --- a/gcc/function.c > +++ b/gcc/function.c > @@ -4780,7 +4780,7 @@ expand_function_start (tree subr) > cfun->nonlocal_goto_save_area, > integer_zero_node, NULL_TREE, NULL_TREE); > r_save = expand_expr (t_save, NULL_RTX, VOIDmode, EXPAND_WRITE); > - r_save = convert_memory_address (Pmode, r_save); > + r_save = adjust_address (r_save, Pmode, 0); > > emit_move_insn (r_save, targetm.builtin_setjmp_frame_value ()); > update_nonlocal_goto_save_area (); > -- H.J. ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: PATCH [5/n]: Prepare x32: PR middle-end/48016: Inconsistency in non-local goto save area 2011-06-29 15:25 ` H.J. Lu @ 2011-06-29 17:03 ` Michael Matz 2011-06-30 18:10 ` H.J. Lu 0 siblings, 1 reply; 11+ messages in thread From: Michael Matz @ 2011-06-29 17:03 UTC (permalink / raw) To: H.J. Lu; +Cc: Richard Guenther, gcc-patches [-- Attachment #1: Type: TEXT/PLAIN, Size: 1183 bytes --] Hi, On Wed, 29 Jun 2011, H.J. Lu wrote: > > diff --git a/gcc/function.c b/gcc/function.c > > index 81c4d39..131bc09 100644 > > --- a/gcc/function.c > > +++ b/gcc/function.c > > @@ -4780,7 +4780,7 @@ expand_function_start (tree subr) > > Â Â Â Â Â Â Â Â Â Â Â cfun->nonlocal_goto_save_area, > > Â Â Â Â Â Â Â Â Â Â Â integer_zero_node, NULL_TREE, NULL_TREE); > > Â Â Â r_save = expand_expr (t_save, NULL_RTX, VOIDmode, EXPAND_WRITE); > > - Â Â Â r_save = convert_memory_address (Pmode, r_save); > > + Â Â Â r_save = adjust_address (r_save, Pmode, 0); This is actually the same problem as in explow.c. t_save is built with ptr_type_node, where it should have been using TREE_TYPE (TREE_TYPE (cfun->nonlocal_goto_save_area)) Then r_save should have the correct mode already, possibly this could be asserted. You are right that r_save needs to correspond to the nonlocal_goto_save_area[0] array-ref, hence pseudos aren't okay, therefore convert_memory_address isn't. Actually I think we might even want to assert that indeed the expanded r_save is of Pmode already. Ciao, Michael. ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: PATCH [5/n]: Prepare x32: PR middle-end/48016: Inconsistency in non-local goto save area 2011-06-29 17:03 ` Michael Matz @ 2011-06-30 18:10 ` H.J. Lu 2011-07-01 8:01 ` Richard Guenther 0 siblings, 1 reply; 11+ messages in thread From: H.J. Lu @ 2011-06-30 18:10 UTC (permalink / raw) To: Michael Matz; +Cc: Richard Guenther, gcc-patches [-- Attachment #1: Type: text/plain, Size: 1508 bytes --] On Wed, Jun 29, 2011 at 9:16 AM, Michael Matz <matz@suse.de> wrote: > Hi, > > On Wed, 29 Jun 2011, H.J. Lu wrote: > >> > diff --git a/gcc/function.c b/gcc/function.c >> > index 81c4d39..131bc09 100644 >> > --- a/gcc/function.c >> > +++ b/gcc/function.c >> > @@ -4780,7 +4780,7 @@ expand_function_start (tree subr) >> > cfun->nonlocal_goto_save_area, >> > integer_zero_node, NULL_TREE, NULL_TREE); >> > r_save = expand_expr (t_save, NULL_RTX, VOIDmode, EXPAND_WRITE); >> > - r_save = convert_memory_address (Pmode, r_save); >> > + r_save = adjust_address (r_save, Pmode, 0); > > This is actually the same problem as in explow.c. t_save is built with > ptr_type_node, where it should have been using > TREE_TYPE (TREE_TYPE (cfun->nonlocal_goto_save_area)) > > Then r_save should have the correct mode already, possibly this could be > asserted. You are right that r_save needs to correspond to the > nonlocal_goto_save_area[0] array-ref, hence pseudos aren't okay, therefore > convert_memory_address isn't. Actually I think we might even want to > assert that indeed the expanded r_save is of Pmode already. > > This patch works for me. OK for trunk if there are no regressions? Thanks. -- H.J. --- 2011-06-30 H.J. Lu <hongjiu.lu@intel.com> PR middle-end/48016 * explow.c (update_nonlocal_goto_save_area): Use proper mode for stack save area. * function.c (expand_function_start): Likewise. [-- Attachment #2: gcc-x32-pr48016-3.patch --] [-- Type: text/plain, Size: 1648 bytes --] 2011-06-30 H.J. Lu <hongjiu.lu@intel.com> PR middle-end/48016 * explow.c (update_nonlocal_goto_save_area): Use proper mode for stack save area. * function.c (expand_function_start): Likewise. diff --git a/gcc/explow.c b/gcc/explow.c index c7d8183..efe6c7e 100644 --- a/gcc/explow.c +++ b/gcc/explow.c @@ -1102,7 +1097,9 @@ update_nonlocal_goto_save_area (void) first one is used for the frame pointer save; the rest are sized by STACK_SAVEAREA_MODE. Create a reference to array index 1, the first of the stack save area slots. */ - t_save = build4 (ARRAY_REF, ptr_type_node, cfun->nonlocal_goto_save_area, + t_save = build4 (ARRAY_REF, + TREE_TYPE (TREE_TYPE (cfun->nonlocal_goto_save_area)), + cfun->nonlocal_goto_save_area, integer_one_node, NULL_TREE, NULL_TREE); r_save = expand_expr (t_save, NULL_RTX, VOIDmode, EXPAND_WRITE); diff --git a/gcc/function.c b/gcc/function.c index 5be018a..0b2f5aa 100644 --- a/gcc/function.c +++ b/gcc/function.c @@ -4782,11 +4782,12 @@ expand_function_start (tree subr) if (!DECL_RTL_SET_P (var)) expand_decl (var); - t_save = build4 (ARRAY_REF, ptr_type_node, + t_save = build4 (ARRAY_REF, + TREE_TYPE (TREE_TYPE (cfun->nonlocal_goto_save_area)), cfun->nonlocal_goto_save_area, integer_zero_node, NULL_TREE, NULL_TREE); r_save = expand_expr (t_save, NULL_RTX, VOIDmode, EXPAND_WRITE); - r_save = convert_memory_address (Pmode, r_save); + gcc_assert (GET_MODE (r_save) == Pmode); emit_move_insn (r_save, targetm.builtin_setjmp_frame_value ()); update_nonlocal_goto_save_area (); ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: PATCH [5/n]: Prepare x32: PR middle-end/48016: Inconsistency in non-local goto save area 2011-06-30 18:10 ` H.J. Lu @ 2011-07-01 8:01 ` Richard Guenther 0 siblings, 0 replies; 11+ messages in thread From: Richard Guenther @ 2011-07-01 8:01 UTC (permalink / raw) To: H.J. Lu; +Cc: Michael Matz, gcc-patches On Thu, Jun 30, 2011 at 8:05 PM, H.J. Lu <hjl.tools@gmail.com> wrote: > On Wed, Jun 29, 2011 at 9:16 AM, Michael Matz <matz@suse.de> wrote: >> Hi, >> >> On Wed, 29 Jun 2011, H.J. Lu wrote: >> >>> > diff --git a/gcc/function.c b/gcc/function.c >>> > index 81c4d39..131bc09 100644 >>> > --- a/gcc/function.c >>> > +++ b/gcc/function.c >>> > @@ -4780,7 +4780,7 @@ expand_function_start (tree subr) >>> > cfun->nonlocal_goto_save_area, >>> > integer_zero_node, NULL_TREE, NULL_TREE); >>> > r_save = expand_expr (t_save, NULL_RTX, VOIDmode, EXPAND_WRITE); >>> > - r_save = convert_memory_address (Pmode, r_save); >>> > + r_save = adjust_address (r_save, Pmode, 0); >> >> This is actually the same problem as in explow.c. t_save is built with >> ptr_type_node, where it should have been using >> TREE_TYPE (TREE_TYPE (cfun->nonlocal_goto_save_area)) >> >> Then r_save should have the correct mode already, possibly this could be >> asserted. You are right that r_save needs to correspond to the >> nonlocal_goto_save_area[0] array-ref, hence pseudos aren't okay, therefore >> convert_memory_address isn't. Actually I think we might even want to >> assert that indeed the expanded r_save is of Pmode already. >> >> > > This patch works for me. OK for trunk if there are no regressions? Ok. Thanks, Richard. > Thanks. > > > -- > H.J. > --- > 2011-06-30 H.J. Lu <hongjiu.lu@intel.com> > > PR middle-end/48016 > * explow.c (update_nonlocal_goto_save_area): Use proper mode > for stack save area. > * function.c (expand_function_start): Likewise. > ^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2011-07-01 8:01 UTC | newest] Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2011-06-11 16:22 PATCH [5/n]: Prepare x32: PR middle-end/48016: Inconsistency in non-local goto save area H.J. Lu 2011-06-15 14:37 ` Michael Matz 2011-06-15 15:10 ` H.J. Lu [not found] ` <Pine.LNX.4.64.1106151659200.17115@wotan.suse.de> 2011-06-16 6:41 ` H.J. Lu 2011-06-16 8:02 ` Richard Guenther 2011-06-16 17:59 ` H.J. Lu 2011-06-25 16:20 ` H.J. Lu 2011-06-29 15:25 ` H.J. Lu 2011-06-29 17:03 ` Michael Matz 2011-06-30 18:10 ` H.J. Lu 2011-07-01 8:01 ` 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).