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]
Other format: [Raw text]

Re: [patch] Fix PR rtl-optimization/15381: reorg incorrectly removes barrier


In message <200405201628.i4KGStZQ009112@hiauly1.hia.nrc.ca>, "John David Anglin
" writes:
 >The enclosed patch fixes a bootstrap failure introduced by this patch:
 >
 >2004-04-09  Caroline Tice  <ctice@apple.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
 >t
 >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
 >were observed.
 >
 >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
 >subtle miscompilations.
 >
 >Dave
 >-- 
 >J. David Anglin                                  dave.anglin@nrc-cnrc.gc.ca
 >National Research Council of Canada              (613) 990-0752 (FAX: 952-660
 >2)
 >
 >2004-05-20  John David Anglin  <dave.anglin@nrc-cnrc.gc.ca>
 >
 >	* 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:

  if (had_barrier
      || (GET_CODE (trial) == JUMP_INSN
          && (simplejump_p (trial) || GET_CODE (PATTER (trial)) == RETURN))


Jeff



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