This is the mail archive of the
mailing list for the GCC project.
Re: [patch] Fix PR rtl-optimization/15381: reorg incorrectly removes barrier
- From: law at redhat dot com
- To: "John David Anglin" <dave at hiauly1 dot hia dot nrc dot ca>
- Cc: gcc-patches at gcc dot gnu dot org
- Date: Thu, 20 May 2004 10:38:18 -0600
- Subject: Re: [patch] Fix PR rtl-optimization/15381: reorg incorrectly removes barrier
- Reply-to: law at redhat dot com
In message <200405201628.i4KGStZQ009112@hiauly1.hia.nrc.ca>, "John David Anglin
>The enclosed patch fixes a bootstrap failure introduced by this patch:
>2004-04-09 Caroline Tice <email@example.com>
> * basic-block.h (struct edge_def): Add new field, crossing_edge.
> (struct basic_block_def): Add new field, partition.
> (UNPARTITIONED, HOT_PARTITION, COLD_PARTITION): New constant macro
>The above patch changed the organization of basic blocks. This triggered
>the bootstrap comparison failure noted in the PR. However, the actual bug
>has nothing to do with the above change.
>The actual problem is in reorg. It causes the miscompilation of the
>routine cpp_classify_number in stage2. The misclassification of floating
>point numbers leads to the bootstrap comparison failure.
>The function cpp_classify_number is miscompiled because delete_from_delay_slo
>incorrectly deletes a barrier following an unconditional branch. The PA
>has a "movb" instruction that can perform a copy from one general register
>to another and then perform a conditional or unconditional branch. This
>isn't a simple jump or return.
>The function delete_from_delay_slot deletes the sequence containing
>the insn to be deleted from the delay slot using delete_related_insns.
>If there is a barrier after the sequence, delete_related_insns removes
>it. Currently, delete_from_delay_slot only restores the barrier
>when the delay insn is a simple jump or return. As a result, the
>barrier isn't restored for a PA "movb" jump.
>Failure to restore the barrier after a PA "movb" jump caused
>mark_target_live_regs to incorrectly determine the resources
>required for a thread after a conditional jump in a block after
>the deleted barrier. Register r24 was live at the start of the
>block but not live in the code preceding the barrier. Removal
>of the barrier caused find_basic_block not to find the basic
>block containing the liveness of r24. As a result, a set setting
>r24 was moved into a delay slot corrupting r24.
>The fix is simply to note whether we had a barrier before we
>delete the sequence containing the insn to be deleted, and then
>remit it after emitting the first insn in the sequence. Something
>similar is done in emit_delay_sequence.
>The patch has been tested on hppa2.0-hp-hpux11.11 (--with-schedule=7100LC),
>hppa1.1-hp-hpux10.20 and hppa64-hp-hpux11.11 before the ssa merge. It has
>also been tested on hppa-unknown-linux-gnu after the merge. No regressions
>Although the PR is marked as a 3.5 regression, I believe the actual bug
>affects all branches.
>Is this ok for 3.5? I would also like to install on 3.3 and 3.4 if
>these branches open for general bug fixes as the bug can cause rather
>J. David Anglin firstname.lastname@example.org
>National Research Council of Canada (613) 990-0752 (FAX: 952-660
>2004-05-20 John David Anglin <email@example.com>
> * reorg.c (delete_from_delay_slot): If we have a barrier after the
> sequence containing the insn to be deleted, always reemit it.
Looks good for mainline. I also think it would be reasonable for
3.3/3.4, but that's not really my decision to make.
If we wanted to be extra safe for 3.3/3.4 we could do something like:
|| (GET_CODE (trial) == JUMP_INSN
&& (simplejump_p (trial) || GET_CODE (PATTER (trial)) == RETURN))