[Bug c/97205] New: arm: Compiler fails with an ICE for -O0 on Trunk and GCC-10 for _Generic feature.

[Bug c/97205] New: arm: Compiler fails with an ICE for -O0 on Trunk and GCC-10 for _Generic feature.

[sripar01 at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97205

            Bug ID: 97205
           Summary: arm: Compiler fails with an ICE for -O0 on Trunk and
                    GCC-10 for _Generic feature.
           Product: gcc
           Version: 11.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: c
          Assignee: unassigned at gcc dot gnu.org
          Reporter: sripar01 at gcc dot gnu.org
  Target Milestone: ---
            Target: arm

Created attachment 49270
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=49270&action=edit
testcase

Nested _Generic feature generating ICE with -O0 on GCC Trunk and GCC-10.

$arm-none-eabi-gcc -v
Using built-in specs.
COLLECT_GCC=/arm/pdtl/builds/fsf-trunk.2226/installed/rhe6x86_64/arm-none-eabi/bin/arm-none-eabi-gcc
COLLECT_LTO_WRAPPER=/arm/pdtl/builds/fsf-trunk.2226/installed/rhe6x86_64/arm-none-eabi/bin/../libexec/gcc/arm-none-eabi/11.0.0/lto-wrapper
Target: arm-none-eabi
Configured with:
/tmp/dgboter/bbs/rhev-vm5--rhe6x86_64/buildbot/rhe6x86_64--arm-none-eabi/build/src/gcc/configure
--target=arm-none-eabi
--prefix=/tmp/dgboter/bbs/rhev-vm5--rhe6x86_64/buildbot/rhe6x86_64--arm-none-eabi/build/build-arm-none-eabi/install//
--with-gmp=/tmp/dgboter/bbs/rhev-vm5--rhe6x86_64/buildbot/rhe6x86_64--arm-none-eabi/build/build-arm-none-eabi/host-tools
--with-mpfr=/tmp/dgboter/bbs/rhev-vm5--rhe6x86_64/buildbot/rhe6x86_64--arm-none-eabi/build/build-arm-none-eabi/host-tools
--with-mpc=/tmp/dgboter/bbs/rhev-vm5--rhe6x86_64/buildbot/rhe6x86_64--arm-none-eabi/build/build-arm-none-eabi/host-tools
--with-isl=/tmp/dgboter/bbs/rhev-vm5--rhe6x86_64/buildbot/rhe6x86_64--arm-none-eabi/build/build-arm-none-eabi/host-tools
--disable-shared --disable-nls --disable-threads --disable-tls
--enable-checking=yes --enable-languages=c,c++,fortran --with-newlib
--with-multilib-list=aprofile --with-pkgversion=fsf-trunk.2226
Thread model: single
Supported LTO compression algorithms: zlib
gcc version 11.0.0 20200609 (experimental) (fsf-trunk.2226)

$ cat x.c
int a;
typedef __attribute__((aligned(2))) int x;
int fn1() {
  x b = ({
    _Generic(_Generic(fn1, default : a), int : _Generic(fn1, default : a));
  });
 return b;
}

$ arm-none-eabi-gcc  x.c -O0 -c -Wall -Werror
during RTL pass: expand
x.c: In function 'fn1':
x.c:4:5: internal compiler error: in gen_movsi, at config/arm/arm.md:6364
    4 |   x b = ({
      |     ^
0x1516e1d gen_movsi(rtx_def*, rtx_def*)
       
/tmp/dgboter/bbs/rhev-vm5--rhe6x86_64/buildbot/rhe6x86_64--arm-none-eabi/build/src/gcc/gcc/config/arm/arm.md:6364
0x925ddf insn_gen_fn::operator()(rtx_def*, rtx_def*) const
       
/tmp/dgboter/bbs/rhev-vm5--rhe6x86_64/buildbot/rhe6x86_64--arm-none-eabi/build/src/gcc/gcc/recog.h:317
0x925ddf emit_move_insn_1(rtx_def*, rtx_def*)
       
/tmp/dgboter/bbs/rhev-vm5--rhe6x86_64/buildbot/rhe6x86_64--arm-none-eabi/build/src/gcc/gcc/expr.c:3759
0x926872 emit_move_insn(rtx_def*, rtx_def*)
       
/tmp/dgboter/bbs/rhev-vm5--rhe6x86_64/buildbot/rhe6x86_64--arm-none-eabi/build/src/gcc/gcc/expr.c:3929
0x92ef96 store_expr(tree_node*, rtx_def*, int, bool, bool)
       
/tmp/dgboter/bbs/rhev-vm5--rhe6x86_64/buildbot/rhe6x86_64--arm-none-eabi/build/src/gcc/gcc/expr.c:6038
0x93172e expand_assignment(tree_node*, tree_node*, bool)
       
/tmp/dgboter/bbs/rhev-vm5--rhe6x86_64/buildbot/rhe6x86_64--arm-none-eabi/build/src/gcc/gcc/expr.c:5588
0x7dd4bc expand_gimple_stmt_1
       
/tmp/dgboter/bbs/rhev-vm5--rhe6x86_64/buildbot/rhe6x86_64--arm-none-eabi/build/src/gcc/gcc/cfgexpand.c:3751
0x7dd4bc expand_gimple_stmt
       
/tmp/dgboter/bbs/rhev-vm5--rhe6x86_64/buildbot/rhe6x86_64--arm-none-eabi/build/src/gcc/gcc/cfgexpand.c:3847
0x7dfd75 expand_gimple_basic_block
       
/tmp/dgboter/bbs/rhev-vm5--rhe6x86_64/buildbot/rhe6x86_64--arm-none-eabi/build/src/gcc/gcc/cfgexpand.c:5888
0x7e0c7e execute
       
/tmp/dgboter/bbs/rhev-vm5--rhe6x86_64/buildbot/rhe6x86_64--arm-none-eabi/build/src/gcc/gcc/cfgexpand.c:6572
Please submit a full bug report,
with preprocessed source if appropriate.
Please include the complete backtrace with any bug report.
See <https://gcc.gnu.org/bugs/> for instructions.

[Bug target/97205] arm: Compiler fails with an ICE for -O0 on Trunk and GCC-10 for _Generic feature.

[sripar01 at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97205

SRINATH PARVATHANENI <sripar01 at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |bernd.edlinger at hotmail dot de,
                   |                            |rguenth at gcc dot gnu.org

--- Comment #1 from SRINATH PARVATHANENI <sripar01 at gcc dot gnu.org> ---
On bisecting I found the following patch is causing the ICE.

70cdb21e579191fe9f0f1d45e328908e59c0179e is the first bad commit
commit 70cdb21e579191fe9f0f1d45e328908e59c0179e
Author: Bernd Edlinger <bernd.edlinger@hotmail.de>
Date:   Wed Aug 28 10:18:23 2019 +0000

    expr.c (expand_assignment): Handle misaligned DECLs.

    2019-09-28  Bernd Edlinger  <bernd.edlinger@hotmail.de>
            Richard Biener  <rguenther@suse.de>

        * expr.c (expand_assignment): Handle misaligned DECLs.
        (expand_expr_real_1): Handle FUNCTION_DECL as unaligned.
        * function.c (assign_parm_adjust_stack_rtl): Check movmisalign optab
        too.
        (assign_parm_setup_stack): Allocate properly aligned stack slots.
        * varasm.c (build_constant_desc): Align constants of misaligned types.
        * config/arm/predicates.md (aligned_operand): New predicate.
        * config/arm/arm.md (movdi, movsi, movhi, movhf, movsf, movdf): Use
        aligned_operand to check restrictions on memory addresses.
        * config/arm/neon.md (movti, mov<VSTRUCT>, mov<VH>): Likewise.
        * config/arm/vec-common.md (mov<VALL>): Likewise.

    Co-Authored-By: Richard Biener <rguenther@suse.de>

[Bug target/97205] arm: Compiler fails with an ICE for -O0 on Trunk and GCC-10 for _Generic feature.

[bernd.edlinger at hotmail dot de]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97205

--- Comment #2 from Bernd Edlinger <bernd.edlinger at hotmail dot de> ---
Thanks for reporting this.

The expansion of assignments to misaligned ssa names
does not handle the case of misaligned stores, which
would result in incorrect code without the assertion.

I have an untested patch below:

diff --git a/gcc/emit-rtl.c b/gcc/emit-rtl.c
index 3706f0a..12b81cd 100644
--- a/gcc/emit-rtl.c
+++ b/gcc/emit-rtl.c
@@ -2089,7 +2089,8 @@ set_mem_attributes_minus_bitpos (rtx ref, tree t, int
objectp,
        {
          gcc_assert (handled_component_p (t)
                      || TREE_CODE (t) == MEM_REF
-                     || TREE_CODE (t) == TARGET_MEM_REF);
+                     || TREE_CODE (t) == TARGET_MEM_REF
+                     || TREE_CODE (t) == SSA_NAME);
          attrs.expr = t;
          attrs.offset_known_p = true;
          attrs.offset = 0;
diff --git a/gcc/expr.c b/gcc/expr.c
index 9d951e8..49f2699 100644
--- a/gcc/expr.c
+++ b/gcc/expr.c
@@ -5200,6 +5200,9 @@ expand_assignment (tree to, tree from, bool nontemporal)
       || (TREE_CODE (to) == MEM_REF
          && (REF_REVERSE_STORAGE_ORDER (to)
              || mem_ref_refers_to_non_mem_p (to)))
+      || (TREE_CODE (to) == SSA_NAME
+         && mode != BLKmode
+         && TYPE_ALIGN (TREE_TYPE (to)) < GET_MODE_ALIGNMENT (mode))
       || TREE_CODE (TREE_TYPE (to)) == ARRAY_TYPE)
     {
       machine_mode mode1;
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 0000000..6600011
--- /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;
+}

[Bug target/97205] arm: Compiler fails with an ICE for -O0 on Trunk and GCC-10 for _Generic feature.

[rguenther at suse dot de]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97205

--- Comment #3 from rguenther at suse dot de <rguenther at suse dot de> ---
On Tue, 27 Oct 2020, bernd.edlinger at hotmail dot de wrote:

> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97205
> 
> --- Comment #2 from Bernd Edlinger <bernd.edlinger at hotmail dot de> ---
> Thanks for reporting this.
> 
> The expansion of assignments to misaligned ssa names
> does not handle the case of misaligned stores, which
> would result in incorrect code without the assertion.
> 
> I have an untested patch below:
> 
> diff --git a/gcc/emit-rtl.c b/gcc/emit-rtl.c
> index 3706f0a..12b81cd 100644
> --- a/gcc/emit-rtl.c
> +++ b/gcc/emit-rtl.c
> @@ -2089,7 +2089,8 @@ set_mem_attributes_minus_bitpos (rtx ref, tree t, int
> objectp,
>         {
>           gcc_assert (handled_component_p (t)
>                       || TREE_CODE (t) == MEM_REF
> -                     || TREE_CODE (t) == TARGET_MEM_REF);
> +                     || TREE_CODE (t) == TARGET_MEM_REF
> +                     || TREE_CODE (t) == SSA_NAME);

Can you track down this?  It's a red herring to have a MEM_EXPR
that just is a SSA_NAME.

>           attrs.expr = t;
>           attrs.offset_known_p = true;
>           attrs.offset = 0;
> diff --git a/gcc/expr.c b/gcc/expr.c
> index 9d951e8..49f2699 100644
> --- a/gcc/expr.c
> +++ b/gcc/expr.c
> @@ -5200,6 +5200,9 @@ expand_assignment (tree to, tree from, bool nontemporal)
>        || (TREE_CODE (to) == MEM_REF
>           && (REF_REVERSE_STORAGE_ORDER (to)
>               || mem_ref_refers_to_non_mem_p (to)))
> +      || (TREE_CODE (to) == SSA_NAME
> +         && mode != BLKmode
> +         && TYPE_ALIGN (TREE_TYPE (to)) < GET_MODE_ALIGNMENT (mode))

But an SSA name is a register, esp. one with non-BLKmode.  And if
expanded to a stack location that stack location should better
be aligned...  or be BLKmode.  So I think the bug is elsewhere?

>        || TREE_CODE (TREE_TYPE (to)) == ARRAY_TYPE)
>      {
>        machine_mode mode1;
> 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 0000000..6600011
> --- /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;
> +}
> 
>

[Bug target/97205] arm: Compiler fails with an ICE for -O0 on Trunk and GCC-10 for _Generic feature.

[sripar01 at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97205

--- Comment #4 from SRINATH PARVATHANENI <sripar01 at gcc dot gnu.org> ---
(In reply to Bernd Edlinger from comment #2)
> Thanks for reporting this.
> 
> The expansion of assignments to misaligned ssa names
> does not handle the case of misaligned stores, which
> would result in incorrect code without the assertion.
> 
> I have an untested patch below:
> 
> diff --git a/gcc/emit-rtl.c b/gcc/emit-rtl.c
> index 3706f0a..12b81cd 100644
> --- a/gcc/emit-rtl.c
> +++ b/gcc/emit-rtl.c
> @@ -2089,7 +2089,8 @@ set_mem_attributes_minus_bitpos (rtx ref, tree t, int
> objectp,
>         {
>           gcc_assert (handled_component_p (t)
>                       || TREE_CODE (t) == MEM_REF
> -                     || TREE_CODE (t) == TARGET_MEM_REF);
> +                     || TREE_CODE (t) == TARGET_MEM_REF
> +                     || TREE_CODE (t) == SSA_NAME);
>           attrs.expr = t;
>           attrs.offset_known_p = true;
>           attrs.offset = 0;
> diff --git a/gcc/expr.c b/gcc/expr.c
> index 9d951e8..49f2699 100644
> --- a/gcc/expr.c
> +++ b/gcc/expr.c
> @@ -5200,6 +5200,9 @@ expand_assignment (tree to, tree from, bool
> nontemporal)
>        || (TREE_CODE (to) == MEM_REF
>           && (REF_REVERSE_STORAGE_ORDER (to)
>               || mem_ref_refers_to_non_mem_p (to)))
> +      || (TREE_CODE (to) == SSA_NAME
> +         && mode != BLKmode
> +         && TYPE_ALIGN (TREE_TYPE (to)) < GET_MODE_ALIGNMENT (mode))
>        || TREE_CODE (TREE_TYPE (to)) == ARRAY_TYPE)
>      {
>        machine_mode mode1;
> 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 0000000..6600011
> --- /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;
> +}

With the above patch I'm getting ICE as below while building arm-none-eabi
target:

checking for scalbnl... during RTL pass: expand

generice_bug/src/gcc/libstdc++-v3/src/c++98/locale_init.cc: In constructor
'std::locale::_Impl::_Impl(std::size_t)':

generice_bug/src/gcc/libstdc++-v3/src/c++98/locale_init.cc:471:3: internal
compiler error: tree check: expected bit_field_ref or mem_ref, have ssa_name in
expand_assignment, at expr.c:5203
  471 |   locale::_Impl::
      |   ^~~~~~

[Bug target/97205] arm: Compiler fails with an ICE for -O0 on Trunk and GCC-10 for _Generic feature.

[bernd.edlinger at hotmail dot de]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97205

--- Comment #5 from Bernd Edlinger <bernd.edlinger at hotmail dot de> ---
(In reply to SRINATH PARVATHANENI from comment #4)
> With the above patch I'm getting ICE as below while building arm-none-eabi
> target:
> 
> checking for scalbnl... during RTL pass: expand
> 
> generice_bug/src/gcc/libstdc++-v3/src/c++98/locale_init.cc: In constructor
> 'std::locale::_Impl::_Impl(std::size_t)':
> 
> generice_bug/src/gcc/libstdc++-v3/src/c++98/locale_init.cc:471:3: internal
> compiler error: tree check: expected bit_field_ref or mem_ref, have ssa_name
> in expand_assignment, at expr.c:5203
>   471 |   locale::_Impl::
>       |   ^~~~~~

Hmm, it looks like I cannot reproduce this,
which version did you use for that ?
what is in line 5203 of expr.c ?

[Bug target/97205] arm: Compiler fails with an ICE for -O0 on Trunk and GCC-10 for _Generic feature.

[bernd.edlinger at hotmail dot de]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97205

--- Comment #6 from Bernd Edlinger <bernd.edlinger at hotmail dot de> ---
(In reply to rguenther@suse.de from comment #3)
> On Tue, 27 Oct 2020, bernd.edlinger at hotmail dot de wrote:
> > --- a/gcc/emit-rtl.c
> > +++ b/gcc/emit-rtl.c
> > @@ -2089,7 +2089,8 @@ set_mem_attributes_minus_bitpos (rtx ref, tree t, int
> > objectp,
> >         {
> >           gcc_assert (handled_component_p (t)
> >                       || TREE_CODE (t) == MEM_REF
> > -                     || TREE_CODE (t) == TARGET_MEM_REF);
> > +                     || TREE_CODE (t) == TARGET_MEM_REF
> > +                     || TREE_CODE (t) == SSA_NAME);
> 
> Can you track down this?  It's a red herring to have a MEM_EXPR
> that just is a SSA_NAME.
> 

This happens here:

          if (MEM_P (to_rtx))
            {
              /* If the field is at offset zero, we could have been given the
                 DECL_RTX of the parent struct.  Don't munge it.  */
              to_rtx = shallow_copy_rtx (to_rtx);
              set_mem_attributes_minus_bitpos (to_rtx, to, 0, bitpos);
              if (volatilep)
                MEM_VOLATILE_P (to_rtx) = 1;
            }

since the patch allows the SSA_NAME to reach this block.


> > --- a/gcc/expr.c
> > +++ b/gcc/expr.c
> > @@ -5200,6 +5200,9 @@ expand_assignment (tree to, tree from, bool nontemporal)
> >        || (TREE_CODE (to) == MEM_REF
> >           && (REF_REVERSE_STORAGE_ORDER (to)
> >               || mem_ref_refers_to_non_mem_p (to)))
> > +      || (TREE_CODE (to) == SSA_NAME
> > +         && mode != BLKmode
> > +         && TYPE_ALIGN (TREE_TYPE (to)) < GET_MODE_ALIGNMENT (mode))
> 
> But an SSA name is a register, esp. one with non-BLKmode.  And if
> expanded to a stack location that stack location should better
> be aligned...  or be BLKmode.  So I think the bug is elsewhere?
> 

Hmm, to avoid the ICE the SSA_NAME would need a naturally
aligned type, maybe replace it somehow in make_ssa_name_fn ...

But isn't it possible that expand_expr_real_1
returns the original unaligned decl here?

            case GIMPLE_SINGLE_RHS:
              {
                r = expand_expr_real (gimple_assign_rhs1 (g), target,
                                      tmode, modifier, alt_rtl,
                                      inner_reference_p);
                break;
              }
            default:
              gcc_unreachable ();
            }
          set_curr_insn_location (saved_loc);
          if (REG_P (r) && !REG_EXPR (r))
            set_reg_attrs_for_decl_rtl (SSA_NAME_VAR (exp), r);
          return r;

[Bug target/97205] arm: Compiler fails with an ICE for -O0 on Trunk and GCC-10 for _Generic feature.

[rguenther at suse dot de]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97205

--- Comment #7 from rguenther at suse dot de <rguenther at suse dot de> ---
On Wed, 28 Oct 2020, bernd.edlinger at hotmail dot de wrote:

> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97205
> 
> --- Comment #6 from Bernd Edlinger <bernd.edlinger at hotmail dot de> ---
> (In reply to rguenther@suse.de from comment #3)
> > On Tue, 27 Oct 2020, bernd.edlinger at hotmail dot de wrote:
> > > --- a/gcc/emit-rtl.c
> > > +++ b/gcc/emit-rtl.c
> > > @@ -2089,7 +2089,8 @@ set_mem_attributes_minus_bitpos (rtx ref, tree t, int
> > > objectp,
> > >         {
> > >           gcc_assert (handled_component_p (t)
> > >                       || TREE_CODE (t) == MEM_REF
> > > -                     || TREE_CODE (t) == TARGET_MEM_REF);
> > > +                     || TREE_CODE (t) == TARGET_MEM_REF
> > > +                     || TREE_CODE (t) == SSA_NAME);
> > 
> > Can you track down this?  It's a red herring to have a MEM_EXPR
> > that just is a SSA_NAME.
> > 
> 
> This happens here:
> 
>           if (MEM_P (to_rtx))
>             {
>               /* If the field is at offset zero, we could have been given the
>                  DECL_RTX of the parent struct.  Don't munge it.  */
>               to_rtx = shallow_copy_rtx (to_rtx);
>               set_mem_attributes_minus_bitpos (to_rtx, to, 0, bitpos);
>               if (volatilep)
>                 MEM_VOLATILE_P (to_rtx) = 1;
>             }
> 
> since the patch allows the SSA_NAME to reach this block.
> 
> 
> > > --- a/gcc/expr.c
> > > +++ b/gcc/expr.c
> > > @@ -5200,6 +5200,9 @@ expand_assignment (tree to, tree from, bool nontemporal)
> > >        || (TREE_CODE (to) == MEM_REF
> > >           && (REF_REVERSE_STORAGE_ORDER (to)
> > >               || mem_ref_refers_to_non_mem_p (to)))
> > > +      || (TREE_CODE (to) == SSA_NAME
> > > +         && mode != BLKmode
> > > +         && TYPE_ALIGN (TREE_TYPE (to)) < GET_MODE_ALIGNMENT (mode))
> > 
> > But an SSA name is a register, esp. one with non-BLKmode.  And if
> > expanded to a stack location that stack location should better
> > be aligned...  or be BLKmode.  So I think the bug is elsewhere?
> > 
> 
> Hmm, to avoid the ICE the SSA_NAME would need a naturally
> aligned type, maybe replace it somehow in make_ssa_name_fn ...

Ah, OK - only now looked at the testcase ;)  Yes, I think for
registers we should ignore decl/type alignment and _always_
use the natural (mode) alignment.  That is,

static unsigned int
align_local_variable (tree decl, bool really_expand)
{
  unsigned int align;

  if (TREE_CODE (decl) == SSA_NAME)
    align = TYPE_ALIGN (TREE_TYPE (decl));

should probably use GET_MODE_ALIGNMENT (TYPE_MODE (TREE_TYPE (decl)))
unless it is a BLKmode var.  There's a duplicate code in
expand_one_stack_var_1, not sure why it special-cases SSA_NAMEs there.

Also, when an SSA name gets a stack location, we should instead use
the underlying decl for the MEM_EXPR, not the SSA name.

> But isn't it possible that expand_expr_real_1
> returns the original unaligned decl here?
> 
>             case GIMPLE_SINGLE_RHS:
>               {
>                 r = expand_expr_real (gimple_assign_rhs1 (g), target,
>                                       tmode, modifier, alt_rtl,
>                                       inner_reference_p);
>                 break;
>               }
>             default:
>               gcc_unreachable ();
>             }
>           set_curr_insn_location (saved_loc);
>           if (REG_P (r) && !REG_EXPR (r))
>             set_reg_attrs_for_decl_rtl (SSA_NAME_VAR (exp), r);
>           return r;
> 
>

[Bug target/97205] arm: Compiler fails with an ICE for -O0 on Trunk and GCC-10 for _Generic feature.

[sripar01 at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97205

--- Comment #8 from SRINATH PARVATHANENI <sripar01 at gcc dot gnu.org> ---
(In reply to Bernd Edlinger from comment #5)
> (In reply to SRINATH PARVATHANENI from comment #4)
> > With the above patch I'm getting ICE as below while building arm-none-eabi
> > target:
> > 
> > checking for scalbnl... during RTL pass: expand
> > 
> > generice_bug/src/gcc/libstdc++-v3/src/c++98/locale_init.cc: In constructor
> > 'std::locale::_Impl::_Impl(std::size_t)':
> > 
> > generice_bug/src/gcc/libstdc++-v3/src/c++98/locale_init.cc:471:3: internal
> > compiler error: tree check: expected bit_field_ref or mem_ref, have ssa_name
> > in expand_assignment, at expr.c:5203
> >   471 |   locale::_Impl::
> >       |   ^~~~~~
> 
> Hmm, it looks like I cannot reproduce this,
> which version did you use for that ?
> what is in line 5203 of expr.c ?

Sorry for confusing you by mentioned ICE, that was because of merge conflict. I
have resolved the conflict and applied the patch correctly. The build is
successful and I have tested the test reported in this Bugzilla and it is
fixed.

Thanks.

[Bug target/97205] arm: Compiler fails with an ICE for -O0 on Trunk and GCC-10 for _Generic feature.

[bernd.edlinger at hotmail dot de]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97205

--- Comment #9 from Bernd Edlinger <bernd.edlinger at hotmail dot de> ---
(In reply to rguenther@suse.de from comment #7)
> On Wed, 28 Oct 2020, bernd.edlinger at hotmail dot de wrote:
> 
> > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97205
> > 
> > --- Comment #6 from Bernd Edlinger <bernd.edlinger at hotmail dot de> ---
> > (In reply to rguenther@suse.de from comment #3)
> > > On Tue, 27 Oct 2020, bernd.edlinger at hotmail dot de wrote:
> > > > --- a/gcc/emit-rtl.c
> > > > +++ b/gcc/emit-rtl.c
> > > > @@ -2089,7 +2089,8 @@ set_mem_attributes_minus_bitpos (rtx ref, tree t, int
> > > > objectp,
> > > >         {
> > > >           gcc_assert (handled_component_p (t)
> > > >                       || TREE_CODE (t) == MEM_REF
> > > > -                     || TREE_CODE (t) == TARGET_MEM_REF);
> > > > +                     || TREE_CODE (t) == TARGET_MEM_REF
> > > > +                     || TREE_CODE (t) == SSA_NAME);
> > > 
> > > Can you track down this?  It's a red herring to have a MEM_EXPR
> > > that just is a SSA_NAME.
> > > 
> > 
> > This happens here:
> > 
> >           if (MEM_P (to_rtx))
> >             {
> >               /* If the field is at offset zero, we could have been given the
> >                  DECL_RTX of the parent struct.  Don't munge it.  */
> >               to_rtx = shallow_copy_rtx (to_rtx);
> >               set_mem_attributes_minus_bitpos (to_rtx, to, 0, bitpos);
> >               if (volatilep)
> >                 MEM_VOLATILE_P (to_rtx) = 1;
> >             }
> > 
> > since the patch allows the SSA_NAME to reach this block.
> > 
> > 
> > > > --- a/gcc/expr.c
> > > > +++ b/gcc/expr.c
> > > > @@ -5200,6 +5200,9 @@ expand_assignment (tree to, tree from, bool nontemporal)
> > > >        || (TREE_CODE (to) == MEM_REF
> > > >           && (REF_REVERSE_STORAGE_ORDER (to)
> > > >               || mem_ref_refers_to_non_mem_p (to)))
> > > > +      || (TREE_CODE (to) == SSA_NAME
> > > > +         && mode != BLKmode
> > > > +         && TYPE_ALIGN (TREE_TYPE (to)) < GET_MODE_ALIGNMENT (mode))
> > > 
> > > But an SSA name is a register, esp. one with non-BLKmode.  And if
> > > expanded to a stack location that stack location should better
> > > be aligned...  or be BLKmode.  So I think the bug is elsewhere?
> > > 
> > 
> > Hmm, to avoid the ICE the SSA_NAME would need a naturally
> > aligned type, maybe replace it somehow in make_ssa_name_fn ...
> 
> Ah, OK - only now looked at the testcase ;)  Yes, I think for
> registers we should ignore decl/type alignment and _always_
> use the natural (mode) alignment.  That is,
> 
> static unsigned int
> align_local_variable (tree decl, bool really_expand)
> {
>   unsigned int align;
> 
>   if (TREE_CODE (decl) == SSA_NAME)
>     align = TYPE_ALIGN (TREE_TYPE (decl));
> 
> should probably use GET_MODE_ALIGNMENT (TYPE_MODE (TREE_TYPE (decl)))
> unless it is a BLKmode var.  There's a duplicate code in
> expand_one_stack_var_1, not sure why it special-cases SSA_NAMEs there.
> 

Ah, yes, using a similar strategy as we did in r274986
where we aligned param_decls, I would say this
completely untested patch goes in the same direction:

diff --git a/gcc/cfgexpand.c b/gcc/cfgexpand.c
index f3f17d3..030d0cb 100644
--- a/gcc/cfgexpand.c
+++ b/gcc/cfgexpand.c
@@ -366,7 +366,18 @@ 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)
+         && ((optab_handler (movmisalign_optab, mode)
+              != CODE_FOR_nothing)
+             || targetm.slow_unaligned_access (mode, align)))
+       align = GET_MODE_ALIGNMENT (mode);
+    }
   else
     {
       align = LOCAL_DECL_ALIGNMENT (decl);
@@ -1022,6 +1033,17 @@ expand_one_stack_var_at (tree decl, rtx base, unsigned
base_align,
     }

   set_rtl (decl, x);
+
+  if (TREE_CODE (decl) == SSA_NAME
+      && GET_MODE (x) != BLKmode
+      && MEM_ALIGN (x) < GET_MODE_ALIGNMENT (GET_MODE (x))
+      && ((optab_handler (movmisalign_optab, GET_MODE (x))
+          != CODE_FOR_nothing)
+         || targetm.slow_unaligned_access (GET_MODE (x), MEM_ALIGN (x))))
+    {
+      gcc_checking_assert (GET_MODE_ALIGNMENT (GET_MODE (x)) <= base_align);
+      set_mem_align (x, GET_MODE_ALIGNMENT (GET_MODE (x)));
+    }
 }

 class stack_vars_data
@@ -1327,13 +1349,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);


> Also, when an SSA name gets a stack location, we should instead use
> the underlying decl for the MEM_EXPR, not the SSA name.
> 

At least the MEM_EXPRs from this test case don't do that.

> > But isn't it possible that expand_expr_real_1
> > returns the original unaligned decl here?
> > 
> >             case GIMPLE_SINGLE_RHS:
> >               {
> >                 r = expand_expr_real (gimple_assign_rhs1 (g), target,
> >                                       tmode, modifier, alt_rtl,
> >                                       inner_reference_p);
> >                 break;
> >               }
> >             default:
> >               gcc_unreachable ();
> >             }
> >           set_curr_insn_location (saved_loc);
> >           if (REG_P (r) && !REG_EXPR (r))
> >             set_reg_attrs_for_decl_rtl (SSA_NAME_VAR (exp), r);
> >           return r;

My only concern left is if this code path might somehow return an
misaligned MEM_P from that gimple_assign_rhs, but probably
there is an obvious reason why that may never happen.

[Bug target/97205] arm: Compiler fails with an ICE for -O0 on Trunk and GCC-10 for _Generic feature.

[rguenther at suse dot de]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97205

--- Comment #10 from rguenther at suse dot de <rguenther at suse dot de> ---
On Wed, 28 Oct 2020, bernd.edlinger at hotmail dot de wrote:

> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97205
> 
> --- Comment #9 from Bernd Edlinger <bernd.edlinger at hotmail dot de> ---
> (In reply to rguenther@suse.de from comment #7)
> > On Wed, 28 Oct 2020, bernd.edlinger at hotmail dot de wrote:
> > 
> > > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97205
> > > 
> > > --- Comment #6 from Bernd Edlinger <bernd.edlinger at hotmail dot de> ---
> > > (In reply to rguenther@suse.de from comment #3)
> > > > On Tue, 27 Oct 2020, bernd.edlinger at hotmail dot de wrote:
> > > > > --- a/gcc/emit-rtl.c
> > > > > +++ b/gcc/emit-rtl.c
> > > > > @@ -2089,7 +2089,8 @@ set_mem_attributes_minus_bitpos (rtx ref, tree t, int
> > > > > objectp,
> > > > >         {
> > > > >           gcc_assert (handled_component_p (t)
> > > > >                       || TREE_CODE (t) == MEM_REF
> > > > > -                     || TREE_CODE (t) == TARGET_MEM_REF);
> > > > > +                     || TREE_CODE (t) == TARGET_MEM_REF
> > > > > +                     || TREE_CODE (t) == SSA_NAME);
> > > > 
> > > > Can you track down this?  It's a red herring to have a MEM_EXPR
> > > > that just is a SSA_NAME.
> > > > 
> > > 
> > > This happens here:
> > > 
> > >           if (MEM_P (to_rtx))
> > >             {
> > >               /* If the field is at offset zero, we could have been given the
> > >                  DECL_RTX of the parent struct.  Don't munge it.  */
> > >               to_rtx = shallow_copy_rtx (to_rtx);
> > >               set_mem_attributes_minus_bitpos (to_rtx, to, 0, bitpos);
> > >               if (volatilep)
> > >                 MEM_VOLATILE_P (to_rtx) = 1;
> > >             }
> > > 
> > > since the patch allows the SSA_NAME to reach this block.
> > > 
> > > 
> > > > > --- a/gcc/expr.c
> > > > > +++ b/gcc/expr.c
> > > > > @@ -5200,6 +5200,9 @@ expand_assignment (tree to, tree from, bool nontemporal)
> > > > >        || (TREE_CODE (to) == MEM_REF
> > > > >           && (REF_REVERSE_STORAGE_ORDER (to)
> > > > >               || mem_ref_refers_to_non_mem_p (to)))
> > > > > +      || (TREE_CODE (to) == SSA_NAME
> > > > > +         && mode != BLKmode
> > > > > +         && TYPE_ALIGN (TREE_TYPE (to)) < GET_MODE_ALIGNMENT (mode))
> > > > 
> > > > But an SSA name is a register, esp. one with non-BLKmode.  And if
> > > > expanded to a stack location that stack location should better
> > > > be aligned...  or be BLKmode.  So I think the bug is elsewhere?
> > > > 
> > > 
> > > Hmm, to avoid the ICE the SSA_NAME would need a naturally
> > > aligned type, maybe replace it somehow in make_ssa_name_fn ...
> > 
> > Ah, OK - only now looked at the testcase ;)  Yes, I think for
> > registers we should ignore decl/type alignment and _always_
> > use the natural (mode) alignment.  That is,
> > 
> > static unsigned int
> > align_local_variable (tree decl, bool really_expand)
> > {
> >   unsigned int align;
> > 
> >   if (TREE_CODE (decl) == SSA_NAME)
> >     align = TYPE_ALIGN (TREE_TYPE (decl));
> > 
> > should probably use GET_MODE_ALIGNMENT (TYPE_MODE (TREE_TYPE (decl)))
> > unless it is a BLKmode var.  There's a duplicate code in
> > expand_one_stack_var_1, not sure why it special-cases SSA_NAMEs there.
> > 
> 
> Ah, yes, using a similar strategy as we did in r274986
> where we aligned param_decls, I would say this
> completely untested patch goes in the same direction:
> 
> diff --git a/gcc/cfgexpand.c b/gcc/cfgexpand.c
> index f3f17d3..030d0cb 100644
> --- a/gcc/cfgexpand.c
> +++ b/gcc/cfgexpand.c
> @@ -366,7 +366,18 @@ 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)
> +         && ((optab_handler (movmisalign_optab, mode)
> +              != CODE_FOR_nothing)
> +             || targetm.slow_unaligned_access (mode, align)))

I wouldn't even restrict it this way - after all if we have an
SSA_NAME its address hasn't been exposed and thus any different
user requested alignment isn't exposed.  Instead we can work
to produce optimal code which means aligning according to the mode
(that's what the RA would do when spilling the 'register').

Note that this would argue we could fix the types for SSA names
upfront but there might be some complication when we have to
adjust the underlying decl and that's a PARM_DECL.

> +       align = GET_MODE_ALIGNMENT (mode);
> +    }
>    else
>      {
>        align = LOCAL_DECL_ALIGNMENT (decl);
> @@ -1022,6 +1033,17 @@ expand_one_stack_var_at (tree decl, rtx base, unsigned
> base_align,
>      }
> 
>    set_rtl (decl, x);
> +
> +  if (TREE_CODE (decl) == SSA_NAME
> +      && GET_MODE (x) != BLKmode
> +      && MEM_ALIGN (x) < GET_MODE_ALIGNMENT (GET_MODE (x))
> +      && ((optab_handler (movmisalign_optab, GET_MODE (x))
> +          != CODE_FOR_nothing)
> +         || targetm.slow_unaligned_access (GET_MODE (x), MEM_ALIGN (x))))

I failed to track down where we'd expand this to a possibly
unaligned mem - but is this just bogus MEM_ALIGN set?  Can we instead
fix that somehow?

> +    {
> +      gcc_checking_assert (GET_MODE_ALIGNMENT (GET_MODE (x)) <= base_align);
> +      set_mem_align (x, GET_MODE_ALIGNMENT (GET_MODE (x)));
> +    }
>  }
> 
>  class stack_vars_data
> @@ -1327,13 +1349,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);
> 
> 
> > Also, when an SSA name gets a stack location, we should instead use
> > the underlying decl for the MEM_EXPR, not the SSA name.
> > 
> 
> At least the MEM_EXPRs from this test case don't do that.

Figured that.  Since we don't always have an underlying decl it's
probably error-prone.

> > > But isn't it possible that expand_expr_real_1
> > > returns the original unaligned decl here?
> > > 
> > >             case GIMPLE_SINGLE_RHS:
> > >               {
> > >                 r = expand_expr_real (gimple_assign_rhs1 (g), target,
> > >                                       tmode, modifier, alt_rtl,
> > >                                       inner_reference_p);
> > >                 break;
> > >               }
> > >             default:
> > >               gcc_unreachable ();
> > >             }
> > >           set_curr_insn_location (saved_loc);
> > >           if (REG_P (r) && !REG_EXPR (r))
> > >             set_reg_attrs_for_decl_rtl (SSA_NAME_VAR (exp), r);
> > >           return r;
> 
> My only concern left is if this code path might somehow return an
> misaligned MEM_P from that gimple_assign_rhs, but probably
> there is an obvious reason why that may never happen.

never say never ... but unless we have a testcase we just claim so.

[Bug target/97205] arm: Compiler fails with an ICE for -O0 on Trunk and GCC-10 for _Generic feature.

[bernd.edlinger at hotmail dot de]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97205

--- Comment #11 from Bernd Edlinger <bernd.edlinger at hotmail dot de> ---
(In reply to rguenther@suse.de from comment #10)
> On Wed, 28 Oct 2020, bernd.edlinger at hotmail dot de wrote:
> 
> > --- Comment #9 from Bernd Edlinger <bernd.edlinger at hotmail dot de> ---
> > (In reply to rguenther@suse.de from comment #7)
> > 
> > Ah, yes, using a similar strategy as we did in r274986
> > where we aligned param_decls, I would say this
> > completely untested patch goes in the same direction:
> > 
> > diff --git a/gcc/cfgexpand.c b/gcc/cfgexpand.c
> > index f3f17d3..030d0cb 100644
> > --- a/gcc/cfgexpand.c
> > +++ b/gcc/cfgexpand.c
> > @@ -366,7 +366,18 @@ 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)
> > +         && ((optab_handler (movmisalign_optab, mode)
> > +              != CODE_FOR_nothing)
> > +             || targetm.slow_unaligned_access (mode, align)))
> 
> I wouldn't even restrict it this way - after all if we have an
> SSA_NAME its address hasn't been exposed and thus any different
> user requested alignment isn't exposed.  Instead we can work
> to produce optimal code which means aligning according to the mode
> (that's what the RA would do when spilling the 'register').
> 

Agreed.

> Note that this would argue we could fix the types for SSA names
> upfront but there might be some complication when we have to
> adjust the underlying decl and that's a PARM_DECL.
> 

I would like to avoid changing the types to be more aligned,
just make the MEM_ALIGN larger than what the TYPE_ALIGN
requires.  Also in the case of the incoming param_decls,
the decl_rtl may reflect the true alignment of the stack,
which may be higher than required by the type.
The backend looks at the MEM_ALIGN and can in some cases
generate better code, based on the MEM_ALIGN and, it
can assert when we have a misaligned MEM_P, also based
on the value in MEM_ALIGN.

> > +       align = GET_MODE_ALIGNMENT (mode);
> > +    }
> >    else
> >      {
> >        align = LOCAL_DECL_ALIGNMENT (decl);
> > @@ -1022,6 +1033,17 @@ expand_one_stack_var_at (tree decl, rtx base, unsigned
> > base_align,
> >      }
> > 
> >    set_rtl (decl, x);
> > +
> > +  if (TREE_CODE (decl) == SSA_NAME
> > +      && GET_MODE (x) != BLKmode
> > +      && MEM_ALIGN (x) < GET_MODE_ALIGNMENT (GET_MODE (x))
> > +      && ((optab_handler (movmisalign_optab, GET_MODE (x))
> > +          != CODE_FOR_nothing)
> > +         || targetm.slow_unaligned_access (GET_MODE (x), MEM_ALIGN (x))))
> 
> I failed to track down where we'd expand this to a possibly
> unaligned mem - but is this just bogus MEM_ALIGN set?  Can we instead
> fix that somehow?
> 

No, this fixes the assertion in the back-end, while the first hunk
makes the memory at least mode aligned, the MEM_ALIGN needs to
be adjusted here, GET_MODE_ALIGNMENT (GET_MODE (x)) is a lower
bound of the true alignment.

Also in the case of the PARM_DECLs it is possible that we take
advantage from a MEM_ALIGN which happens to be larger than
what's implied by TYPE_ALIGN.

> > +    {
> > +      gcc_checking_assert (GET_MODE_ALIGNMENT (GET_MODE (x)) <= base_align);
> > +      set_mem_align (x, GET_MODE_ALIGNMENT (GET_MODE (x)));
> > +    }
> >  }
> > 
> >  class stack_vars_data
> > @@ -1327,13 +1349,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);
> > 
> > 
> > > Also, when an SSA name gets a stack location, we should instead use
> > > the underlying decl for the MEM_EXPR, not the SSA name.
> > > 
> > 
> > At least the MEM_EXPRs from this test case don't do that.
> 
> Figured that.  Since we don't always have an underlying decl it's
> probably error-prone.
> 

I tried this, but it did not find anything so far:

diff --git a/gcc/emit-rtl.c b/gcc/emit-rtl.c
index 3706f0a..e7e9b3d 100644
--- a/gcc/emit-rtl.c
+++ b/gcc/emit-rtl.c
@@ -2196,6 +2196,7 @@ set_mem_expr (rtx mem, tree expr)
 {
   mem_attrs attrs (*get_mem_attrs (mem));
   attrs.expr = expr;
+  gcc_checking_assert (expr == NULL || TREE_CODE (expr) != SSA_NAME);
   set_mem_attrs (mem, &attrs);
 }


> > > > But isn't it possible that expand_expr_real_1
> > > > returns the original unaligned decl here?
> > > > 
> > > >             case GIMPLE_SINGLE_RHS:
> > > >               {
> > > >                 r = expand_expr_real (gimple_assign_rhs1 (g), target,
> > > >                                       tmode, modifier, alt_rtl,
> > > >                                       inner_reference_p);
> > > >                 break;
> > > >               }
> > > >             default:
> > > >               gcc_unreachable ();
> > > >             }
> > > >           set_curr_insn_location (saved_loc);
> > > >           if (REG_P (r) && !REG_EXPR (r))
> > > >             set_reg_attrs_for_decl_rtl (SSA_NAME_VAR (exp), r);
> > > >           return r;
> > 
> > My only concern left is if this code path might somehow return an
> > misaligned MEM_P from that gimple_assign_rhs, but probably
> > there is an obvious reason why that may never happen.
> 
> never say never ... but unless we have a testcase we just claim so.

[Bug target/97205] arm: Compiler fails with an ICE for -O0 on Trunk and GCC-10 for _Generic feature.

[bernd.edlinger at hotmail dot de]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97205

--- Comment #12 from Bernd Edlinger <bernd.edlinger at hotmail dot de> ---
(In reply to Bernd Edlinger from comment #11)
> (In reply to rguenther@suse.de from comment #10)
> > 
> > I failed to track down where we'd expand this to a possibly
> > unaligned mem - but is this just bogus MEM_ALIGN set?  Can we instead
> > fix that somehow?
> > 
> 
> No, this fixes the assertion in the back-end, while the first hunk
> makes the memory at least mode aligned, the MEM_ALIGN needs to
> be adjusted here, GET_MODE_ALIGNMENT (GET_MODE (x)) is a lower
> bound of the true alignment.
> 
> Also in the case of the PARM_DECLs it is possible that we take
> advantage from a MEM_ALIGN which happens to be larger than
> what's implied by TYPE_ALIGN.
> 

In case, I misunderstood the question.
set_rtl does this, but it is also called from a lot
of other places, for instance set_parm_rtl where
I did not want to change the alignment.

[Bug target/97205] arm: Compiler fails with an ICE for -O0 on Trunk and GCC-10 for _Generic feature.

[cvs-commit at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97205

--- Comment #13 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Bernd Edlinger <edlinger@gcc.gnu.org>:

https://gcc.gnu.org/g:23ac7a009ecfeec3eab79136abed8aac9768b458

commit r11-4668-g23ac7a009ecfeec3eab79136abed8aac9768b458
Author: Bernd Edlinger <bernd.edlinger@hotmail.de>
Date:   Sun Nov 1 07:32:20 2020 +0100

    Fix PR97205

    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.

[Bug target/97205] arm: Compiler fails with an ICE for -O0 on Trunk and GCC-10 for _Generic feature.

[edlinger at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97205

Bernd Edlinger <edlinger at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
         Resolution|---                         |FIXED
             Status|UNCONFIRMED                 |RESOLVED

--- Comment #14 from Bernd Edlinger <edlinger at gcc dot gnu.org> ---
fixed on trunk.

[Bug target/97205] arm: Compiler fails with an ICE for -O0 on Trunk and GCC-10 for _Generic feature.

[sripar01 at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97205

--- Comment #15 from SRINATH PARVATHANENI <sripar01 at gcc dot gnu.org> ---
(In reply to Bernd Edlinger from comment #14)
> fixed on trunk.

Thanks Bernd for fixing this on trunk, would you mind backporting this to
GCC-10 as well?

Thanks you.

Regards,
Srinath.

[Bug target/97205] arm: Compiler fails with an ICE for -O0 on Trunk and GCC-10 for _Generic feature.

[bernd.edlinger at hotmail dot de]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97205

--- Comment #16 from Bernd Edlinger <bernd.edlinger at hotmail dot de> ---
(In reply to SRINATH PARVATHANENI from comment #15)
> (In reply to Bernd Edlinger from comment #14)
> > fixed on trunk.
> 
> Thanks Bernd for fixing this on trunk, would you mind backporting this to
> GCC-10 as well?
> 
> Thanks you.
> 
> Regards,
> Srinath.

Since it is a gcc_checking_assert that triggers the ICE,
I assumed that does not affect a release build,
is that correct?

[Bug target/97205] arm: Compiler fails with an ICE for -O0 on Trunk and GCC-10 for _Generic feature.

[rguenther at suse dot de]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97205

--- Comment #17 from rguenther at suse dot de <rguenther at suse dot de> ---
On Mon, 23 Nov 2020, bernd.edlinger at hotmail dot de wrote:

> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97205
> 
> --- Comment #16 from Bernd Edlinger <bernd.edlinger at hotmail dot de> ---
> (In reply to SRINATH PARVATHANENI from comment #15)
> > (In reply to Bernd Edlinger from comment #14)
> > > fixed on trunk.
> > 
> > Thanks Bernd for fixing this on trunk, would you mind backporting this to
> > GCC-10 as well?
> > 
> > Thanks you.
> > 
> > Regards,
> > Srinath.
> 
> Since it is a gcc_checking_assert that triggers the ICE,
> I assumed that does not affect a release build,
> is that correct?

That is correct.

[Bug target/97205] arm: Compiler fails with an ICE for -O0 on Trunk and GCC-10 for _Generic feature.

[acoplan at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97205

Alex Coplan <acoplan at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |acoplan at gcc dot gnu.org

--- Comment #18 from Alex Coplan <acoplan at gcc dot gnu.org> ---
(In reply to rguenther@suse.de from comment #17)
> On Mon, 23 Nov 2020, bernd.edlinger at hotmail dot de wrote:
> 
> > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97205
> > 
> > --- Comment #16 from Bernd Edlinger <bernd.edlinger at hotmail dot de> ---
> > (In reply to SRINATH PARVATHANENI from comment #15)
> > > (In reply to Bernd Edlinger from comment #14)
> > > > fixed on trunk.
> > > 
> > > Thanks Bernd for fixing this on trunk, would you mind backporting this to
> > > GCC-10 as well?
> > > 
> > > Thanks you.
> > > 
> > > Regards,
> > > Srinath.
> > 
> > Since it is a gcc_checking_assert that triggers the ICE,
> > I assumed that does not affect a release build,
> > is that correct?
> 
> That is correct.

Is it OK to backport to GCC 10 in any case?

[Bug target/97205] arm: Compiler fails with an ICE for -O0 on Trunk and GCC-10 for _Generic feature.

[acoplan at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97205

--- Comment #19 from Alex Coplan <acoplan at gcc dot gnu.org> ---
Ah, I missed that Srinath had already requested a backport here:
https://gcc.gnu.org/pipermail/gcc-patches/2021-April/569318.html

[Bug target/97205] arm: Compiler fails with an ICE for -O0 on Trunk and GCC-10 for _Generic feature.

[sripar01 at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97205

--- Comment #20 from SRINATH PARVATHANENI <sripar01 at gcc dot gnu.org> ---
(In reply to rguenther@suse.de from comment #17)
> On Mon, 23 Nov 2020, bernd.edlinger at hotmail dot de wrote:
> 
> > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97205
> > 
> > --- Comment #16 from Bernd Edlinger <bernd.edlinger at hotmail dot de> ---
> > (In reply to SRINATH PARVATHANENI from comment #15)
> > > (In reply to Bernd Edlinger from comment #14)
> > > > fixed on trunk.
> > > 
> > > Thanks Bernd for fixing this on trunk, would you mind backporting this to
> > > GCC-10 as well?
> > > 
> > > Thanks you.
> > > 
> > > Regards,
> > > Srinath.
> > 
> > Since it is a gcc_checking_assert that triggers the ICE,
> > I assumed that does not affect a release build,
> > is that correct?
> 
> That is correct.

I did not understand what you meant above, please correct me if I'm wrong.
My observation:

$ arm-none-eabi-gcc bugx.c  -O0 -c
during RTL pass: expand
bugx.c: In function 'fn1':
bugx.c:4:5: internal compiler error: in gen_movsi, at config/arm/arm.md:6364
    4 |   x b = ({
      |     ^
0x14ede14 gen_movsi(rtx_def*, rtx_def*)
       
/data/dgboter/bbs/build22--cen7x86_64/buildbot/cen7x86_64--arm-none-eabi--all-profiles/build/src/gcc/gcc/config/arm/arm.md:6364
0x92153f insn_gen_fn::operator()(rtx_def*, rtx_def*) const
       
/data/dgboter/bbs/build22--cen7x86_64/buildbot/cen7x86_64--arm-none-eabi--all-profiles/build/src/gcc/gcc/recog.h:317
0x92153f emit_move_insn_1(rtx_def*, rtx_def*)
       
/data/dgboter/bbs/build22--cen7x86_64/buildbot/cen7x86_64--arm-none-eabi--all-profiles/build/src/gcc/gcc/expr.c:3759
0x921cf4 emit_move_insn(rtx_def*, rtx_def*)
       
/data/dgboter/bbs/build22--cen7x86_64/buildbot/cen7x86_64--arm-none-eabi--all-profiles/build/src/gcc/gcc/expr.c:3855
0x92a6f0 store_expr(tree_node*, rtx_def*, int, bool, bool)
       
/data/dgboter/bbs/build22--cen7x86_64/buildbot/cen7x86_64--arm-none-eabi--all-profiles/build/src/gcc/gcc/expr.c:5931
0x92c973 expand_assignment(tree_node*, tree_node*, bool)
       
/data/dgboter/bbs/build22--cen7x86_64/buildbot/cen7x86_64--arm-none-eabi--all-profiles/build/src/gcc/gcc/expr.c:5516
0x7d965c expand_gimple_stmt_1
       
/data/dgboter/bbs/build22--cen7x86_64/buildbot/cen7x86_64--arm-none-eabi--all-profiles/build/src/gcc/gcc/cfgexpand.c:3755
0x7d965c expand_gimple_stmt
       
/data/dgboter/bbs/build22--cen7x86_64/buildbot/cen7x86_64--arm-none-eabi--all-profiles/build/src/gcc/gcc/cfgexpand.c:3851
0x7dbf65 expand_gimple_basic_block
       
/data/dgboter/bbs/build22--cen7x86_64/buildbot/cen7x86_64--arm-none-eabi--all-profiles/build/src/gcc/gcc/cfgexpand.c:5895
0x7dce7b execute
       
/data/dgboter/bbs/build22--cen7x86_64/buildbot/cen7x86_64--arm-none-eabi--all-profiles/build/src/gcc/gcc/cfgexpand.c:6550
Please submit a full bug report,
with preprocessed source if appropriate.
Please include the complete backtrace with any bug report.
See <https://gcc.gnu.org/bugs/> for instructions.

$ arm-none-eabi-gcc -v
Using built-in specs.
COLLECT_GCC=/arm/pdtl/builds/latest-fsf-10/installed/cen7x86_64/arm-none-eabi/bin/arm-none-eabi-gcc
COLLECT_LTO_WRAPPER=/arm/pdtl/builds/fsf-10.233/installed/cen7x86_64/arm-none-eabi/bin/../libexec/gcc/arm-none-eabi/10.3.1/lto-wrapper
Target: arm-none-eabi
Configured with:
/data/dgboter/bbs/build22--cen7x86_64/buildbot/cen7x86_64--arm-none-eabi--all-profiles/build/src/gcc/configure
--target=arm-none-eabi
--prefix=/data/dgboter/bbs/build22--cen7x86_64/buildbot/cen7x86_64--arm-none-eabi--all-profiles/build/build-arm-none-eabi/install//
--with-gmp=/data/dgboter/bbs/build22--cen7x86_64/buildbot/cen7x86_64--arm-none-eabi--all-profiles/build/build-arm-none-eabi/host-tools
--with-mpfr=/data/dgboter/bbs/build22--cen7x86_64/buildbot/cen7x86_64--arm-none-eabi--all-profiles/build/build-arm-none-eabi/host-tools
--with-mpc=/data/dgboter/bbs/build22--cen7x86_64/buildbot/cen7x86_64--arm-none-eabi--all-profiles/build/build-arm-none-eabi/host-tools
--with-isl=/data/dgboter/bbs/build22--cen7x86_64/buildbot/cen7x86_64--arm-none-eabi--all-profiles/build/build-arm-none-eabi/host-tools
--disable-shared --disable-nls --disable-threads --disable-tls
--enable-checking=yes --enable-languages=c,c++,fortran --with-newlib
--with-multilib-list=aprofile,rmprofile --with-pkgversion=fsf-10.233
Thread model: single
Supported LTO compression algorithms: zlib
gcc version 10.3.1 20210507 (fsf-10.233)

I still see the ICE with GCC-10, should I be backporting this patch, if yes,
please approve the following patch:
https://gcc.gnu.org/pipermail/gcc-patches/2021-April/569318.html

[Bug target/97205] arm: Compiler fails with an ICE for -O0 on Trunk and GCC-10 for _Generic feature.

[bernd.edlinger at hotmail dot de]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97205

--- Comment #21 from Bernd Edlinger <bernd.edlinger at hotmail dot de> ---
Hi Srinath,

when we add new assertions to gcc we use always a gcc_checking_assert
nowadays, that is also the case here.

The assertion is only firing in your compiler because it is a development
snapshot 10.3.1.  So that is an experimemtal version.

That means the release version 10.3.0 will not have an ICE with this test case,
except the long-standing potential wrong code due to unaligned memory access,
however most contemporary ARM chips do no longer trap on an unaligned
load/store.

But on the other hand, this change might affects all targets whether
or not they have any issue at all.

So it might trigger a completely unrelated bug in a completely different area
of the compiler.

That said I do not object this back-port, I just wanted to say that it
fixes a non-issue in a released compiler version.

And even a small risk of a regression seems to be not worth it.


Regards
Bernd.

[Bug target/97205] arm: Compiler fails with an ICE for -O0 on Trunk and GCC-10 for _Generic feature.

[rguenther at suse dot de]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97205

--- Comment #22 from rguenther at suse dot de <rguenther at suse dot de> ---
On Wed, 19 May 2021, bernd.edlinger at hotmail dot de wrote:

> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97205
> 
> --- Comment #21 from Bernd Edlinger <bernd.edlinger at hotmail dot de> ---
> Hi Srinath,
> 
> when we add new assertions to gcc we use always a gcc_checking_assert
> nowadays, that is also the case here.
> 
> The assertion is only firing in your compiler because it is a development
> snapshot 10.3.1.  So that is an experimemtal version.

10.3.1 is not an experimental version, so the assert should not fire
there unless you configure with --enable-checking (which you might
have done).

[Bug target/97205] arm: Compiler fails with an ICE for -O0 on Trunk and GCC-10 for _Generic feature.

[sripar01 at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97205

--- Comment #23 from SRINATH PARVATHANENI <sripar01 at gcc dot gnu.org> ---
(In reply to rguenther@suse.de from comment #22)
> On Wed, 19 May 2021, bernd.edlinger at hotmail dot de wrote:
> 
> > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97205
> > 
> > --- Comment #21 from Bernd Edlinger <bernd.edlinger at hotmail dot de> ---
> > Hi Srinath,
> > 
> > when we add new assertions to gcc we use always a gcc_checking_assert
> > nowadays, that is also the case here.
> > 
> > The assertion is only firing in your compiler because it is a development
> > snapshot 10.3.1.  So that is an experimemtal version.
> 
> 10.3.1 is not an experimental version, so the assert should not fire
> there unless you configure with --enable-checking (which you might
> have done).

Thanks rguenther@suse.de and Bernd Edlinger.

My I'm again having confusion with the wording development/release/experiment
versions.

All development versions checks for gcc_checking_assert whether
--enable-checking=yes or --enable-checking=no (eg: 10.3.1), but release
versions only check for gcc_checking_assert when --enable-checking=yes
(eg:10.3.0).

If I wanted to build a toolchain using development version (eg:10.3.1) and for
this test to not to fail , I should either pass --enable-checking=no or apply
this patch (which was committed to trunk) on the top of gcc-10.3.1 source code.

Is my understanding correct?

[Bug target/97205] arm: Compiler fails with an ICE for -O0 on Trunk and GCC-10 for _Generic feature.

[rguenther at suse dot de]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97205

--- Comment #24 from rguenther at suse dot de <rguenther at suse dot de> ---
On Thu, 20 May 2021, sripar01 at gcc dot gnu.org wrote:

> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97205
> 
> --- Comment #23 from SRINATH PARVATHANENI <sripar01 at gcc dot gnu.org> ---
> (In reply to rguenther@suse.de from comment #22)
> > On Wed, 19 May 2021, bernd.edlinger at hotmail dot de wrote:
> > 
> > > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97205
> > > 
> > > --- Comment #21 from Bernd Edlinger <bernd.edlinger at hotmail dot de> ---
> > > Hi Srinath,
> > > 
> > > when we add new assertions to gcc we use always a gcc_checking_assert
> > > nowadays, that is also the case here.
> > > 
> > > The assertion is only firing in your compiler because it is a development
> > > snapshot 10.3.1.  So that is an experimemtal version.
> > 
> > 10.3.1 is not an experimental version, so the assert should not fire
> > there unless you configure with --enable-checking (which you might
> > have done).
> 
> Thanks rguenther@suse.de and Bernd Edlinger.
> 
> My I'm again having confusion with the wording development/release/experiment
> versions.
> 
> All development versions checks for gcc_checking_assert whether
> --enable-checking=yes or --enable-checking=no (eg: 10.3.1), but release
> versions only check for gcc_checking_assert when --enable-checking=yes
> (eg:10.3.0).

No.  gcc_checking_assert is enabled when --enable-checking=yes
--enable-checking defaults to =yes on the master branch.  On release
branches it defaults to =release, that includes snapshots and thus
10.3.1.

> If I wanted to build a toolchain using development version (eg:10.3.1) and for
> this test to not to fail , I should either pass --enable-checking=no or apply
> this patch (which was committed to trunk) on the top of gcc-10.3.1 source code.
> 
> Is my understanding correct?

You don't need anything special for 10.3.1, that is not a development
version.  If you do not specify any --enable-checking at configure
time the default is that the assert does not trigger and thus
the ICE should not happen.

[Bug target/97205] arm: Compiler fails with an ICE for -O0 on Trunk and GCC-10 for _Generic feature.

[sripar01 at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97205

--- Comment #25 from SRINATH PARVATHANENI <sripar01 at gcc dot gnu.org> ---
(In reply to rguenther@suse.de from comment #24)
> On Thu, 20 May 2021, sripar01 at gcc dot gnu.org wrote:
> 
> > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97205
> > 
> > --- Comment #23 from SRINATH PARVATHANENI <sripar01 at gcc dot gnu.org> ---
> > (In reply to rguenther@suse.de from comment #22)
> > > On Wed, 19 May 2021, bernd.edlinger at hotmail dot de wrote:
> > > 
> > > > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97205
> > > > 
> > > > --- Comment #21 from Bernd Edlinger <bernd.edlinger at hotmail dot de> ---
> > > > Hi Srinath,
> > > > 
> > > > when we add new assertions to gcc we use always a gcc_checking_assert
> > > > nowadays, that is also the case here.
> > > > 
> > > > The assertion is only firing in your compiler because it is a development
> > > > snapshot 10.3.1.  So that is an experimemtal version.
> > > 
> > > 10.3.1 is not an experimental version, so the assert should not fire
> > > there unless you configure with --enable-checking (which you might
> > > have done).
> > 
> > Thanks rguenther@suse.de and Bernd Edlinger.
> > 
> > My I'm again having confusion with the wording development/release/experiment
> > versions.
> > 
> > All development versions checks for gcc_checking_assert whether
> > --enable-checking=yes or --enable-checking=no (eg: 10.3.1), but release
> > versions only check for gcc_checking_assert when --enable-checking=yes
> > (eg:10.3.0).
> 
> No.  gcc_checking_assert is enabled when --enable-checking=yes
> --enable-checking defaults to =yes on the master branch.  On release
> branches it defaults to =release, that includes snapshots and thus
> 10.3.1.
> 
> > If I wanted to build a toolchain using development version (eg:10.3.1) and for
> > this test to not to fail , I should either pass --enable-checking=no or apply
> > this patch (which was committed to trunk) on the top of gcc-10.3.1 source code.
> > 
> > Is my understanding correct?
> 
> You don't need anything special for 10.3.1, that is not a development
> version.  If you do not specify any --enable-checking at configure
> time the default is that the assert does not trigger and thus
> the ICE should not happen.

Thanks for the clarification.