[Bug c/105554] New: ICE: in emit_block_move_hints, at expr.cc:1829

[Bug c/105554] New: ICE: in emit_block_move_hints, at expr.cc:1829

[zhenyang.xu at uwaterloo dot ca]

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

            Bug ID: 105554
           Summary: ICE: in emit_block_move_hints, at expr.cc:1829
           Product: gcc
           Version: 13.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: c
          Assignee: unassigned at gcc dot gnu.org
          Reporter: zhenyang.xu at uwaterloo dot ca
  Target Milestone: ---

$ gcc-trunk -v
Using built-in specs.
COLLECT_GCC=gcc-trunk
COLLECT_LTO_WRAPPER=/scratch/software/gcc-trunk/libexec/gcc/x86_64-pc-linux-gnu/13.0.0/lto-wrapper
Target: x86_64-pc-linux-gnu
Configured with: /tmp/tmp.lx931ugYgb-gcc-builder/gcc/configure
--enable-languages=c,c++,lto --enable-checking-yes --enable-multiarch
--prefix=/scratch/software/gcc-trunk --disable-bootstrap
Thread model: posix
Supported LTO compression algorithms: zlib
gcc version 13.0.0 20220510 (experimental) [master -gbb2921ab8] (GCC)

$ cat mutant_133870291_1648466141340.c
__attribute__((target_clones("arch=core-avx2", "default")))
a(__attribute__((__vector_size__(4 * sizeof(long)))) long) {}

$ gcc-trunk -O3 -g -Wall -Wextra -c mutant_133870291_1648466141340.c
mutant_133870291_1648466141340.c:2:1: warning: return type defaults to ‘int’
[-Wimplicit-int]
    2 | a(__attribute__((__vector_size__(4 * sizeof(long)))) long) {}
      | ^
mutant_133870291_1648466141340.c: In function ‘a’:
mutant_133870291_1648466141340.c:2:1: note: the ABI for passing parameters with
32-byte alignment has changed in GCC 4.6
mutant_133870291_1648466141340.c:2:61: warning: control reaches end of non-void
function [-Wreturn-type]
    2 | a(__attribute__((__vector_size__(4 * sizeof(long)))) long) {}
      |                                                             ^
mutant_133870291_1648466141340.c:2:1: warning: AVX vector argument without AVX
enabled changes the ABI [-Wpsabi]
    2 | a(__attribute__((__vector_size__(4 * sizeof(long)))) long) {}
      | ^
during RTL pass: expand
mutant_133870291_1648466141340.c:2:1: internal compiler error: in
emit_block_move_hints, at expr.cc:1829
0x726cd5 emit_block_move_hints(rtx_def*, rtx_def*, rtx_def*, block_op_methods,
unsigned int, long, unsigned long, unsigned long, unsigned long, bool, bool*,
bool)
        /tmp/tmp.lx931ugYgb-gcc-builder/gcc/gcc/expr.cc:1829
0xc29395 emit_block_move(rtx_def*, rtx_def*, rtx_def*, block_op_methods)
        /tmp/tmp.lx931ugYgb-gcc-builder/gcc/gcc/expr.cc:1915
0xc8dc82 assign_parm_setup_block
        /tmp/tmp.lx931ugYgb-gcc-builder/gcc/gcc/function.cc:3124
0xc8dc82 assign_parms
        /tmp/tmp.lx931ugYgb-gcc-builder/gcc/gcc/function.cc:3705
0xc8f602 expand_function_start(tree_node*)
        /tmp/tmp.lx931ugYgb-gcc-builder/gcc/gcc/function.cc:5161
0xafda98 execute
        /tmp/tmp.lx931ugYgb-gcc-builder/gcc/gcc/cfgexpand.cc:6691
Please submit a full bug report, with preprocessed source (by using
-freport-bug).
Please include the complete backtrace with any bug report.
See <https://gcc.gnu.org/bugs/> for instructions.

[Bug target/105554] ICE: in emit_block_move_hints, at expr.cc:1829

[marxin at gcc dot gnu.org]

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

Martin Liška <marxin at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |jsm28 at gcc dot gnu.org,
                   |                            |marxin at gcc dot gnu.org
     Ever confirmed|0                           |1
   Last reconfirmed|                            |2022-05-11
             Status|UNCONFIRMED                 |NEW

--- Comment #1 from Martin Liška <marxin at gcc dot gnu.org> ---
Well, the syntax is newly accepted with r11-4494-ga4223abb3deb24e8, is it
correct?

[Bug target/105554] ICE: in emit_block_move_hints, at expr.cc:1829

[rguenth at gcc dot gnu.org]

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

Richard Biener <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Keywords|                            |ice-on-valid-code

--- Comment #2 from Richard Biener <rguenth at gcc dot gnu.org> ---
The following also ICEs with GCC 10:

typedef long v4di __attribute__((__vector_size__(4 * sizeof(long))));
__attribute__((target_clones("arch=core-avx2", "default")))
void a(v4di x) {}

[Bug target/105554] [9/10/11/12/13 Regression] ICE: in emit_block_move_hints, at expr.cc:1829 since r9-5509-g5928bc2ec06dd4e7

[marxin at gcc dot gnu.org]

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

Martin Liška <marxin at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Assignee|unassigned at gcc dot gnu.org      |marxin at gcc dot gnu.org
            Summary|ICE: in                     |[9/10/11/12/13 Regression]
                   |emit_block_move_hints, at   |ICE: in
                   |expr.cc:1829                |emit_block_move_hints, at
                   |                            |expr.cc:1829 since
                   |                            |r9-5509-g5928bc2ec06dd4e7
             Status|NEW                         |ASSIGNED

