> -----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