RE: [PATCH] Fix MIPS-specific ICE in gcc.dg/pr77834.c (PR rtl-optimization/79150).

[Matthew Fortune <Matthew dot Fortune at imgtec dot com>]

Hi Catherine,

What is your opinion on this patch? I am OK with the temporary
workaround on the basis that the additional nop is successfully
eliminated so there is no code size increase. Also, I am happy
enough that the CFG is fixed very soon after the block move is
expanded so the 'bad' basic block is fixed almost immediately
anyway making the offending check potentially unnecessary in
the first place.

Thanks,
Matthew

> -----Original Message-----
> From: Toma Tabacu
> Sent: 07 March 2017 11:44
> To: gcc-patches@gcc.gnu.org
> Cc: Matthew Fortune; Segher Boessenkool (segher@kernel.crashing.org)
> Subject: [PATCH] Fix MIPS-specific ICE in gcc.dg/pr77834.c (PR rtl-
> optimization/79150).
> 
> Hi,
> 
> This ICE is caused by "gcc_assert (!JUMP_P (last))" in
> commit_one_edge_insertion (gcc/cfgrtl.c):
> 
>   if (returnjump_p (last))
>     {
>       /* ??? Remove all outgoing edges from BB and add one for EXIT.
> 	 This is not currently a problem because this only happens
> 	 for the (single) epilogue, which already has a fallthru edge
> 	 to EXIT.  */
> 
>       e = single_succ_edge (bb);
>       gcc_assert (e->dest == EXIT_BLOCK_PTR_FOR_FN (cfun)
> 		  && single_succ_p (bb) && (e->flags & EDGE_FALLTHRU));
> 
>       e->flags &= ~EDGE_FALLTHRU;
>       emit_barrier_after (last);
> 
>       if (before)
> 	delete_insn (before);
>     }
>   else
>     gcc_assert (!JUMP_P (last));
> 
> The assert is triggered because we're removing an edge which ends on a
> conditional jump, which is part of a loop.
> 
> The reason for the existence of this loop is the size of the vector used
> in gcc.dg/pr77834.c.
> When compiling code which uses vectors for MIPS, a looping memcpy is
> generated if the vectors are big enough (>32 bytes for MIPS32 or >64
> bytes for MIPS64).
> This takes place in mips_expand_block_move (gcc/config/mips.c).
> 
> In our case, a looping memcpy gets generated for a partition copy which
> is inserted on an edge (in insert_partition_copy_on_edge, gcc/tree-
> outof-ssa.c).
> This happens during PHI node elimination, which is done by eliminate_phi
> (gcc/tree-outof-ssa.c), as a part of expand_phi_nodes, which is called
> by the expand pass (gcc/cfgexpand.c).
> 
> My original fix was to update the CFG by calling
> find_many_sub_basic_blocks with an all-one block bitmap (which also
> happens in cfgexpand.c, after edge
> removal) whenever an edge contains an insn which satisfies
> control_flow_insn_p.
> However, Segher Boessenkool said that this would be too risky for stage
> 4 and suggested inserting a NOP after the conditional jump in order to
> fool the assert. This assumes that it is safe to delay the CFG update
> for this particular case.
> 
> This patch changes mips_block_move_loop to emit a NOP after the
> conditional jump, if the jump is the last insn of the loop. This
> prevents "gcc_assert (!JUMP_P (last))" from triggering.
> 
> The NOP will never make it into the final assembly code because it is
> removed during the cse1 pass through DCE for -O1, -O2, and -O3, and it's
> not even emitted for -O0 and -Os because looping memcpy's are not
> generated for those optimization levels, as can be seen in
> mips_expand_block_move from mips.c:
> 
>       if (INTVAL (length) <= MIPS_MAX_MOVE_BYTES_STRAIGHT)
> 	{
> 	  mips_block_move_straight (dest, src, INTVAL (length));
> 	  return true;
> 	}
>       else if (optimize)
> 	{
> 	  mips_block_move_loop (dest, src, INTVAL (length),
> 				MIPS_MAX_MOVE_BYTES_PER_LOOP_ITER);
> 	  return true;
> 	}
> 
> Tested with mips-mti-elf.
> 
> Regards,
> Toma Tabacu
> 
> gcc/
> 
> 	* config/mips/mips.c (mips_block_move_loop): Emit a NOP after the
> 	conditional jump, if the jump is the last insn of the loop.
> 
> diff --git a/gcc/config/mips/mips.c b/gcc/config/mips/mips.c index
> 4e13fbe..43e719f 100644
> --- a/gcc/config/mips/mips.c
> +++ b/gcc/config/mips/mips.c
> @@ -8098,6 +8098,9 @@ mips_block_move_loop (rtx dest, rtx src,
> HOST_WIDE_INT length,
>    /* Mop up any left-over bytes.  */
>    if (leftover)
>      mips_block_move_straight (dest, src, leftover);
> +  else
> +    /* Temporary fix for PR79150.  */
> +    emit_insn (gen_nop ());
>  }
> 
>  /* Expand a movmemsi instruction, which copies LENGTH bytes from