--- Comment #3 from Martin Liška <marxin at gcc dot gnu.org> ---
Started with r9-5509-g5928bc2ec06dd4e7, I'll take a look.

[Bug target/105554] [9/10/11/12/13 Regression] ICE: in emit_block_move_hints, at expr.cc:1829 since r9-5509-g5928bc2ec06dd4e7

[jakub at gcc dot gnu.org]

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

Jakub Jelinek <jakub at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |jakub at gcc dot gnu.org
   Target Milestone|---                         |9.5

[Bug target/105554] [10/11/12/13 Regression] ICE: in emit_block_move_hints, at expr.cc:1829 since r9-5509-g5928bc2ec06dd4e7

[rguenth at gcc dot gnu.org]

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

Richard Biener <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|9.5                         |10.4

--- Comment #4 from Richard Biener <rguenth at gcc dot gnu.org> ---
GCC 9 branch is being closed

[Bug target/105554] [10/11/12/13 Regression] ICE: in emit_block_move_hints, at expr.cc:1829 since r9-5509-g5928bc2ec06dd4e7

[jakub at gcc dot gnu.org]

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

Jakub Jelinek <jakub at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|10.4                        |10.5

--- Comment #5 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
GCC 10.4 is being released, retargeting bugs to GCC 10.5.

[Bug target/105554] [10/11/12/13 Regression] ICE: in emit_block_move_hints, at expr.cc:1829 since r9-5509-g5928bc2ec06dd4e7

[rguenth at gcc dot gnu.org]

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

Richard Biener <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Priority|P3                          |P2

[Bug target/105554] [10/11/12/13 Regression] ICE: in emit_block_move_hints, at expr.cc:1829 since r9-5509-g5928bc2ec06dd4e7

[marxin at gcc dot gnu.org]

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

Martin Liška <marxin at gcc dot gnu.org> changed:

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

--- Comment #6 from Martin Liška <marxin at gcc dot gnu.org> ---
We fail in the param assignment:

(gdb) pp x
(reg:V4DI 82)
(gdb) pp y
(mem/c:BLK (reg/f:DI 76 virtual-incoming-args) [1 x+0 S32 A256])

So we will likely need something similar to what we have in tree-inline.cc:

  5928    /* For vector typed decls make sure to update DECL_MODE according
  5929       to the new function context.  */
  5930    if (VECTOR_TYPE_P (TREE_TYPE (copy)))
  5931      SET_DECL_MODE (copy, TYPE_MODE (TREE_TYPE (copy)));

@Richi: Do you have a clue where to adjust it?

[Bug target/105554] [10/11/12/13 Regression] ICE: in emit_block_move_hints, at expr.cc:1829 since r9-5509-g5928bc2ec06dd4e7

[rguenth at gcc dot gnu.org]

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

--- Comment #7 from Richard Biener <rguenth at gcc dot gnu.org> ---
(In reply to Martin Liška from comment #6)
> We fail in the param assignment:
> 
> (gdb) pp x
> (reg:V4DI 82)
> (gdb) pp y
> (mem/c:BLK (reg/f:DI 76 virtual-incoming-args) [1 x+0 S32 A256])
> 
> So we will likely need something similar to what we have in tree-inline.cc:
> 
>   5928    /* For vector typed decls make sure to update DECL_MODE according
>   5929       to the new function context.  */
>   5930    if (VECTOR_TYPE_P (TREE_TYPE (copy)))
>   5931      SET_DECL_MODE (copy, TYPE_MODE (TREE_TYPE (copy)));
> 
> @Richi: Do you have a clue where to adjust it?

I think it goes wrong in use_register_for_decl (called from
assign_parm_setup_block).

diff --git a/gcc/function.cc b/gcc/function.cc
index d975b001ec9..b54f9b33c6a 100644
--- a/gcc/function.cc
+++ b/gcc/function.cc
@@ -2229,7 +2229,9 @@ use_register_for_decl (const_tree decl)
     }

   /* Only register-like things go in registers.  */
