Message ID | 1420719782.2473.122.camel@yam-132-YW-E178-FTW |
---|---|
State | New |
Headers | show |
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. jeff
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; }