* [GCC-10 backport][PATCH] arm: _Generic feature failing with ICE for -O0 (pr97205).
@ 2021-04-30 15:18 Srinath Parvathaneni
2021-05-19 12:22 ` Srinath Parvathaneni
0 siblings, 1 reply; 3+ messages in thread
From: Srinath Parvathaneni @ 2021-04-30 15:18 UTC (permalink / raw)
To: gcc-patches; +Cc: Kyrylo.Tkachov, Richard.Earnshaw
[-- Attachment #1: Type: text/plain, Size: 4092 bytes --]
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;
+}
[-- Attachment #2: rb14389.patch --]
[-- Type: text/plain, Size: 3197 bytes --]
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;
+}
^ permalink raw reply [flat|nested] 3+ messages in thread
* RE: [GCC-10 backport][PATCH] arm: _Generic feature failing with ICE for -O0 (pr97205).
2021-04-30 15:18 [GCC-10 backport][PATCH] arm: _Generic feature failing with ICE for -O0 (pr97205) Srinath Parvathaneni
@ 2021-05-19 12:22 ` Srinath Parvathaneni
2021-05-19 14:44 ` Richard Earnshaw
0 siblings, 1 reply; 3+ messages in thread
From: Srinath Parvathaneni @ 2021-05-19 12:22 UTC (permalink / raw)
To: Srinath Parvathaneni, gcc-patches; +Cc: Kyrylo Tkachov, Richard Earnshaw
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;
> +}
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [GCC-10 backport][PATCH] arm: _Generic feature failing with ICE for -O0 (pr97205).
2021-05-19 12:22 ` Srinath Parvathaneni
@ 2021-05-19 14:44 ` Richard Earnshaw
0 siblings, 0 replies; 3+ messages in thread
From: Richard Earnshaw @ 2021-05-19 14:44 UTC (permalink / raw)
To: Srinath Parvathaneni, gcc-patches; +Cc: Richard Earnshaw
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;
>> +}
>
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2021-05-19 14:44 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-04-30 15:18 [GCC-10 backport][PATCH] arm: _Generic feature failing with ICE for -O0 (pr97205) Srinath Parvathaneni
2021-05-19 12:22 ` Srinath Parvathaneni
2021-05-19 14:44 ` Richard Earnshaw
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).