Hmmm... I can't seem to reproduce it with current trunk and 4.8.2

After trying out some option combinations, I managed to reproduce it on 4.8 and trunk. For me it ICEs with -O2, but works with -O3 _unless_ I add -fPIC, in which case it ICEs.

cond_exec_process_insns is throwing a wobbly, since it is not expecting to handle a jump insn (in this case a return).

(jump_insn 62 61 51 4 (parallel [
            (return)
            (set/f (reg/f:SI 13 sp)
                (plus:SI (reg/f:SI 13 sp)
                    (const_int 16 [0x10])))
            (set/f (reg:SI 3 r3)
                (mem/c:SI (reg/f:SI 13 sp) [25 S4 A32]))
            (set/f (reg:SI 4 r4)
                (mem/c:SI (plus:SI (reg/f:SI 13 sp)
                        (const_int 4 [0x4])) [25 S4 A32]))
            (set/f (reg:SI 5 r5)
                (mem/c:SI (plus:SI (reg/f:SI 13 sp)
                        (const_int 8 [0x8])) [25 S4 A32]))
            (set/f (reg:SI 15 pc)
                (mem/c:SI (plus:SI (reg/f:SI 13 sp)
                        (const_int 12 [0xc])) [25 S4 A32]))
        ]) image.c:1709 -1
     (expr_list:REG_UNUSED (reg:SI 15 pc)
        (expr_list:REG_UNUSED (reg:SI 3 r3)
            (expr_list:REG_CFA_RESTORE (reg:SI 5 r5)
                (expr_list:REG_CFA_RESTORE (reg:SI 4 r4)
                    (expr_list:REG_CFA_RESTORE (reg:SI 3 r3)
                        (nil))))))
 -> return)

Seems to work with 4.7, so a regression.

Reduced testcase, ICEs with -march=armv7-a -mthumb -O2:

void *fn1 (void *);
void *fn2 (void *, const char *);
void fn3 (void *);
void fn4 (void *, int);

void *
test (void *x)
{
  void *a, *b;
  if (!(a = fn1 (x)))
    return (void *) 0;
  if (!(b = fn2 (a, "w")))
    {
      fn3 (a);
      return (void *) 0;
    }
  fn3 (a);
  fn4 (b, 1);
  return b;
}

I think the problem is that ifcvt relies on consistent counting of insns, but the various functions count different things.
- count_bb_insns counts CALL_INSN and INSN
- flow_find_cross_jump counts in some cases the jump insn (quite complicated test whether it counts it or not), and doesn't count insns with USE or CLOBBER PATTERNs
- flow_find_head_matching_sequence counts all CALL_INSN/INSN, including USE and CLOBBER
- first_active_insn finds what count_bb_insns counts
- last_active_insn (..., TRUE) finds what count_bb_insns except it skips over USE patterns

I guess best would be to count/skip/etc. the same things consistently, the problem is that some of the functions have other uses etc.

So, perhaps
1) let count_bb_insns not count insns with USE or CLOBBER PATTERNs
2) perhaps not count any JUMP_INSNs in flow_find_cross_jump if dir_p == NULL (i.e.
when called from ifcvt)?
3) perhaps not count USE/CLOBBER insns in flow_find_head_matching_sequence if
stop_after is non-zero?
4) perhaps add also skip_use argument to first_active_insn and if TRUE, ignore USE insns and for both {first,last}_active_insn if skip_use is TRUE, also
ignore CLOBBER insns
5) in find_active_insn_{before,after} ignore USE/CLOBBER insns
and document this properly?

> I think the problem is that ifcvt relies on consistent counting of insns,
> but the various functions count different things.

What kind of insns is responsible for the discrepancy that leads to the ICE?

> I guess best would be to count/skip/etc. the same things consistently, the
> problem is that some of the functions have other uses etc.

In any case, this would be a sensible approach.

> 1) let count_bb_insns not count insns with USE or CLOBBER PATTERNs

Agreed.

> 2) perhaps not count any JUMP_INSNs in flow_find_cross_jump if dir_p == NULL
> (i.e.
> when called from ifcvt)?
> 3) perhaps not count USE/CLOBBER insns in flow_find_head_matching_sequence if
> stop_after is non-zero?

I'd first make the functions behave the same wrt USE and CLOBBER insns.

> 4) perhaps add also skip_use argument to first_active_insn and if TRUE,
> ignore USE insns and for both {first,last}_active_insn if skip_use is TRUE,
> also ignore CLOBBER insns
>
> 5) in find_active_insn_{before,after} ignore USE/CLOBBER insns
> and document this properly?

I'm less sure about these ones: does their behavior need to be in keeping with the insns counting?

