This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]

Re: PATH: bug fix in reorg.c (second try)



  In message <200007311715.SAA02000@net.HCC.nl>you write:
  > Hello,
  > 
  > This is the second time I am trying to send this patch. Could some one
  > review this patch?
  > 
  > Text from last patch:
  > Last year there was a bug report sent about a problem in reorg on
  > machines with multiple delay slots:
  > 
  > http://gcc.gnu.org/ml/gcc-bugs/1999-11/msg00842.html
  > 
  > The problem was in routine try_merge_delay_insns. This routine deletes
  > duplicate insns.
  > The code fragment that was showing the problem looked like:
  > 
  >         bne label_77
  >          r0=r5          delayed insn1
  >          r5=1           delayed insn2
  >         br label_77
  >          r0=r5          delayed insn3
  > 
  > The problem was that the 'delayed insn3' was deleted because the resources
  > were not calculated correctly.
  > In the old situation only the references resources are calculated and not
  > the set resources. This means that the insn 'r5=1' is never seen. This
  > results in removing 'delayed insn3'.
  > After the patch below this is fixed. I reorganized the loop. I now
  > run the code seperate for every delay slot.
  > When the example above is used the first time the references
  > for 'r0=r5' and the set and refence resources for 'r5=1' are set.
  > This must be done because the insn 'r5=1' is allways executed. The
  > second time I enter the loop only the references for 'r5=1' are set.
  > I checked with Alan Lehotsky that the patch below fixed his problem. He
  > has a sharc with 2 delay slots. I also verified that the c4x wich has
  > 3 delay slots worked. For targets with 1 delay slot the code change has
  > no effect.
  > 
  >         Herman.
  > 
  > 2000-07-31 Herman A.J. ten Brugge <Haj.Ten.Brugge@net.HCC.nl>
  > 
  >         * reorg.c (try_merge_delay_insns): Reorganize the code so we
  >         correctly calculate the set_resources.
This looks pretty good.  I've only got one question about the change before
approving it.

You account for any references in the first filled delay slot of INSN, but
I don't see where any sets which occur in that delay slot are accounted
for.

  > +          /* If this is not an annulling branch, take into account anythin
  > g
  > +	     needed in INSN's delay slot. This prevents two increments from
  > +	     being incorrectly folded into one. If we are annulling, this
  > +	     would be the correct thing to do. (The alternative, looking at
  > +	     things set in NEXT_TO_MATCH will essentially disable this
  > +	     optimization. This method is somewhat of a kludge, but I don't
  > +	     see a better way.)  */
  > +	  mark_referenced_resources (next_to_match, &needed, 1);
Here's the code in question.

Why don't you need to check the set resources for the first filled delay
slot of INSN?

jeff


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]