| Message ID | 428eb6a9-a647-b46c-6b8f-c7456773dacd@mittosystems.com |
|---|---|
| State | New |
| Headers | show |
| Series | Fix PR middle-end/86705 | expand |
On Sun, Jul 29, 2018 at 6:27 PM Jozef Lawrynowicz <jozef.l@mittosystems.com> wrote: > > pr45678-2.c ICEs for msp430-elf with -mlarge, because an alignment of > POINTER_SIZE is attempted. POINTER_SIZE with -mlarge is 20-bits, so further > code in the middle-end that expects this to be a power or 2 causes odd > alignments to be set, in this case eventually resulting in an ICE. > > The test ICEs on gcc-7-branch, gcc-8-branch, and current trunk. It > successfully builds on gcc-6-branch. > The failure is caused by r235172. > > Successfully bootstrapped and regtested the attached patch for > x86-64-pc-linux-gnu, and msp430-elf with -mlarge, on trunk. > > Ok for gcc-7-branch, gcc-8-branch and trunk? I wonder if most (if not all) places you touch want to use get_mode_alignment (Pmode) instead? (or ptr_mode) Anyhow, the patch is otherwise obvious though factoring the thing might be nice (thus my suggestion above...) Richard.
On 30/07/18 14:29, Richard Biener wrote: > On Sun, Jul 29, 2018 at 6:27 PM Jozef Lawrynowicz > <jozef.l@mittosystems.com> wrote: >> pr45678-2.c ICEs for msp430-elf with -mlarge, because an alignment of >> POINTER_SIZE is attempted. POINTER_SIZE with -mlarge is 20-bits, so further >> code in the middle-end that expects this to be a power or 2 causes odd >> alignments to be set, in this case eventually resulting in an ICE. >> >> The test ICEs on gcc-7-branch, gcc-8-branch, and current trunk. It >> successfully builds on gcc-6-branch. >> The failure is caused by r235172. >> >> Successfully bootstrapped and regtested the attached patch for >> x86-64-pc-linux-gnu, and msp430-elf with -mlarge, on trunk. >> >> Ok for gcc-7-branch, gcc-8-branch and trunk? > I wonder if most (if not all) places you touch want to use > get_mode_alignment (Pmode) instead? (or ptr_mode) > > Anyhow, the patch is otherwise obvious though factoring > the thing might be nice (thus my suggestion above...) > > Richard. Thanks for the suggestion, using GET_MODE_ALIGNMENT does seem like a neater idea. After retesting, I went ahead and committed the below patch onto trunk, will backport to gcc-7/8-branch later. diff --git a/gcc/cfgexpand.c b/gcc/cfgexpand.c index d6e3c38..7353d5d 100644 --- a/gcc/cfgexpand.c +++ b/gcc/cfgexpand.c @@ -1257,10 +1257,10 @@ set_parm_rtl (tree parm, rtx x) allocate it, which means that in-frame portion is just a pointer. ??? We've got a pseudo for sure here, do we actually dynamically allocate its spilling area if needed? - ??? Isn't it a problem when POINTER_SIZE also exceeds - MAX_SUPPORTED_STACK_ALIGNMENT, as on cris and lm32? */ + ??? Isn't it a problem when Pmode alignment also exceeds + MAX_SUPPORTED_STACK_ALIGNMENT, as can happen on cris and lm32? */ if (align > MAX_SUPPORTED_STACK_ALIGNMENT) - align = POINTER_SIZE; + align = GET_MODE_ALIGNMENT (Pmode); record_alignment_for_reg_var (align); } @@ -1381,7 +1381,7 @@ expand_one_ssa_partition (tree var) /* If the variable alignment is very large we'll dynamicaly allocate it, which means that in-frame portion is just a pointer. */ if (align > MAX_SUPPORTED_STACK_ALIGNMENT) - align = POINTER_SIZE; + align = GET_MODE_ALIGNMENT (Pmode); record_alignment_for_reg_var (align); @@ -1608,7 +1608,7 @@ expand_one_var (tree var, bool toplevel, bool really_expand) /* If the variable alignment is very large we'll dynamicaly allocate it, which means that in-frame portion is just a pointer. */ if (align > MAX_SUPPORTED_STACK_ALIGNMENT) - align = POINTER_SIZE; + align = GET_MODE_ALIGNMENT (Pmode); } record_alignment_for_reg_var (align);
From e655a518a06a848dc398504f28272750e1a2be9f Mon Sep 17 00:00:00 2001 From: Jozef Lawrynowicz <jozefl.opensrc@gmail.com> Date: Tue, 13 Mar 2018 11:25:56 +0000 Subject: [PATCH] When aligning to POINTER_SIZE, round alignment to largest power of 2 that is <= POINTER_SIZE PR middle-end/86705 * gcc/cfgexpand.c (set_parm_rtl): Before using POINTER_SIZE as an alignment value, round it to the largest power of 2 less than or equal to itself. (expand_one_ssa_partition): Likewise. (expand_one_var): Likewise. --- gcc/cfgexpand.c | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/gcc/cfgexpand.c b/gcc/cfgexpand.c index d6e3c38..a56db7a 100644 --- a/gcc/cfgexpand.c +++ b/gcc/cfgexpand.c @@ -1258,9 +1258,12 @@ set_parm_rtl (tree parm, rtx x) pointer. ??? We've got a pseudo for sure here, do we actually dynamically allocate its spilling area if needed? ??? Isn't it a problem when POINTER_SIZE also exceeds - MAX_SUPPORTED_STACK_ALIGNMENT, as on cris and lm32? */ + MAX_SUPPORTED_STACK_ALIGNMENT, as on cris and lm32? + POINTER_SIZE may not be a power of 2 e.g. for msp430-elf with the large + data model, so align to the largest power of 2 that is + <= POINTER_SIZE. */ if (align > MAX_SUPPORTED_STACK_ALIGNMENT) - align = POINTER_SIZE; + align = (unsigned)1 << floor_log2 (POINTER_SIZE); record_alignment_for_reg_var (align); } @@ -1381,7 +1384,7 @@ expand_one_ssa_partition (tree var) /* If the variable alignment is very large we'll dynamicaly allocate it, which means that in-frame portion is just a pointer. */ if (align > MAX_SUPPORTED_STACK_ALIGNMENT) - align = POINTER_SIZE; + align = (unsigned)1 << floor_log2 (POINTER_SIZE); record_alignment_for_reg_var (align); @@ -1608,7 +1611,7 @@ expand_one_var (tree var, bool toplevel, bool really_expand) /* If the variable alignment is very large we'll dynamicaly allocate it, which means that in-frame portion is just a pointer. */ if (align > MAX_SUPPORTED_STACK_ALIGNMENT) - align = POINTER_SIZE; + align = (unsigned)1 << floor_log2 (POINTER_SIZE); } record_alignment_for_reg_var (align); -- 2.7.4