On Thu, 2015-01-08 at 14:18 -0700, Jeff Law wrote:
> On 01/08/15 05:23, Oleg Endo wrote:
> > Hi,
> >
> > Currently reg_set_p doesn't handle sequence rtx, which I've identified
> > as the root cause of PR 64479. There is another alternative fix for the
> > PR, but I'd like to get some comments regarding letting reg_set_p also
> > handle sequence rtx:
> >
> > Index: gcc/rtlanal.c
> > ===================================================================
> > --- gcc/rtlanal.c (revision 218544)
> > +++ gcc/rtlanal.c (working copy)
> > @@ -875,17 +875,24 @@
> > {
> > /* We can be passed an insn or part of one. If we are passed an insn,
> > check if a side-effect of the insn clobbers REG. */
> > - if (INSN_P (insn)
> > - && (FIND_REG_INC_NOTE (insn, reg)
> > - || (CALL_P (insn)
> > - && ((REG_P (reg)
> > - && REGNO (reg) < FIRST_PSEUDO_REGISTER
> > - && overlaps_hard_reg_set_p (regs_invalidated_by_call,
> > - GET_MODE (reg), REGNO (reg)))
> > - || MEM_P (reg)
> > - || find_reg_fusage (insn, CLOBBER, reg)))))
> > - return 1;
> > + if (INSN_P (insn) && FIND_REG_INC_NOTE (insn, reg))
> > + return true;
> >
> > + /* After delay slot handling, call and branch insns might be in a
> > + sequence. Check all the elements there. */
> > + if (INSN_P (insn) && GET_CODE (PATTERN (insn)) == SEQUENCE)
> > + for (int i = 0; i < XVECLEN (PATTERN (insn), 0); ++i)
> > + if (reg_set_p (reg, XVECEXP (PATTERN (insn), 0, i)))
> > + return true;
> > +
> > + if (CALL_P (insn)
> > + && ((REG_P (reg) && REGNO (reg) < FIRST_PSEUDO_REGISTER
> > + && overlaps_hard_reg_set_p (regs_invalidated_by_call,
> > + GET_MODE (reg), REGNO (reg)))
> > + || MEM_P (reg)
> > + || find_reg_fusage (insn, CLOBBER, reg)))
> > + return true;
> > +
> > return set_of (reg, insn) != NULL_RTX;
> > }
> >
> >
> > Would that be OK to do if it passes testing, or is there something that
> > could potentially/obviously blow up?
> It looks reasonable to me.
>
> Any particular reason why the SEQUENCE handling isn't done first, then
> the REG_INC and CALL insn handling? I'd probably explicitly return
> false if we had a sequence and none of its elements returned true.
> There's no need to check anything on the toplevel SEQUENCE to the best
> of my knowledge.
No meaningful reason. Attached is an updated patch that applies on 4.8
and trunk. Bootstrapped on trunk i686-pc-linux-gnu. Tested with make
-k check RUNTESTFLAGS="--target_board=sh-sim
\{-m2/-ml,-m2/-mb,-m2a/-mb,-m4/-ml,-m4/-mb,-m4a/-ml,-m4a/-mb}" on trunk
sh-elf. In the PR it has been reported, that the patch fixes the
problems when building a native SH GCC.
Although the related SH problem occurs only on 4.8, I'd like to install
this on trunk and 4.9, too, to avoid future surprises. OK?
Cheers,
Oleg
gcc/ChangeLog:
PR target/64479
* rtlanal.c (set_reg_p): Handle SEQUENCE constructs.
Index: gcc/rtlanal.c
===================================================================
--- gcc/rtlanal.c (revision 218544)
+++ gcc/rtlanal.c (working copy)
@@ -873,6 +873,17 @@
int
reg_set_p (const_rtx reg, const_rtx insn)
{
+ /* After delay slot handling, call and branch insns might be in a
+ sequence. Check all the elements there. */
+ if (INSN_P (insn) && GET_CODE (PATTERN (insn)) == SEQUENCE)
+ {
+ for (int i = 0; i < XVECLEN (PATTERN (insn), 0); ++i)
+ if (reg_set_p (reg, XVECEXP (PATTERN (insn), 0, i)))
+ return true;
+
+ return false;
+ }
+
/* We can be passed an insn or part of one. If we are passed an insn,
check if a side-effect of the insn clobbers REG. */
if (INSN_P (insn)
@@ -884,7 +895,7 @@
GET_MODE (reg), REGNO (reg)))
|| MEM_P (reg)
|| find_reg_fusage (insn, CLOBBER, reg)))))
- return 1;
+ return true;
return set_of (reg, insn) != NULL_RTX;
}
