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: crossjumping tweeks


> On Thu, Dec 06, 2001 at 12:16:05PM +0100, Jan Hubicka wrote:
> > 
> > Thu Dec  6 12:12:24 CET 2001  Jan Hubicka  <jh@suse.cz>
> > 
> > 	* cfgcleanup.c (flow_find_cross_jump): Replace tests for note
> > 	by active_insn_p; count conditional jump as instruction.
> 
> This patch breaks c-torture/execute/920501-3.c on x86, because
> 
> > !       /* Count everything except for unconditional jump as insn.  */
> > !       if (!simplejump_p (i1) && !returnjump_p (i1))
> > ! 	ninsns++;
> 
> this test is true of an unconditional computed jump, not just a
> conditional jump.  We attempt to merge two basic blocks which have
> nothing in common, and reach the final set of loops (which are
> conditional on ninsns being nonzero) without last2 having been set to
> anything at all; null pointer dereference, crash.
Oops. It is de-facto correct to merge two unconditional jumps, but it
is incorrect to die later.
> 
> Looking at the whole function, it is only correct to increment ninsns
> if we have candidates for merging.  The insns being skipped over here
> are insns to be ignored, not insns to be merged.  I think the proper

They are to be merged. They are ignored for checking just because they've
been already checked in special way.

> change, to get conditional jumps considered as merge candidates, would
> be to change "onlyjump_p" to "simplejump_p" in both of the conditions
> that look like
> 
>   if (onlyjump_p (i1)
>       || (returnjump_p (i1) && !side_effects_p (PATTERN (i1))))
> 
> and delete the increment of ninsns in the first block.  I'll be
> bootstrapping this change to see if it breaks anything else, stay
> tuned.  
> 
> (If you wouldn't mind providing me with a sample of code that this was
> intended to optimize better, that would help.)
I will try to prepare something. Basically we need to merge two conditional
jumps in case they are appearing as only instruction in the BB (this is the
case for floating point compare-and-branch condition to get crossjumping
get over them and continue merging in earlier path).

While it may look strange in the trivial case, where only two conditioanl
jumps are merged, it makes sense if it allow more merging. It is dificult
to merge only when it does, so we merge always.  The bb-reorder.c in cfg-branch
is able to undo the unnecesary merges, while mainline not, so your patch
may be sane approach for the mainline.

I will check more.

Honza
> 
> zw


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