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] Jump bypassing and improved constant propagation


On Sat, May 25, 2002 at 01:02:21PM -0600, Roger Sayle wrote:
> Being a single block by the time this reaches GCSE, there's nothing
> that the jump bypass optimization can do to reduce it.  I'm currently
> investigating why "-fif-conversion2" isn't able to perform these
> clever transformations after GCSE.

Because if-conversion2 is run after reload and can't allocate registers.
What you might try to remove the call to if_convert that is presently
co-incident with delete_null_pointer_checks.

There are pros and cons here.  On the one hand, it's possible to hoist
"z=(x==y)" whereas "if (x==y) z=1; else z=0;" cannot be moved.  On the
other hand, if left in if/then/else form GCSE might be able to prove that
one of the two arms is partially redundant and eliminate it.  Which would
be more valuable for conditional move expressions than setcc expressions,
but the point still holds.

In any case, my gut feel is that your jump threading optimization is more
important to real applications than global cse of setcc expressions.  So
long as we are able to do local cse of setcc expressions during cse2, we
should be fine.

One thing to check is how placement of if_convert affects loop.  The
existing code doesn't understand the CFG and has some, err, interesting
heuristics that it picks up from wanderint around jumps.  Thus I have no
trouble believing that a loop with no internal branches can be optimized
better that a loop with an embedded if/then/else.

So you might also look at moving the if_convert call that is currently
placed immediately before cse2 to between gcse and loop.  I can't think
off-hand of anything loop might do to create more if-conversion opportunities,
so there doesn't seem to be a reason to run if_convert twice in (gcse, cse2].

As for the patch itself:

> !   rtx set = PATTERN (jump);

Use pc_set.  Some targets clobber scratch registers here.

> +   /* Delete the cc mode setter.  */
> +   if (setcc != NULL)
> +     delete_insn (setcc);

Unless this is a cc0 setter, this is dangerous.  GCSE can
hoist conditional expressions across basic blocks.  This
is particularly relevant to e.g. sparcv9 which has four
CCmode fp comparison registers, and uses pseudos for them
at this stage of compilation.

You have to delete the cc0 setter, but everyone else should
be left to be deleted as dead code.

> ! 	  /* Check for MODE_CC setting instructions followed by
> ! 	     conditional branch instructions first.  */
> ! 	  if (alter_jumps
> ! 	      && GET_CODE (PATTERN (insn)) == SET

Should use single_set.

> ! 	      && GET_CODE (NEXT_INSN (insn)) == JUMP_INSN
> ! 	      && condjump_p (NEXT_INSN (insn))
> ! 	      && ! simplejump_p (NEXT_INSN (insn)))

Should be

	&& any_condjump_p (NEXT_INSN (insn))
	&& onlyjump_p (NEXT_INSN (insn))

and you should be skipping past NOTE insns.

> - #ifdef HAVE_cc0
> - 	  /* Similar code for machines that use a pair of CC0 setter and
> - 	     conditional jump insn.  */
> - 	  else if (alter_jumps

Err, so what are you replacing this with?

> + 	  if (INSN_P (insn))
> + 	    {
> + 	      if (GET_CODE (insn) == JUMP_INSN
> + 		  && condjump_p (insn)
> + 		  && ! simplejump_p (insn))
> + 		changed |= bypass_block (bb, NULL_RTX, insn);
> + 	      else if (GET_CODE (PATTERN (insn)) == SET
> + 		       && GET_CODE (NEXT_INSN (insn)) == JUMP_INSN
> + 		       && condjump_p (NEXT_INSN (insn))
> + 		       && ! simplejump_p (NEXT_INSN (insn)))

Similar commentary to above.  Also it seems like these tests overlap.
Perhaps could be restructured as

	/* setcc initially null */
	if (GET_CODE (insn) == INSN)
	  {
	    if (setcc)
	      break;
	    if (!single_set (insn))
	      break;
	  }
	else if (GET_CODE (insn) == JUMP_INSN)
	  {
	    if (any_condjump_p (insn) && onlyjump_p (insn))
	      ...
	    break;
	  }

which also takes care of the intervening NOTE problem.

> + 		  rtx dest = SET_DEST (PATTERN (insn));
> + 		  if (GET_MODE_CLASS (GET_MODE (dest)) == MODE_CC)
> + 		    changed |= bypass_block (bb, insn, NEXT_INSN (insn));

Why are you so hot on CCmode?  What if the target uses DImode
comparisons to feed the jump insn (e.g. alpha)?

My only guess is that you are under the impression that CCmode registers
are guaranteed to be dead at the ends of blocks.  Which is often true,
but in general false, as discussed above.

So your test really wants to be "is SET_DEST (setcc) consumed by
the branch".  For this you need (ideally) bb->global_live_at_end
or (not-as-ideal) REG_N_REFS(regno) == 2, neither of which are
valid at this time.

I think you're going to have little choice but to run this as a
separate pass after having updated life info.

One more thing: have you checked to see if this completely obsoletes
thread_jump?  If so, we can delete it.  If not, which cases are still
caught by the old code?  Are there enough remaining to justify the
time spent?



r~


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