-  if (DECL_MODE (decl) == BLKmode)
+  if (DECL_MODE (decl) == BLKmode
+      || (VECTOR_TYPE_P (TREE_TYPE (decl))
+         && TYPE_MODE (TREE_TYPE (decl)) == BLKmode))
     return false;

   /* If -ffloat-store specified, don't put explicit float variables

fixes the ICE, not sure if we should adjust the PARM_DECLs mode somewhere
in target cloning instead though?

[Bug target/105554] [10/11/12/13 Regression] ICE: in emit_block_move_hints, at expr.cc:1829 since r9-5509-g5928bc2ec06dd4e7

[rguenth at gcc dot gnu.org]

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

--- Comment #8 from Richard Biener <rguenth at gcc dot gnu.org> ---
(In reply to Richard Biener from comment #7)
> (In reply to Martin Liška from comment #6)
> > We fail in the param assignment:
> > 
> > (gdb) pp x
> > (reg:V4DI 82)
> > (gdb) pp y
> > (mem/c:BLK (reg/f:DI 76 virtual-incoming-args) [1 x+0 S32 A256])
> > 
> > So we will likely need something similar to what we have in tree-inline.cc:
> > 
> >   5928    /* For vector typed decls make sure to update DECL_MODE according
> >   5929       to the new function context.  */
> >   5930    if (VECTOR_TYPE_P (TREE_TYPE (copy)))
> >   5931      SET_DECL_MODE (copy, TYPE_MODE (TREE_TYPE (copy)));
> > 
> > @Richi: Do you have a clue where to adjust it?
> 
> I think it goes wrong in use_register_for_decl (called from
> assign_parm_setup_block).
> 
> diff --git a/gcc/function.cc b/gcc/function.cc
> index d975b001ec9..b54f9b33c6a 100644
> --- a/gcc/function.cc
> +++ b/gcc/function.cc
> @@ -2229,7 +2229,9 @@ use_register_for_decl (const_tree decl)
>      }
>  
>    /* Only register-like things go in registers.  */
> -  if (DECL_MODE (decl) == BLKmode)
> +  if (DECL_MODE (decl) == BLKmode
> +      || (VECTOR_TYPE_P (TREE_TYPE (decl))
> +         && TYPE_MODE (TREE_TYPE (decl)) == BLKmode))
>      return false;
>  
>    /* If -ffloat-store specified, don't put explicit float variables
> 
> fixes the ICE, not sure if we should adjust the PARM_DECLs mode somewhere
> in target cloning instead though?

Like in copy_arguments_nochange?

[Bug target/105554] [10/11/12/13 Regression] ICE: in emit_block_move_hints, at expr.cc:1829 since r9-5509-g5928bc2ec06dd4e7

[marxin at gcc dot gnu.org]

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

--- Comment #9 from Martin Liška <marxin at gcc dot gnu.org> ---
> fixes the ICE, not sure if we should adjust the PARM_DECLs mode somewhere
> in target cloning instead though?

We can do that in create_target_clone which calls
node->create_version_clone_with_body where a new function_decl is created.

However, the following code does not work:

  for (tree arg = DECL_ARGUMENTS (new_node->decl); arg; arg = DECL_CHAIN (arg))
    if (VECTOR_TYPE_P (TREE_TYPE (arg)))
      SET_DECL_MODE (arg, TYPE_MODE (TREE_TYPE (arg)));

I'm curious about what needs to be changed once we can get a proper MODE for
the new argument.

[Bug target/105554] [10/11/12/13 Regression] ICE: in emit_block_move_hints, at expr.cc:1829 since r9-5509-g5928bc2ec06dd4e7

[rguenther at suse dot de]

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

--- Comment #10 from rguenther at suse dot de <rguenther at suse dot de> ---
On Mon, 16 Jan 2023, marxin at gcc dot gnu.org wrote:

> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=105554
> 
> --- Comment #9 from Martin Liška <marxin at gcc dot gnu.org> ---
> > fixes the ICE, not sure if we should adjust the PARM_DECLs mode somewhere
> > in target cloning instead though?
> 
> We can do that in create_target_clone which calls
> node->create_version_clone_with_body where a new function_decl is created.
> 
> However, the following code does not work:
> 
>   for (tree arg = DECL_ARGUMENTS (new_node->decl); arg; arg = DECL_CHAIN (arg))
>     if (VECTOR_TYPE_P (TREE_TYPE (arg)))
>       SET_DECL_MODE (arg, TYPE_MODE (TREE_TYPE (arg)));
> 
> I'm curious about what needs to be changed once we can get a proper MODE for
> the new argument.

Well, most definitely the new decls target options need to be 
instantiated?

[Bug target/105554] [10/11/12/13 Regression] ICE: in emit_block_move_hints, at expr.cc:1829 since r9-5509-g5928bc2ec06dd4e7

[marxin at gcc dot gnu.org]

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

--- Comment #11 from Martin Liška <marxin at gcc dot gnu.org> ---
> 
> Well, most definitely the new decls target options need to be 
> instantiated?

Then it must be a tree pass that will properly call set_current_function :/ The
multiple_target pass is an IPA pass. Can we go for now with the comment 7?

[Bug target/105554] [10/11/12/13 Regression] ICE: in emit_block_move_hints, at expr.cc:1829 since r9-5509-g5928bc2ec06dd4e7

[rguenth at gcc dot gnu.org]

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

--- Comment #12 from Richard Biener <rguenth at gcc dot gnu.org> ---
(In reply to Martin Liška from comment #11)
> > 
> > Well, most definitely the new decls target options need to be 
> > instantiated?
> 
> Then it must be a tree pass that will properly call set_current_function :/
> The multiple_target pass is an IPA pass. Can we go for now with the comment
> 7?

Hmm, can we deal with this when the clones are materialized (that needs a cfun,
no?).

[Bug target/105554] [10/11/12/13 Regression] ICE: in emit_block_move_hints, at expr.cc:1829 since r9-5509-g5928bc2ec06dd4e7

[marxin at gcc dot gnu.org]

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

Martin Liška <marxin at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Assignee|marxin at gcc dot gnu.org          |unassigned at gcc dot gnu.org
             Status|ASSIGNED                    |NEW

[Bug target/105554] [10/11/12/13 Regression] ICE: in emit_block_move_hints, at expr.cc:1829 since r9-5509-g5928bc2ec06dd4e7

[jakub at gcc dot gnu.org]

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

--- Comment #13 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
A lot of ipa passes use push_cfun:
grep push_cfun ipa*.cc
ipa-fnsummary.cc:       push_cfun (DECL_STRUCT_FUNCTION (node->decl));
ipa-fnsummary.cc:  push_cfun (DECL_STRUCT_FUNCTION (node->decl));
ipa-modref.cc:      push_cfun (f);
ipa-modref.cc:  push_cfun (DECL_STRUCT_FUNCTION (node->decl));
ipa-modref.cc:  push_cfun (DECL_STRUCT_FUNCTION (node->decl));
ipa-prop.cc:  push_cfun (func);
ipa-pure-const.cc:  push_cfun (DECL_STRUCT_FUNCTION (decl));
ipa-sra.cc:      push_cfun (DECL_STRUCT_FUNCTION (node->decl));
ipa-sra.cc:       push_cfun (fun);
ipa-utils.cc:      push_cfun (dstcfun);
ipa-visibility.cc:            push_cfun (DECL_STRUCT_FUNCTION
(e->caller->decl));
ipa-visibility.cc:                    push_cfun (DECL_STRUCT_FUNCTION
(e->caller->decl));
After all, even create_target_clone -> create_version_clone_with_body ->
tree_function_versioning already called and and then pop_cfun () again.
multiple-target.cc is something so seldom used that another push_cfun/pop_cfun
around the newly proposed loop wouldn't be that bad.

[Bug target/105554] [10/11/12/13 Regression] ICE: in emit_block_move_hints, at expr.cc:1829 since r9-5509-g5928bc2ec06dd4e7

[jakub at gcc dot gnu.org]

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

--- Comment #14 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
So, I have tried
--- gcc/cgraphclones.cc.jj      2023-02-24 11:05:19.704595633 +0100
+++ gcc/cgraphclones.cc 2023-03-16 19:12:30.452503051 +0100
@@ -1094,6 +1094,15 @@ cgraph_node::create_version_clone_with_b
       || in_lto_p)
     new_version_node->unique_name = true;

+  if (target_attributes)
+    {
+      push_cfun (DECL_STRUCT_FUNCTION (new_decl));
+      for (tree arg = DECL_ARGUMENTS (new_decl); arg; arg = DECL_CHAIN (arg))
+       if (VECTOR_TYPE_P (TREE_TYPE (arg)))
+         SET_DECL_MODE (arg, TYPE_MODE (TREE_TYPE (arg)));
+      pop_cfun ();
+    }
+
   /* Update the call_expr on the edges to call the new version node. */
   update_call_expr (new_version_node);

