| Message ID | VI1PR0802MB2368DD4BD75B0CA78FE01C629B5E9@VI1PR0802MB2368.eurprd08.prod.outlook.com |
|---|---|
| State | New |
| Headers | show |
| Series | [GCC-10,backport] arm: _Generic feature failing with ICE for -O0 (pr97205). | expand |
Ping!! > -----Original Message----- > From: Srinath Parvathaneni <srinath.parvathaneni@arm.com> > Sent: 30 April 2021 16:24 > To: gcc-patches@gcc.gnu.org > Cc: Kyrylo Tkachov <Kyrylo.Tkachov@arm.com>; Richard Earnshaw > <Richard.Earnshaw@arm.com> > Subject: [GCC-10 backport][PATCH] arm: _Generic feature failing with ICE for > -O0 (pr97205). > > Hi, > > This is a backport to GCC-10 to fix PR97205, patch applies cleanly on the > branch. > > Regression tested and found no issues. > > Ok for GCC-10 backport? > > Regards, > Srinath. > > This makes sure that stack allocated SSA_NAMEs are > at least MODE_ALIGNED. Also increase the MEM_ALIGN > for the corresponding rtl objects. > > gcc: > 2020-11-03 Bernd Edlinger <bernd.edlinger@hotmail.de> > > PR target/97205 > * cfgexpand.c (align_local_variable): Make SSA_NAMEs > at least MODE_ALIGNED. > (expand_one_stack_var_at): Increase MEM_ALIGN for SSA_NAMEs. > > gcc/testsuite: > 2020-11-03 Bernd Edlinger <bernd.edlinger@hotmail.de> > > PR target/97205 > * gcc.c-torture/compile/pr97205.c: New test. > > (cherry picked from commit > 23ac7a009ecfeec3eab79136abed8aac9768b458) > > > ############### Attachment also inlined for ease of reply > ############### > > > diff --git a/gcc/cfgexpand.c b/gcc/cfgexpand.c index > bf4f194ed993134109cc21be9cb0ed8a5c170824..4fef5d6ebf420ce4d6f59606e > cd064f45ae59065 100644 > --- a/gcc/cfgexpand.c > +++ b/gcc/cfgexpand.c > @@ -366,7 +366,15 @@ align_local_variable (tree decl, bool really_expand) > unsigned int align; > > if (TREE_CODE (decl) == SSA_NAME) > - align = TYPE_ALIGN (TREE_TYPE (decl)); > + { > + tree type = TREE_TYPE (decl); > + machine_mode mode = TYPE_MODE (type); > + > + align = TYPE_ALIGN (type); > + if (mode != BLKmode > + && align < GET_MODE_ALIGNMENT (mode)) > + align = GET_MODE_ALIGNMENT (mode); > + } > else > { > align = LOCAL_DECL_ALIGNMENT (decl); @@ -999,20 +1007,21 @@ > expand_one_stack_var_at (tree decl, rtx base, unsigned base_align, > x = plus_constant (Pmode, base, offset); > x = gen_rtx_MEM (TREE_CODE (decl) == SSA_NAME > ? TYPE_MODE (TREE_TYPE (decl)) > - : DECL_MODE (SSAVAR (decl)), x); > + : DECL_MODE (decl), x); > + > + /* Set alignment we actually gave this decl if it isn't an SSA name. > + If it is we generate stack slots only accidentally so it isn't as > + important, we'll simply set the alignment directly on the MEM. */ > + > + if (base == virtual_stack_vars_rtx) > + offset -= frame_phase; > + align = known_alignment (offset); > + align *= BITS_PER_UNIT; > + if (align == 0 || align > base_align) > + align = base_align; > > if (TREE_CODE (decl) != SSA_NAME) > { > - /* Set alignment we actually gave this decl if it isn't an SSA name. > - If it is we generate stack slots only accidentally so it isn't as > - important, we'll simply use the alignment that is already set. */ > - if (base == virtual_stack_vars_rtx) > - offset -= frame_phase; > - align = known_alignment (offset); > - align *= BITS_PER_UNIT; > - if (align == 0 || align > base_align) > - align = base_align; > - > /* One would think that we could assert that we're not decreasing > alignment here, but (at least) the i386 port does exactly this > via the MINIMUM_ALIGNMENT hook. */ > @@ -1022,6 +1031,8 @@ expand_one_stack_var_at (tree decl, rtx base, > unsigned base_align, > } > > set_rtl (decl, x); > + > + set_mem_align (x, align); > } > > class stack_vars_data > @@ -1327,13 +1338,11 @@ expand_one_stack_var_1 (tree var) > { > tree type = TREE_TYPE (var); > size = tree_to_poly_uint64 (TYPE_SIZE_UNIT (type)); > - byte_align = TYPE_ALIGN_UNIT (type); > } > else > - { > - size = tree_to_poly_uint64 (DECL_SIZE_UNIT (var)); > - byte_align = align_local_variable (var, true); > - } > + size = tree_to_poly_uint64 (DECL_SIZE_UNIT (var)); > + > + byte_align = align_local_variable (var, true); > > /* We handle highly aligned variables in expand_stack_vars. */ > gcc_assert (byte_align * BITS_PER_UNIT <= > MAX_SUPPORTED_STACK_ALIGNMENT); diff --git a/gcc/testsuite/gcc.c- > torture/compile/pr97205.c b/gcc/testsuite/gcc.c-torture/compile/pr97205.c > new file mode 100644 > index > 0000000000000000000000000000000000000000..6600011fcf84660edcba8d9 > 68c78ee6aaa0aa923 > --- /dev/null > +++ b/gcc/testsuite/gcc.c-torture/compile/pr97205.c > @@ -0,0 +1,7 @@ > +int a; > +typedef __attribute__((aligned(2))) int x; int f () { > + x b = a; > + return b; > +}
Looking through the bugzilla logs shows: Since it is a gcc_checking_assert that triggers the ICE, I assumed that does not affect a release build, is that correct? So it would appear that the decision was taken that a backport was not needed. Have I missed something? R. On 19/05/2021 13:22, Srinath Parvathaneni via Gcc-patches wrote: > Ping!! > >> -----Original Message----- >> From: Srinath Parvathaneni <srinath.parvathaneni@arm.com> >> Sent: 30 April 2021 16:24 >> To: gcc-patches@gcc.gnu.org >> Cc: Kyrylo Tkachov <Kyrylo.Tkachov@arm.com>; Richard Earnshaw >> <Richard.Earnshaw@arm.com> >> Subject: [GCC-10 backport][PATCH] arm: _Generic feature failing with ICE for >> -O0 (pr97205). >> >> Hi, >> >> This is a backport to GCC-10 to fix PR97205, patch applies cleanly on the >> branch. >> >> Regression tested and found no issues. >> >> Ok for GCC-10 backport? >> >> Regards, >> Srinath. >> >> This makes sure that stack allocated SSA_NAMEs are >> at least MODE_ALIGNED. Also increase the MEM_ALIGN >> for the corresponding rtl objects. >> >> gcc: >> 2020-11-03 Bernd Edlinger <bernd.edlinger@hotmail.de> >> >> PR target/97205 >> * cfgexpand.c (align_local_variable): Make SSA_NAMEs >> at least MODE_ALIGNED. >> (expand_one_stack_var_at): Increase MEM_ALIGN for SSA_NAMEs. >> >> gcc/testsuite: >> 2020-11-03 Bernd Edlinger <bernd.edlinger@hotmail.de> >> >> PR target/97205 >> * gcc.c-torture/compile/pr97205.c: New test. >> >> (cherry picked from commit >> 23ac7a009ecfeec3eab79136abed8aac9768b458) >> >> >> ############### Attachment also inlined for ease of reply >> ############### >> >> >> diff --git a/gcc/cfgexpand.c b/gcc/cfgexpand.c index >> bf4f194ed993134109cc21be9cb0ed8a5c170824..4fef5d6ebf420ce4d6f59606e >> cd064f45ae59065 100644 >> --- a/gcc/cfgexpand.c >> +++ b/gcc/cfgexpand.c >> @@ -366,7 +366,15 @@ align_local_variable (tree decl, bool really_expand) >> unsigned int align; >> >> if (TREE_CODE (decl) == SSA_NAME) >> - align = TYPE_ALIGN (TREE_TYPE (decl)); >> + { >> + tree type = TREE_TYPE (decl); >> + machine_mode mode = TYPE_MODE (type); >> + >> + align = TYPE_ALIGN (type); >> + if (mode != BLKmode >> + && align < GET_MODE_ALIGNMENT (mode)) >> + align = GET_MODE_ALIGNMENT (mode); >> + } >> else >> { >> align = LOCAL_DECL_ALIGNMENT (decl); @@ -999,20 +1007,21 @@ >> expand_one_stack_var_at (tree decl, rtx base, unsigned base_align, >> x = plus_constant (Pmode, base, offset); >> x = gen_rtx_MEM (TREE_CODE (decl) == SSA_NAME >> ? TYPE_MODE (TREE_TYPE (decl)) >> - : DECL_MODE (SSAVAR (decl)), x); >> + : DECL_MODE (decl), x); >> + >> + /* Set alignment we actually gave this decl if it isn't an SSA name. >> + If it is we generate stack slots only accidentally so it isn't as >> + important, we'll simply set the alignment directly on the MEM. */ >> + >> + if (base == virtual_stack_vars_rtx) >> + offset -= frame_phase; >> + align = known_alignment (offset); >> + align *= BITS_PER_UNIT; >> + if (align == 0 || align > base_align) >> + align = base_align; >> >> if (TREE_CODE (decl) != SSA_NAME) >> { >> - /* Set alignment we actually gave this decl if it isn't an SSA name. >> - If it is we generate stack slots only accidentally so it isn't as >> - important, we'll simply use the alignment that is already set. */ >> - if (base == virtual_stack_vars_rtx) >> - offset -= frame_phase; >> - align = known_alignment (offset); >> - align *= BITS_PER_UNIT; >> - if (align == 0 || align > base_align) >> - align = base_align; >> - >> /* One would think that we could assert that we're not decreasing >> alignment here, but (at least) the i386 port does exactly this >> via the MINIMUM_ALIGNMENT hook. */ >> @@ -1022,6 +1031,8 @@ expand_one_stack_var_at (tree decl, rtx base, >> unsigned base_align, >> } >> >> set_rtl (decl, x); >> + >> + set_mem_align (x, align); >> } >> >> class stack_vars_data >> @@ -1327,13 +1338,11 @@ expand_one_stack_var_1 (tree var) >> { >> tree type = TREE_TYPE (var); >> size = tree_to_poly_uint64 (TYPE_SIZE_UNIT (type)); >> - byte_align = TYPE_ALIGN_UNIT (type); >> } >> else >> - { >> - size = tree_to_poly_uint64 (DECL_SIZE_UNIT (var)); >> - byte_align = align_local_variable (var, true); >> - } >> + size = tree_to_poly_uint64 (DECL_SIZE_UNIT (var)); >> + >> + byte_align = align_local_variable (var, true); >> >> /* We handle highly aligned variables in expand_stack_vars. */ >> gcc_assert (byte_align * BITS_PER_UNIT <= >> MAX_SUPPORTED_STACK_ALIGNMENT); diff --git a/gcc/testsuite/gcc.c- >> torture/compile/pr97205.c b/gcc/testsuite/gcc.c-torture/compile/pr97205.c >> new file mode 100644 >> index >> 0000000000000000000000000000000000000000..6600011fcf84660edcba8d9 >> 68c78ee6aaa0aa923 >> --- /dev/null >> +++ b/gcc/testsuite/gcc.c-torture/compile/pr97205.c >> @@ -0,0 +1,7 @@ >> +int a; >> +typedef __attribute__((aligned(2))) int x; int f () { >> + x b = a; >> + return b; >> +} >
diff --git a/gcc/cfgexpand.c b/gcc/cfgexpand.c index bf4f194ed993134109cc21be9cb0ed8a5c170824..4fef5d6ebf420ce4d6f59606ecd064f45ae59065 100644 --- a/gcc/cfgexpand.c +++ b/gcc/cfgexpand.c @@ -366,7 +366,15 @@ align_local_variable (tree decl, bool really_expand) unsigned int align; if (TREE_CODE (decl) == SSA_NAME) - align = TYPE_ALIGN (TREE_TYPE (decl)); + { + tree type = TREE_TYPE (decl); + machine_mode mode = TYPE_MODE (type); + + align = TYPE_ALIGN (type); + if (mode != BLKmode + && align < GET_MODE_ALIGNMENT (mode)) + align = GET_MODE_ALIGNMENT (mode); + } else { align = LOCAL_DECL_ALIGNMENT (decl); @@ -999,20 +1007,21 @@ expand_one_stack_var_at (tree decl, rtx base, unsigned base_align, x = plus_constant (Pmode, base, offset); x = gen_rtx_MEM (TREE_CODE (decl) == SSA_NAME ? TYPE_MODE (TREE_TYPE (decl)) - : DECL_MODE (SSAVAR (decl)), x); + : DECL_MODE (decl), x); + + /* Set alignment we actually gave this decl if it isn't an SSA name. + If it is we generate stack slots only accidentally so it isn't as + important, we'll simply set the alignment directly on the MEM. */ + + if (base == virtual_stack_vars_rtx) + offset -= frame_phase; + align = known_alignment (offset); + align *= BITS_PER_UNIT; + if (align == 0 || align > base_align) + align = base_align; if (TREE_CODE (decl) != SSA_NAME) { - /* Set alignment we actually gave this decl if it isn't an SSA name. - If it is we generate stack slots only accidentally so it isn't as - important, we'll simply use the alignment that is already set. */ - if (base == virtual_stack_vars_rtx) - offset -= frame_phase; - align = known_alignment (offset); - align *= BITS_PER_UNIT; - if (align == 0 || align > base_align) - align = base_align; - /* One would think that we could assert that we're not decreasing alignment here, but (at least) the i386 port does exactly this via the MINIMUM_ALIGNMENT hook. */ @@ -1022,6 +1031,8 @@ expand_one_stack_var_at (tree decl, rtx base, unsigned base_align, } set_rtl (decl, x); + + set_mem_align (x, align); } class stack_vars_data @@ -1327,13 +1338,11 @@ expand_one_stack_var_1 (tree var) { tree type = TREE_TYPE (var); size = tree_to_poly_uint64 (TYPE_SIZE_UNIT (type)); - byte_align = TYPE_ALIGN_UNIT (type); } else - { - size = tree_to_poly_uint64 (DECL_SIZE_UNIT (var)); - byte_align = align_local_variable (var, true); - } + size = tree_to_poly_uint64 (DECL_SIZE_UNIT (var)); + + byte_align = align_local_variable (var, true); /* We handle highly aligned variables in expand_stack_vars. */ gcc_assert (byte_align * BITS_PER_UNIT <= MAX_SUPPORTED_STACK_ALIGNMENT); diff --git a/gcc/testsuite/gcc.c-torture/compile/pr97205.c b/gcc/testsuite/gcc.c-torture/compile/pr97205.c new file mode 100644 index 0000000000000000000000000000000000000000..6600011fcf84660edcba8d968c78ee6aaa0aa923 --- /dev/null +++ b/gcc/testsuite/gcc.c-torture/compile/pr97205.c @@ -0,0 +1,7 @@ +int a; +typedef __attribute__((aligned(2))) int x; +int f () +{ + x b = a; + return b; +}
Hi, This is a backport to GCC-10 to fix PR97205, patch applies cleanly on the branch. Regression tested and found no issues. Ok for GCC-10 backport? Regards, Srinath. This makes sure that stack allocated SSA_NAMEs are at least MODE_ALIGNED. Also increase the MEM_ALIGN for the corresponding rtl objects. gcc: 2020-11-03 Bernd Edlinger <bernd.edlinger@hotmail.de> PR target/97205 * cfgexpand.c (align_local_variable): Make SSA_NAMEs at least MODE_ALIGNED. (expand_one_stack_var_at): Increase MEM_ALIGN for SSA_NAMEs. gcc/testsuite: 2020-11-03 Bernd Edlinger <bernd.edlinger@hotmail.de> PR target/97205 * gcc.c-torture/compile/pr97205.c: New test. (cherry picked from commit 23ac7a009ecfeec3eab79136abed8aac9768b458) ############### Attachment also inlined for ease of reply ############### diff --git a/gcc/cfgexpand.c b/gcc/cfgexpand.c index bf4f194ed993134109cc21be9cb0ed8a5c170824..4fef5d6ebf420ce4d6f59606ecd064f45ae59065 100644 --- a/gcc/cfgexpand.c +++ b/gcc/cfgexpand.c @@ -366,7 +366,15 @@ align_local_variable (tree decl, bool really_expand) unsigned int align; if (TREE_CODE (decl) == SSA_NAME) - align = TYPE_ALIGN (TREE_TYPE (decl)); + { + tree type = TREE_TYPE (decl); + machine_mode mode = TYPE_MODE (type); + + align = TYPE_ALIGN (type); + if (mode != BLKmode + && align < GET_MODE_ALIGNMENT (mode)) + align = GET_MODE_ALIGNMENT (mode); + } else { align = LOCAL_DECL_ALIGNMENT (decl); @@ -999,20 +1007,21 @@ expand_one_stack_var_at (tree decl, rtx base, unsigned base_align, x = plus_constant (Pmode, base, offset); x = gen_rtx_MEM (TREE_CODE (decl) == SSA_NAME ? TYPE_MODE (TREE_TYPE (decl)) - : DECL_MODE (SSAVAR (decl)), x); + : DECL_MODE (decl), x); + + /* Set alignment we actually gave this decl if it isn't an SSA name. + If it is we generate stack slots only accidentally so it isn't as + important, we'll simply set the alignment directly on the MEM. */ + + if (base == virtual_stack_vars_rtx) + offset -= frame_phase; + align = known_alignment (offset); + align *= BITS_PER_UNIT; + if (align == 0 || align > base_align) + align = base_align; if (TREE_CODE (decl) != SSA_NAME) { - /* Set alignment we actually gave this decl if it isn't an SSA name. - If it is we generate stack slots only accidentally so it isn't as - important, we'll simply use the alignment that is already set. */ - if (base == virtual_stack_vars_rtx) - offset -= frame_phase; - align = known_alignment (offset); - align *= BITS_PER_UNIT; - if (align == 0 || align > base_align) - align = base_align; - /* One would think that we could assert that we're not decreasing alignment here, but (at least) the i386 port does exactly this via the MINIMUM_ALIGNMENT hook. */ @@ -1022,6 +1031,8 @@ expand_one_stack_var_at (tree decl, rtx base, unsigned base_align, } set_rtl (decl, x); + + set_mem_align (x, align); } class stack_vars_data @@ -1327,13 +1338,11 @@ expand_one_stack_var_1 (tree var) { tree type = TREE_TYPE (var); size = tree_to_poly_uint64 (TYPE_SIZE_UNIT (type)); - byte_align = TYPE_ALIGN_UNIT (type); } else - { - size = tree_to_poly_uint64 (DECL_SIZE_UNIT (var)); - byte_align = align_local_variable (var, true); - } + size = tree_to_poly_uint64 (DECL_SIZE_UNIT (var)); + + byte_align = align_local_variable (var, true); /* We handle highly aligned variables in expand_stack_vars. */ gcc_assert (byte_align * BITS_PER_UNIT <= MAX_SUPPORTED_STACK_ALIGNMENT); diff --git a/gcc/testsuite/gcc.c-torture/compile/pr97205.c b/gcc/testsuite/gcc.c-torture/compile/pr97205.c new file mode 100644 index 0000000000000000000000000000000000000000..6600011fcf84660edcba8d968c78ee6aaa0aa923 --- /dev/null +++ b/gcc/testsuite/gcc.c-torture/compile/pr97205.c @@ -0,0 +1,7 @@ +int a; +typedef __attribute__((aligned(2))) int x; +int f () +{ + x b = a; + return b; +}