Message ID | ea5da06f-39aa-5e27-ce87-4e50233a72a8@codesourcery.com |
---|---|
State | New |
Headers | show |
Series | middle-end: move initialization of stack_limit_rtx [PR103163] | expand |
On 1/8/22 9:24 PM, Sandra Loosemore wrote: > This patch fixes the ICE I reported in PR103163. We were initializing > stack_limit_rtx before the register properties it depends on were > getting set. I moved it to the same function where stack_pointer_rtx, > frame_pointer_rtx, etc are being initialized. > > Besides nios2 where I observed it, this bug was also reported to affect > powerpc. Anybody want to check it there? Otherwise, OK to check in? Ping? https://gcc.gnu.org/pipermail/gcc-patches/2022-January/587958.html -Sandra
On Mon, Jan 17, 2022 at 4:15 AM Sandra Loosemore <sandra@codesourcery.com> wrote: > > On 1/8/22 9:24 PM, Sandra Loosemore wrote: > > This patch fixes the ICE I reported in PR103163. We were initializing > > stack_limit_rtx before the register properties it depends on were > > getting set. I moved it to the same function where stack_pointer_rtx, > > frame_pointer_rtx, etc are being initialized. > > > > Besides nios2 where I observed it, this bug was also reported to affect > > powerpc. Anybody want to check it there? Otherwise, OK to check in? > > Ping? > > https://gcc.gnu.org/pipermail/gcc-patches/2022-January/587958.html This move will now also re-initialize the pointer during target_reinit (), is that desired and correct? Richard. > -Sandra
On 1/17/22 1:29 AM, Richard Biener wrote: > On Mon, Jan 17, 2022 at 4:15 AM Sandra Loosemore > <sandra@codesourcery.com> wrote: >> >> On 1/8/22 9:24 PM, Sandra Loosemore wrote: >>> This patch fixes the ICE I reported in PR103163. We were initializing >>> stack_limit_rtx before the register properties it depends on were >>> getting set. I moved it to the same function where stack_pointer_rtx, >>> frame_pointer_rtx, etc are being initialized. >>> >>> Besides nios2 where I observed it, this bug was also reported to affect >>> powerpc. Anybody want to check it there? Otherwise, OK to check in? >> >> Ping? >> >> https://gcc.gnu.org/pipermail/gcc-patches/2022-January/587958.html > > This move will now also re-initialize the pointer during target_reinit (), > is that desired and correct? I did consider that when I wrote the patch -- I can't claim to be an expert on the re-initialization parts of the compiler any more (it's been so long since I worked on that refactoring, and my brain is suffering from bit rot), but it seems reasonable to handle stack_limit_rtx the same way as other frame-related rtx variables, in case the register properties have changed in some way that would affect them. -Sandra
On Mon, Jan 17, 2022 at 7:20 PM Sandra Loosemore <sandra@codesourcery.com> wrote: > > On 1/17/22 1:29 AM, Richard Biener wrote: > > On Mon, Jan 17, 2022 at 4:15 AM Sandra Loosemore > > <sandra@codesourcery.com> wrote: > >> > >> On 1/8/22 9:24 PM, Sandra Loosemore wrote: > >>> This patch fixes the ICE I reported in PR103163. We were initializing > >>> stack_limit_rtx before the register properties it depends on were > >>> getting set. I moved it to the same function where stack_pointer_rtx, > >>> frame_pointer_rtx, etc are being initialized. > >>> > >>> Besides nios2 where I observed it, this bug was also reported to affect > >>> powerpc. Anybody want to check it there? Otherwise, OK to check in? > >> > >> Ping? > >> > >> https://gcc.gnu.org/pipermail/gcc-patches/2022-January/587958.html > > > > This move will now also re-initialize the pointer during target_reinit (), > > is that desired and correct? > > I did consider that when I wrote the patch -- I can't claim to be an > expert on the re-initialization parts of the compiler any more (it's > been so long since I worked on that refactoring, and my brain is > suffering from bit rot), but it seems reasonable to handle > stack_limit_rtx the same way as other frame-related rtx variables, in > case the register properties have changed in some way that would affect > them. That was my thought as well, so as you did consider it the patch is OK. Thanks, Richard. > > -Sandra
commit bd91ec874339f9fd256b2d83de7159f6c11f0000 Author: Sandra Loosemore <sandra@codesourcery.com> Date: Sat Jan 8 19:59:26 2022 -0800 middle-end: move initialization of stack_limit_rtx [PR103163] stack_limit_rtx was being initialized before init_reg_modes_target (), resulting in the REG expression being created incorrectly and an ICE later in compilation. 2022-01-08 Sandra Loosemore <sandra@codesourcery.com> PR middle-end/103163 gcc/ * emit-rtl.c (init_emit_regs): Initialize stack_limit_rtx here... (init_emit_once): ...not here. diff --git a/gcc/emit-rtl.c b/gcc/emit-rtl.c index feeee16..76dbe42 100644 --- a/gcc/emit-rtl.c +++ b/gcc/emit-rtl.c @@ -6097,6 +6097,13 @@ init_emit_regs (void) if ((unsigned) PIC_OFFSET_TABLE_REGNUM != INVALID_REGNUM) pic_offset_table_rtx = gen_raw_REG (Pmode, PIC_OFFSET_TABLE_REGNUM); + /* Process stack-limiting command-line options. */ + if (opt_fstack_limit_symbol_arg != NULL) + stack_limit_rtx + = gen_rtx_SYMBOL_REF (Pmode, ggc_strdup (opt_fstack_limit_symbol_arg)); + if (opt_fstack_limit_register_no >= 0) + stack_limit_rtx = gen_rtx_REG (Pmode, opt_fstack_limit_register_no); + for (i = 0; i < (int) MAX_MACHINE_MODE; i++) { mode = (machine_mode) i; @@ -6177,13 +6184,6 @@ init_emit_once (void) /* Create the unique rtx's for certain rtx codes and operand values. */ - /* Process stack-limiting command-line options. */ - if (opt_fstack_limit_symbol_arg != NULL) - stack_limit_rtx - = gen_rtx_SYMBOL_REF (Pmode, ggc_strdup (opt_fstack_limit_symbol_arg)); - if (opt_fstack_limit_register_no >= 0) - stack_limit_rtx = gen_rtx_REG (Pmode, opt_fstack_limit_register_no); - /* Don't use gen_rtx_CONST_INT here since gen_rtx_CONST_INT in this case tries to use these variables. */ for (i = - MAX_SAVED_CONST_INT; i <= MAX_SAVED_CONST_INT; i++)