(In reply to Eric Botcazou from comment #6)
> > I think the problem is that ifcvt relies on consistent counting of insns,
> > but the various functions count different things.
> 
> What kind of insns is responsible for the discrepancy that leads to the ICE?

On the given testcase the JUMP_INSN at the end of bb is returnjump_p (without additional side effects), so it isn't counted and thus that problem isn't present.  But the problem on the testcase is insn with USE pattern right before the jump, count_bb_insns counts it as active insn, flow_find_cross_jump doesn't, and we subtract from the former the latter to see how many insns we should allow at most for flow_find_head_matching_sequence, and because of the discrepancy it is one bigger than it should and we end up with
then_last_head being after then_end, which violates the assumptions the code makes.

> > 3) perhaps not count USE/CLOBBER insns in flow_find_head_matching_sequence if
> > stop_after is non-zero?
> 
> I'd first make the functions behave the same wrt USE and CLOBBER insns.

Perhaps we can ignore those always in flow_find_head_matching_sequence?
 
> > 4) perhaps add also skip_use argument to first_active_insn and if TRUE,
> > ignore USE insns and for both {first,last}_active_insn if skip_use is TRUE,
> > also ignore CLOBBER insns
> >
> > 5) in find_active_insn_{before,after} ignore USE/CLOBBER insns
> > and document this properly?
> 
> I'm less sure about these ones: does their behavior need to be in keeping
> with the insns counting?

Perhaps, though I'd say it might be a ticking bomb.

> Perhaps we can ignore those always in flow_find_head_matching_sequence?

Yes, that seems to be the most sensible thing to do, so that count_bb_insns, flow_find_cross_jump and flow_find_head_matching_sequence agree about them.

> Perhaps, though I'd say it might be a ticking bomb.

If the bomb would be an ICE, that's good enough for now in my opinion.

Created attachment 31466 [details]
gcc49-pr58668.patch

So like this?  Note, the USE/CLOBBER change for flow_find_cross_jump
has been added in 2011 for 4.7 as PR43920 fix: http://gcc.gnu.org/ml/gcc-patches/2011-03/msg02246.html

> So like this?  Note, the USE/CLOBBER change for flow_find_cross_jump
> has been added in 2011 for 4.7 as PR43920 fix:
> http://gcc.gnu.org/ml/gcc-patches/2011-03/msg02246.html

Yes, let's try that, but use the same idiom in flow_find_cross_jump as yours (i.e. remove the useless p1) added by the above fix so that your patch will
have an explicit link to the above fix.

Author: jakub
Date: Tue Jan  7 07:54:47 2014
New Revision: 206385

URL: http://gcc.gnu.org/viewcvs?rev=206385&root=gcc&view=rev
Log:
	PR rtl-optimization/58668
	* cfgcleanup.c (flow_find_cross_jump): Don't count
	any jumps if dir_p is NULL.  Remove p1 variable, use active_insn_p
	to determine what is counted.
	(flow_find_head_matching_sequence): Use active_insn_p to determine
	what is counted.
	(try_head_merge_bb): Adjust for the flow_find_head_matching_sequence
	counting change.
	* ifcvt.c (count_bb_insns): Use active_insn_p && !JUMP_P to
	determine what is counted.

	* gcc.dg/pr58668.c: New test.

Added:
    trunk/gcc/testsuite/gcc.dg/pr58668.c
Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/cfgcleanup.c
    trunk/gcc/ifcvt.c
    trunk/gcc/testsuite/ChangeLog

Author: jakub
Date: Tue Jan  7 16:51:16 2014
New Revision: 206398

URL: http://gcc.gnu.org/viewcvs?rev=206398&root=gcc&view=rev
Log:
	PR rtl-optimization/58668
	* cfgcleanup.c (flow_find_cross_jump): Don't count
	any jumps if dir_p is NULL.  Remove p1 variable and make USE/CLOBBER
	check consistent with other places.
	(flow_find_head_matching_sequence): Don't count USE or CLOBBER insns.
	(try_head_merge_bb): Adjust for the flow_find_head_matching_sequence
	counting change.
	* ifcvt.c (count_bb_insns): Don't count USE or CLOBBER insns.

	* gcc.dg/pr58668.c: New test.

Added:
    branches/gcc-4_8-branch/gcc/testsuite/gcc.dg/pr58668.c
Modified:
    branches/gcc-4_8-branch/gcc/ChangeLog
    branches/gcc-4_8-branch/gcc/cfgcleanup.c
    branches/gcc-4_8-branch/gcc/ifcvt.c
    branches/gcc-4_8-branch/gcc/testsuite/ChangeLog

Hopefully fixed.