> -----Original Message-----
> From: Matthew Fortune [mailto:matthew.fort...@imgtec.com]
> Sent: Wednesday, March 15, 2017 11:30 AM
> To: Moore, Catherine <catherine_mo...@mentor.com>
> Cc: Segher Boessenkool (seg...@kernel.crashing.org)
> <seg...@kernel.crashing.org>; Toma Tabacu
> <toma.tab...@imgtec.com>; gcc-patches@gcc.gnu.org
> Subject: RE: [PATCH] Fix MIPS-specific ICE in gcc.dg/pr77834.c (PR rtl-
> optimization/79150).
> 
> 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.
> 
I'm okay with the workaround for stage 4, but would like to see the pr remain 
open until a proper fix is installed on trunk. 
Toma, would you be able to pursue the original patch that you attached to the 
bug report?

Catherine
> 
> > -----Original Message-----
> > From: Toma Tabacu
> > Sent: 07 March 2017 11:44
> > To: gcc-patches@gcc.gnu.org
> > Cc: Matthew Fortune; Segher Boessenkool
> (seg...@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

Reply via email to