This is the mail archive of the
mailing list for the GCC project.
Re: PATH: bug fix in reorg.c (second try)
- To: Herman ten Brugge <Haj dot Ten dot Brugge at net dot HCC dot nl>
- Subject: Re: PATH: bug fix in reorg.c (second try)
- From: Jeffrey A Law <law at cygnus dot com>
- Date: Thu, 14 Sep 2000 17:36:28 -0600
- cc: gcc-patches at gcc dot gnu dot org
- Reply-To: law at cygnus dot com
In message <200007311715.SAA02000@net.HCC.nl>you write:
> 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:
> 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.
> 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
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
> + /* If this is not an annulling branch, take into account anythin
> + 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?