Re: gcse.c patch breaks simd-2.c

[Roger Sayle <roger at www dot eyesopen dot com>]

Hi Aldy,
> After your gcse.c change, compilation is ICEing with:
>
> 	internal compiler error: in reg_overlap_mentioned_p, at rtlanal.c:1635
> 	Please submit a full bug report,
>
> I thought the problem was mine because of the SIMD changes I made the
> same day, but after playing CVS games, I narrowed the problem to the
> gcse.c changes.
>
> Could you take a look at this?

Certainly.  Obviously, my patch which reduces the number of jumps we
redirect in GCSE is not directly responsible for ICE trying to handle
subregs of vector register modes in combine, but one can see how GCSE
changes can expose or hide different instruction sequences for combine.


>From my analysis so far, I believe the bug is on or around line 1093
of combine.c in the function can_combine_p.  This is handed the insn:

(insn 152 151 153 0 (nil) (set (zero_extract:SI (subreg:SI (reg:V8HI 135) 12)
            (const_int 16 [0x10])
            (const_int 16 [0x10]))
        (reg:SI 215)) 106 {insvsi} (insn_list 148 (insn_list 151 (nil)))
    (expr_list:REG_DEAD (reg:SI 215)
        (nil)))


Unfortunately, not understanding GCC's vector modes I'm not sure if
this instruction is valid or not (or even what it means), but given
it appears in break.i.00.rtl, I presume its OK.

Anyway, the code in can_combine_p recognizes that this is indeed a
set, and extracts the source, "src" and destination "dest".  But
it looks as though combine assumes that "dest" is always a register:

      /* Make sure that DEST is not used after SUCC but before I3.  */
      || (succ && ! all_adjacent
          && reg_used_between_p (dest, succ, i3))

Clearly, dest isn't a simple register:

(zero_extract:SI (subreg:SI (reg:V8HI 135) 12)
    (const_int 16 [0x10])
    (const_int 16 [0x10]))

And hence reg_used_between eventually fails painfully in abort() called
from reg_overlap_mentioned_p.


The solution is either (1) strengthen the tests in can_combine_p, perhaps
as simple as adding the following line immediately before the above test:

	/* DEST must be either a REG or CC0.  */
	|| ! (REG_P (dest) || CC0_P (dest))

[The logic for this can be seen on line 1122 of combine.c]


and/or (2) add support for ZERO_EXTRACT and SIGN_EXTRACT to the function
reg_overlap_mentioned_p on line 1570 of rtlanal.c.  This function
already handles SUBREGs, so the "reg" in the name is a misnomer.  It
is currently the call to abort() at the bottom of this function (line
1635) that causes the current ICE.


Perhaps a suitable fix would be to turn:

  /* Overly conservative.  */
  if (GET_CODE (x) == STRICT_LOW_PART)
    x = XEXP (x, 0);

into

  /* Overly conservative.  */
  if (GET_CODE (x) == STRICT_LOW_PART
      || GET_CODE (x) == ZERO_EXTRACT
      || GET_CODE (x) == SIGN_EXTRACT)
    x = XEXP (x, 0);



As you can probably tell I'm well out of my depth with GCC's vector
modes, and just thought I'd check with you guys whether either or
both of the above changes is the appropriate fix.

Anyone?  Help!


Roger
--
Roger Sayle,                         E-mail: roger at eyesopen dot com
OpenEye Scientific Software,         WWW: http://www.eyesopen.com/
Suite 1107, 3600 Cerrillos Road,     Tel: (+1) 505-473-7385
Santa Fe, New Mexico, 87507.         Fax: (+1) 505-473-0833