diff mbox

[RFC] Handle sequence in reg_set_p

Message ID 1420976424.2473.177.camel@yam-132-YW-E178-FTW
State New
Headers show

Commit Message

Oleg Endo Jan. 11, 2015, 11:40 a.m. UTC 
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.

Comments

Jeff Law Jan. 12, 2015, 7:01 p.m. UTC | #1 
On 01/11/15 04:40, Oleg Endo wrote:
>>
>> 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.
OK.

Jeff
diff mbox

Patch

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