but that really didn't help, the problem seems to be actually different.

From what I can see, tree_function_versioning does:
6240      DECL_RESULT (new_decl) = DECL_RESULT (old_decl);
6241      DECL_ARGUMENTS (new_decl) = DECL_ARGUMENTS (old_decl);
6242      initialize_cfun (new_decl, old_decl,
6243                       new_entry ? new_entry->count :
old_entry_block->count);
and initialize_cfun will call allocate_function, which does:
4845              /* Now that we have activated any function-specific
attributes
4846                 that might affect layout, particularly vector modes,
relayout
4847                 each of the parameters and the result.  */
4848              relayout_decl (result);
4849              for (tree parm = DECL_ARGUMENTS (fndecl); parm;
4850                   parm = DECL_CHAIN (parm))
4851                relayout_decl (parm);
So, we temporarily set DECL_ARGUMENTS and DECL_RESULT of new_decl to
arguments/result of old_decl and allocate_function called relayout_decl on
those, later on we create new arguments (which supposedly have correct
DECL_MODE).  But we've changed the old DECL_RESULT/DECL_ARGUMENTS.

[Bug target/105554] [10/11/12/13 Regression] ICE: in emit_block_move_hints, at expr.cc:1829 since r9-5509-g5928bc2ec06dd4e7

[jakub at gcc dot gnu.org]

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

--- Comment #15 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Created attachment 54686
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=54686&action=edit
gcc13-pr105554.patch

This untested patch seems to work.

[Bug target/105554] [10/11/12/13 Regression] ICE: in emit_block_move_hints, at expr.cc:1829 since r9-5509-g5928bc2ec06dd4e7

[rguenth at gcc dot gnu.org]

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

--- Comment #16 from Richard Biener <rguenth at gcc dot gnu.org> ---
(In reply to Jakub Jelinek from comment #14)
> So, I have tried
> --- gcc/cgraphclones.cc.jj	2023-02-24 11:05:19.704595633 +0100
> +++ gcc/cgraphclones.cc	2023-03-16 19:12:30.452503051 +0100
> @@ -1094,6 +1094,15 @@ cgraph_node::create_version_clone_with_b
>        || in_lto_p)
>      new_version_node->unique_name = true;
>  
> +  if (target_attributes)
> +    {
> +      push_cfun (DECL_STRUCT_FUNCTION (new_decl));
> +      for (tree arg = DECL_ARGUMENTS (new_decl); arg; arg = DECL_CHAIN
> (arg))
> +	if (VECTOR_TYPE_P (TREE_TYPE (arg)))
> +	  SET_DECL_MODE (arg, TYPE_MODE (TREE_TYPE (arg)));
> +      pop_cfun ();
> +    }
> +
>    /* Update the call_expr on the edges to call the new version node. */
>    update_call_expr (new_version_node);
>  
> but that really didn't help, the problem seems to be actually different.
> 
> From what I can see, tree_function_versioning does:
> 6240	  DECL_RESULT (new_decl) = DECL_RESULT (old_decl);
> 6241	  DECL_ARGUMENTS (new_decl) = DECL_ARGUMENTS (old_decl);
> 6242	  initialize_cfun (new_decl, old_decl,
> 6243			   new_entry ? new_entry->count : old_entry_block->count);
> and initialize_cfun will call allocate_function, which does:
> 4845		  /* Now that we have activated any function-specific attributes
> 4846		     that might affect layout, particularly vector modes, relayout
> 4847		     each of the parameters and the result.  */
> 4848		  relayout_decl (result);
> 4849		  for (tree parm = DECL_ARGUMENTS (fndecl); parm;
> 4850		       parm = DECL_CHAIN (parm))
> 4851		    relayout_decl (parm);
> So, we temporarily set DECL_ARGUMENTS and DECL_RESULT of new_decl to
> arguments/result of old_decl and allocate_function called relayout_decl on
> those, later on we create new arguments (which supposedly have correct
> DECL_MODE).  But we've changed the old DECL_RESULT/DECL_ARGUMENTS.

