Fix cc0 handling in try_redirect_by_replacing_jump

Richard Sandiford richard@codesourcery.com
Mon Jan 29 11:15:00 GMT 2007


Steven Bosscher's patch to make ifcvt work in cfglayout mode triggered a
problem on cc0 targets.  ifcvt inserts the if-converted form before the
original jump and, after Steven's patch, uses redirect_edge_and_branch_force
to redirect the jump to the join block.  The problem is that
try_redirect_by_replacing_jump expects the instruction before the
jump to the cc0 setter, and deletes it without even looking at it.

The two obvious fixes seemed to be:

  (1) Make try_redirect_by_replacing_jump check only_sets_cc0_p
      (as other cfgrtl.c routines do).

  (2) Make ifcvt insert the new code before the cc0 setter.

(2) would work, but would be quite invasive for non-cc0 targets.
It also seems like a bad interface: inserting before BB_END is
the natural thing to do conceptually, and worked fine before.
(The original cc0 gets deleted as dead later on.)

The current try_redirect_by_replacing_jump code might be reasonable if
the only modification you're making is to the jump itself, such as in
jump threading.  However, it doesn't seem reasonable if it's supposed
to be a general interface.  The caller might have moved the cc0 setter
(as here), and the cc0 setter might not even be dead; the caller might be
trying to reuse it some other situation.  It also seems like a bad idea
to delete something without looking at it.  Finally, as said above,
other cfgrtl.c routines already check only_sets_cc0_p.

I'm therefore treating this as a latent bug in try_redirect_by_replacing_jump.
Patch tested on m68k-linux-gnu and bootstrapped & regression-tested on
x86_64-linux-gnu (which was perhaps a bit pointless, given that the
code is guarded by HAVE_cc0).  OK to install?

(This message is more about (1) vs (2); I realise the patch probably
qualifies as obvious if (1) is the preferred approach.)

Richard
...wearing a flame-retardant jacket in case of an anti-cc0 backlash.


gcc/
	* cfgrtl.c (try_redirect_by_replacing_jump): Check only_sets_cc0_p.

Index: gcc/cfgrtl.c
===================================================================
--- gcc/cfgrtl.c	2007-01-25 07:20:06.012350000 -0800
+++ gcc/cfgrtl.c	2007-01-25 07:21:37.033214000 -0800
@@ -728,7 +728,8 @@ try_redirect_by_replacing_jump (edge e, 
      the cc0 setter too.  */
   kill_from = insn;
 #ifdef HAVE_cc0
-  if (reg_mentioned_p (cc0_rtx, PATTERN (insn)))
+  if (reg_mentioned_p (cc0_rtx, PATTERN (insn))
+      && only_sets_cc0_p (PREV_INSN (insn)))
     kill_from = PREV_INSN (insn);
 #endif
 



More information about the Gcc-patches mailing list