Ick - that's one of the worst place to do this ... what happens if we
postpone setting DECL_RESULT/DECL_ARGUMENTS to after initialize_cfun
(and remove the setting of them there)?

I suppose on its own this doesn't fix the original issue but possibly
clobbering the original function arg decls layouts looks wrong ...

[Bug target/105554] [10/11/12/13 Regression] ICE: in emit_block_move_hints, at expr.cc:1829 since r9-5509-g5928bc2ec06dd4e7

[jakub at gcc dot gnu.org]

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

--- Comment #17 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
(In reply to Richard Biener from comment #16)
> Ick - that's one of the worst place to do this ... what happens if we
> postpone setting DECL_RESULT/DECL_ARGUMENTS to after initialize_cfun
> (and remove the setting of them there)?

If DECL_RESULT is NULL, then allocate_struct_function will segfault.
I think if DECL_ARGUMENTS was NULL there, it would be ok with it.

But initialize_cfun starts with:
  if (!DECL_ARGUMENTS (new_fndecl))
    DECL_ARGUMENTS (new_fndecl) = DECL_ARGUMENTS (callee_fndecl);
  if (!DECL_RESULT (new_fndecl))
    DECL_RESULT (new_fndecl) = DECL_RESULT (callee_fndecl);
which makes it redundant with the only caller of it doing:
  DECL_RESULT (new_decl) = DECL_RESULT (old_decl);
  DECL_ARGUMENTS (new_decl) = DECL_ARGUMENTS (old_decl);
  initialize_cfun (new_decl, old_decl,
                   new_entry ? new_entry->count : old_entry_block->count);

[Bug target/105554] [10/11/12/13 Regression] ICE: in emit_block_move_hints, at expr.cc:1829 since r9-5509-g5928bc2ec06dd4e7

[jakub at gcc dot gnu.org]

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

Jakub Jelinek <jakub at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #54686|0                           |1
        is obsolete|                            |

--- Comment #18 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Created attachment 54689
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=54689&action=edit
gcc13-pr105554.patch

This also works.

[Bug target/105554] [10/11/12/13 Regression] ICE: in emit_block_move_hints, at expr.cc:1829 since r9-5509-g5928bc2ec06dd4e7

[rguenth at gcc dot gnu.org]

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

--- Comment #19 from Richard Biener <rguenth at gcc dot gnu.org> ---
Created attachment 54690
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=54690&action=edit
not working patch

So this one fails badly in ipa_param_body_adjustments::common_initialization
where it needs struct function.  Possibly it could be refactored to spit
out the get_new_param_chain early and doing the rest late.

I'm leaving the param_adjustments case untouched with a comment.

Let me test what I have.

[Bug target/105554] [10/11/12/13 Regression] ICE: in emit_block_move_hints, at expr.cc:1829 since r9-5509-g5928bc2ec06dd4e7

[jakub at gcc dot gnu.org]

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

Jakub Jelinek <jakub at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #54689|0                           |1
        is obsolete|                            |

--- Comment #20 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Created attachment 54691
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=54691&action=edit
gcc13-pr105554.patch

Another working version.

[Bug target/105554] [10/11/12/13 Regression] ICE: in emit_block_move_hints, at expr.cc:1829 since r9-5509-g5928bc2ec06dd4e7

[cvs-commit at gcc dot gnu.org]

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

--- Comment #21 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Jakub Jelinek <jakub@gcc.gnu.org>:

https://gcc.gnu.org/g:24c06560a7fa39049911eeb8777325d112e0deb9

commit r13-6739-g24c06560a7fa39049911eeb8777325d112e0deb9
Author: Jakub Jelinek <jakub@redhat.com>
Date:   Fri Mar 17 18:59:56 2023 +0100

    tree-inline: Fix up multiversioning with vector arguments [PR105554]

    The following testcase ICEs, because we call tree_function_versioning from
    old_decl which has target attributes not supporting V4DImode and so
    DECL_MODE of DECL_ARGUMENTS is BLKmode, while new_decl supports those.
    tree_function_versioning initially copies DECL_RESULT and DECL_ARGUMENTS
    from old_decl to new_decl, then calls initialize_cfun to create cfun
    and only when the cfun is created it can later actually remap_decl
    DECL_RESULT and DECL_ARGUMENTS etc.
    The problem is that initialize_cfun -> push_struct_function ->
    allocate_struct_function calls relayout_decl on DECL_RESULT and
    DECL_ARGUMENTS, which clobbers DECL_MODE of old_decl and we then ICE
because
    of it.
    In particular, allocate_struct_function does:
          if (!abstract_p)
            {
              /* Now that we have activated any function-specific attributes
                 that might affect layout, particularly vector modes, relayout
                 each of the parameters and the result.  */
              relayout_decl (result);
              for (tree parm = DECL_ARGUMENTS (fndecl); parm;
                   parm = DECL_CHAIN (parm))
                relayout_decl (parm);

              /* Similarly relayout the function decl.  */
              targetm.target_option.relayout_function (fndecl);
            }

          if (!abstract_p && aggregate_value_p (result, fndecl))
            {
     #ifdef PCC_STATIC_STRUCT_RETURN
              cfun->returns_pcc_struct = 1;
     #endif
              cfun->returns_struct = 1;
            }
    Now, in the case of tree_function_versioning, I believe all that we need
    from these is possibly the
    targetm.target_option.relayout_function (fndecl);
    call (arm only), we will remap DECL_RESULT and DECL_ARGUMENTS later on
    and copy_decl_for_dup_finish in that case will handle all we need:
      /* For vector typed decls make sure to update DECL_MODE according
         to the new function context.  */
      if (VECTOR_TYPE_P (TREE_TYPE (copy)))
        SET_DECL_MODE (copy, TYPE_MODE (TREE_TYPE (copy)));
    We don't need the cfun->returns_*struct either, because we override it
    in initialize_cfun a few lines later:
      /* Copy items we preserve during cloning.  */
    ...
      cfun->returns_struct = src_cfun->returns_struct;
      cfun->returns_pcc_struct = src_cfun->returns_pcc_struct;

    So, to avoid the clobbering of DECL_RESULT/DECL_ARGUMENTS of old_decl,
    the following patch arranges allocate_struct_function to be called with
    abstract_p true and calls targetm.target_option.relayout_function (fndecl);
    by hand.

    The removal of DECL_RESULT/DECL_ARGUMENTS copying at the start of
    initialize_cfun is removed because the only caller -
    tree_function_versioning, does that unconditionally before.

    2023-03-17  Jakub Jelinek  <jakub@redhat.com>

            PR target/105554
            * function.h (push_struct_function): Add ABSTRACT_P argument
defaulted
            to false.
            * function.cc (push_struct_function): Add ABSTRACT_P argument, pass
it
            to allocate_struct_function instead of false.
            * tree-inline.cc (initialize_cfun): Don't copy DECL_ARGUMENTS
            nor DECL_RESULT here.  Pass true as ABSTRACT_P to
            push_struct_function.  Call targetm.target_option.relayout_function
            after it.
            (tree_function_versioning): Formatting fix.

            * gcc.target/i386/pr105554.c: New test.

[Bug target/105554] [10/11/12 Regression] ICE: in emit_block_move_hints, at expr.cc:1829 since r9-5509-g5928bc2ec06dd4e7

[jakub at gcc dot gnu.org]

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

Jakub Jelinek <jakub at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Assignee|unassigned at gcc dot gnu.org      |jakub at gcc dot gnu.org
             Status|NEW                         |ASSIGNED
            Summary|[10/11/12/13 Regression]    |[10/11/12 Regression] ICE:
                   |ICE: in                     |in emit_block_move_hints,
                   |emit_block_move_hints, at   |at expr.cc:1829 since
                   |expr.cc:1829 since          |r9-5509-g5928bc2ec06dd4e7
                   |r9-5509-g5928bc2ec06dd4e7   |

--- Comment #22 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Fixed on the trunk so far.

[Bug target/105554] [10/11/12 Regression] ICE: in emit_block_move_hints, at expr.cc:1829 since r9-5509-g5928bc2ec06dd4e7

[cvs-commit at gcc dot gnu.org]

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

--- Comment #23 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The releases/gcc-12 branch has been updated by Jakub Jelinek
<jakub@gcc.gnu.org>:

https://gcc.gnu.org/g:b2747a4213635352f8315114c5925f178fed0544

commit r12-9293-gb2747a4213635352f8315114c5925f178fed0544
Author: Jakub Jelinek <jakub@redhat.com>
Date:   Fri Mar 17 18:59:56 2023 +0100

    tree-inline: Fix up multiversioning with vector arguments [PR105554]

    The following testcase ICEs, because we call tree_function_versioning from
    old_decl which has target attributes not supporting V4DImode and so
    DECL_MODE of DECL_ARGUMENTS is BLKmode, while new_decl supports those.
    tree_function_versioning initially copies DECL_RESULT and DECL_ARGUMENTS
    from old_decl to new_decl, then calls initialize_cfun to create cfun
    and only when the cfun is created it can later actually remap_decl
    DECL_RESULT and DECL_ARGUMENTS etc.
    The problem is that initialize_cfun -> push_struct_function ->
    allocate_struct_function calls relayout_decl on DECL_RESULT and
    DECL_ARGUMENTS, which clobbers DECL_MODE of old_decl and we then ICE
because
    of it.
    In particular, allocate_struct_function does:
          if (!abstract_p)
            {
              /* Now that we have activated any function-specific attributes
                 that might affect layout, particularly vector modes, relayout
                 each of the parameters and the result.  */
              relayout_decl (result);
              for (tree parm = DECL_ARGUMENTS (fndecl); parm;
                   parm = DECL_CHAIN (parm))
                relayout_decl (parm);

              /* Similarly relayout the function decl.  */
              targetm.target_option.relayout_function (fndecl);
            }

          if (!abstract_p && aggregate_value_p (result, fndecl))
            {
     #ifdef PCC_STATIC_STRUCT_RETURN
              cfun->returns_pcc_struct = 1;
     #endif
              cfun->returns_struct = 1;
            }
    Now, in the case of tree_function_versioning, I believe all that we need
    from these is possibly the
    targetm.target_option.relayout_function (fndecl);
    call (arm only), we will remap DECL_RESULT and DECL_ARGUMENTS later on
    and copy_decl_for_dup_finish in that case will handle all we need:
      /* For vector typed decls make sure to update DECL_MODE according
         to the new function context.  */
      if (VECTOR_TYPE_P (TREE_TYPE (copy)))
        SET_DECL_MODE (copy, TYPE_MODE (TREE_TYPE (copy)));
    We don't need the cfun->returns_*struct either, because we override it
    in initialize_cfun a few lines later:
      /* Copy items we preserve during cloning.  */
    ...
      cfun->returns_struct = src_cfun->returns_struct;
      cfun->returns_pcc_struct = src_cfun->returns_pcc_struct;

    So, to avoid the clobbering of DECL_RESULT/DECL_ARGUMENTS of old_decl,
    the following patch arranges allocate_struct_function to be called with
    abstract_p true and calls targetm.target_option.relayout_function (fndecl);
    by hand.

    The removal of DECL_RESULT/DECL_ARGUMENTS copying at the start of
    initialize_cfun is removed because the only caller -
    tree_function_versioning, does that unconditionally before.

    2023-03-17  Jakub Jelinek  <jakub@redhat.com>

            PR target/105554
            * function.h (push_struct_function): Add ABSTRACT_P argument
defaulted
            to false.
            * function.cc (push_struct_function): Add ABSTRACT_P argument, pass
it
            to allocate_struct_function instead of false.
            * tree-inline.cc (initialize_cfun): Don't copy DECL_ARGUMENTS
            nor DECL_RESULT here.  Pass true as ABSTRACT_P to
            push_struct_function.  Call targetm.target_option.relayout_function
            after it.
            (tree_function_versioning): Formatting fix.

            * gcc.target/i386/pr105554.c: New test.

    (cherry picked from commit 24c06560a7fa39049911eeb8777325d112e0deb9)

[Bug target/105554] [10/11 Regression] ICE: in emit_block_move_hints, at expr.cc:1829 since r9-5509-g5928bc2ec06dd4e7

[jakub at gcc dot gnu.org]

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

Jakub Jelinek <jakub at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
            Summary|[10/11/12 Regression] ICE:  |[10/11 Regression] ICE: in
                   |in emit_block_move_hints,   |emit_block_move_hints, at
                   |at expr.cc:1829 since       |expr.cc:1829 since
                   |r9-5509-g5928bc2ec06dd4e7   |r9-5509-g5928bc2ec06dd4e7

--- Comment #24 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Fixed for 12.3 too.

[Bug target/105554] [10/11 Regression] ICE: in emit_block_move_hints, at expr.cc:1829 since r9-5509-g5928bc2ec06dd4e7

[cvs-commit at gcc dot gnu.org]

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

--- Comment #25 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The releases/gcc-11 branch has been updated by Jakub Jelinek
<jakub@gcc.gnu.org>:

https://gcc.gnu.org/g:40a9f3d335da7e1977beff2357afac39c500650f

commit r11-10727-g40a9f3d335da7e1977beff2357afac39c500650f
Author: Jakub Jelinek <jakub@redhat.com>
Date:   Fri Mar 17 18:59:56 2023 +0100

    tree-inline: Fix up multiversioning with vector arguments [PR105554]

    The following testcase ICEs, because we call tree_function_versioning from
    old_decl which has target attributes not supporting V4DImode and so
    DECL_MODE of DECL_ARGUMENTS is BLKmode, while new_decl supports those.
    tree_function_versioning initially copies DECL_RESULT and DECL_ARGUMENTS
    from old_decl to new_decl, then calls initialize_cfun to create cfun
    and only when the cfun is created it can later actually remap_decl
    DECL_RESULT and DECL_ARGUMENTS etc.
    The problem is that initialize_cfun -> push_struct_function ->
    allocate_struct_function calls relayout_decl on DECL_RESULT and
    DECL_ARGUMENTS, which clobbers DECL_MODE of old_decl and we then ICE
because
    of it.
    In particular, allocate_struct_function does:
          if (!abstract_p)
            {
              /* Now that we have activated any function-specific attributes
                 that might affect layout, particularly vector modes, relayout
                 each of the parameters and the result.  */
              relayout_decl (result);
              for (tree parm = DECL_ARGUMENTS (fndecl); parm;
                   parm = DECL_CHAIN (parm))
                relayout_decl (parm);

              /* Similarly relayout the function decl.  */
              targetm.target_option.relayout_function (fndecl);
            }

          if (!abstract_p && aggregate_value_p (result, fndecl))
            {
     #ifdef PCC_STATIC_STRUCT_RETURN
              cfun->returns_pcc_struct = 1;
     #endif
              cfun->returns_struct = 1;
            }
    Now, in the case of tree_function_versioning, I believe all that we need
    from these is possibly the
    targetm.target_option.relayout_function (fndecl);
    call (arm only), we will remap DECL_RESULT and DECL_ARGUMENTS later on
    and copy_decl_for_dup_finish in that case will handle all we need:
      /* For vector typed decls make sure to update DECL_MODE according
         to the new function context.  */
      if (VECTOR_TYPE_P (TREE_TYPE (copy)))
        SET_DECL_MODE (copy, TYPE_MODE (TREE_TYPE (copy)));
    We don't need the cfun->returns_*struct either, because we override it
    in initialize_cfun a few lines later:
      /* Copy items we preserve during cloning.  */
    ...
      cfun->returns_struct = src_cfun->returns_struct;
      cfun->returns_pcc_struct = src_cfun->returns_pcc_struct;

    So, to avoid the clobbering of DECL_RESULT/DECL_ARGUMENTS of old_decl,
    the following patch arranges allocate_struct_function to be called with
    abstract_p true and calls targetm.target_option.relayout_function (fndecl);
    by hand.

    The removal of DECL_RESULT/DECL_ARGUMENTS copying at the start of
    initialize_cfun is removed because the only caller -
    tree_function_versioning, does that unconditionally before.

    2023-03-17  Jakub Jelinek  <jakub@redhat.com>

            PR target/105554
            * function.h (push_struct_function): Add ABSTRACT_P argument
defaulted
            to false.
            * function.c (push_struct_function): Add ABSTRACT_P argument, pass
it
            to allocate_struct_function instead of false.
            * tree-inline.c (initialize_cfun): Don't copy DECL_ARGUMENTS
            nor DECL_RESULT here.  Pass true as ABSTRACT_P to
            push_struct_function.  Call targetm.target_option.relayout_function
            after it.
            (tree_function_versioning): Formatting fix.

            * gcc.target/i386/pr105554.c: New test.

    (cherry picked from commit 24c06560a7fa39049911eeb8777325d112e0deb9)

[Bug target/105554] [10 Regression] ICE: in emit_block_move_hints, at expr.cc:1829 since r9-5509-g5928bc2ec06dd4e7

[jakub at gcc dot gnu.org]

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

Jakub Jelinek <jakub at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
            Summary|[10/11 Regression] ICE: in  |[10 Regression] ICE: in
                   |emit_block_move_hints, at   |emit_block_move_hints, at
                   |expr.cc:1829 since          |expr.cc:1829 since
                   |r9-5509-g5928bc2ec06dd4e7   |r9-5509-g5928bc2ec06dd4e7

--- Comment #26 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Fixed for 11.4 as well.

[Bug target/105554] [10 Regression] ICE: in emit_block_move_hints, at expr.cc:1829 since r9-5509-g5928bc2ec06dd4e7

[cvs-commit at gcc dot gnu.org]

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

--- Comment #27 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The releases/gcc-10 branch has been updated by Jakub Jelinek
<jakub@gcc.gnu.org>:

https://gcc.gnu.org/g:06b34c0d76153549fb83001c3d90b798066a1851

commit r10-11380-g06b34c0d76153549fb83001c3d90b798066a1851
Author: Jakub Jelinek <jakub@redhat.com>
Date:   Fri Mar 17 18:59:56 2023 +0100

    tree-inline: Fix up multiversioning with vector arguments [PR105554]

    The following testcase ICEs, because we call tree_function_versioning from
    old_decl which has target attributes not supporting V4DImode and so
    DECL_MODE of DECL_ARGUMENTS is BLKmode, while new_decl supports those.
    tree_function_versioning initially copies DECL_RESULT and DECL_ARGUMENTS
    from old_decl to new_decl, then calls initialize_cfun to create cfun
    and only when the cfun is created it can later actually remap_decl
    DECL_RESULT and DECL_ARGUMENTS etc.
    The problem is that initialize_cfun -> push_struct_function ->
    allocate_struct_function calls relayout_decl on DECL_RESULT and
    DECL_ARGUMENTS, which clobbers DECL_MODE of old_decl and we then ICE
because
    of it.
    In particular, allocate_struct_function does:
          if (!abstract_p)
            {
              /* Now that we have activated any function-specific attributes
                 that might affect layout, particularly vector modes, relayout
                 each of the parameters and the result.  */
              relayout_decl (result);
              for (tree parm = DECL_ARGUMENTS (fndecl); parm;
                   parm = DECL_CHAIN (parm))
                relayout_decl (parm);

              /* Similarly relayout the function decl.  */
              targetm.target_option.relayout_function (fndecl);
            }

          if (!abstract_p && aggregate_value_p (result, fndecl))
            {
     #ifdef PCC_STATIC_STRUCT_RETURN
              cfun->returns_pcc_struct = 1;
     #endif
              cfun->returns_struct = 1;
            }
    Now, in the case of tree_function_versioning, I believe all that we need
    from these is possibly the
    targetm.target_option.relayout_function (fndecl);
    call (arm only), we will remap DECL_RESULT and DECL_ARGUMENTS later on
    and copy_decl_for_dup_finish in that case will handle all we need:
      /* For vector typed decls make sure to update DECL_MODE according
         to the new function context.  */
      if (VECTOR_TYPE_P (TREE_TYPE (copy)))
        SET_DECL_MODE (copy, TYPE_MODE (TREE_TYPE (copy)));
    We don't need the cfun->returns_*struct either, because we override it
    in initialize_cfun a few lines later:
      /* Copy items we preserve during cloning.  */
    ...
      cfun->returns_struct = src_cfun->returns_struct;
      cfun->returns_pcc_struct = src_cfun->returns_pcc_struct;

    So, to avoid the clobbering of DECL_RESULT/DECL_ARGUMENTS of old_decl,
    the following patch arranges allocate_struct_function to be called with
    abstract_p true and calls targetm.target_option.relayout_function (fndecl);
    by hand.

    The removal of DECL_RESULT/DECL_ARGUMENTS copying at the start of
    initialize_cfun is removed because the only caller -
    tree_function_versioning, does that unconditionally before.

    2023-03-17  Jakub Jelinek  <jakub@redhat.com>

            PR target/105554
            * function.h (push_struct_function): Add ABSTRACT_P argument
defaulted
            to false.
            * function.c (push_struct_function): Add ABSTRACT_P argument, pass
it
            to allocate_struct_function instead of false.
            * tree-inline.c (initialize_cfun): Don't copy DECL_ARGUMENTS
            nor DECL_RESULT here.  Pass true as ABSTRACT_P to
            push_struct_function.  Call targetm.target_option.relayout_function
            after it.
            (tree_function_versioning): Formatting fix.

            * gcc.target/i386/pr105554.c: New test.

    (cherry picked from commit 24c06560a7fa39049911eeb8777325d112e0deb9)

[Bug target/105554] [10 Regression] ICE: in emit_block_move_hints, at expr.cc:1829 since r9-5509-g5928bc2ec06dd4e7

[jakub at gcc dot gnu.org]

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

Jakub Jelinek <jakub at gcc dot gnu.org> changed:

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

--- Comment #28 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Fixed for 10.5 too.