Fw: conditional trap sched2 abort() on mainline

["David S. Miller" <davem at redhat dot com>]

I just realized that I should have at least CC:'d the patches
list since I include a potential fix for the problem I discovered.

Begin forwarded message:

Date: Fri, 10 Oct 2003 05:07:28 -0700
From: "David S. Miller" <davem@redhat.com>
To: gcc-bugs@gcc.gnu.org
Subject: conditional trap sched2 abort() on mainline



There seems to be some kind of death note issue on the mainline
tree on conditional trap platforms.

The case I'm analyzing on Sparc64 looks like this, we start with these
two basic blocks:

(code_label 812 823 642 32 714 "" [1 uses])
(note:HI 642 812 257 32 [bb 32] NOTE_INSN_BASIC_BLOCK)
(insn:HI 257 642 258 32 mm/memory.c:760 (set (reg:CC 100 %icc)
        (compare:CC (reg:SI 8 %o0 [orig:150+4 ] [150])
            (const_int -1 [0xffffffffffffffff]))) 0 {*cmpsi_insn} (nil)
    (expr_list:REG_DEAD (reg:SI 8 %o0 [orig:150+4 ] [150])
        (nil)))
(jump_insn:HI 258 257 793 32 mm/memory.c:760 (set (pc)
        (if_then_else (eq (reg:CC 100 %icc)
                (const_int 0 [0x0]))
            (label_ref 218)
            (pc))) 37 {*normal_branch} (insn_list 257 (nil))
    (expr_list:REG_DEAD (reg:CC 100 %icc)
        (expr_list:REG_BR_PROB (const_int 2900 [0xb54])
            (nil))))
(code_label 793 258 674 33 712 "" [2 uses])
(note:HI 674 793 419 33 [bb 33] NOTE_INSN_BASIC_BLOCK)
(insn:HI 419 674 420 33 mm/memory.c:770 (trap_if (const_int 1 [0x1])
        (const_int 5 [0x5])) 362 {trap} (nil)
    (nil))
(barrier:HI 420 419 205)

Basically this is:
	if (condition)
		__builtin_trap();

Then we run the second if-conversion pass via if_convert(1), this
leaves us with:

(note:HI 642 812 835 32 [bb 32] NOTE_INSN_BASIC_BLOCK)
(insn 835 642 836 32 mm/memory.c:770 (set (reg:CC 100 %icc)
        (compare:CC (reg:SI 8 %o0 [orig:150+4 ] [150])
            (const_int -1 [0xffffffffffffffff]))) -1 (nil)
    (nil))
(insn 836 835 257 32 mm/memory.c:770 (trap_if (ne (reg:CC 100 %icc)
            (const_int 0 [0x0]))
        (const_int 5 [0x5])) -1 (nil)
    (nil))
(insn:HI 257 836 837 32 mm/memory.c:760 (set (reg:CC 100 %icc)
        (compare:CC (reg:SI 8 %o0 [orig:150+4 ] [150])
            (const_int -1 [0xffffffffffffffff]))) 0 {*cmpsi_insn} (nil)
    (expr_list:REG_DEAD (reg:SI 8 %o0 [orig:150+4 ] [150])
        (nil)))
(jump_insn 837 257 838 32 (set (pc)
        (label_ref 218)) -1 (nil)
    (nil))

We've transformed the above code to use the conditional trap
instruction, which is fine, but we're missing a REG_DEAD note
on insn 836 for %icc.

This is a big problem because we next run the second scheduling
pass, since we have CHECK_DEAD_NOTES debugging on we delete and
count the death notes in each basic block, then after scheduling
we recreate them with a update_life_info() call or two, then we
verify the basic block REG_DEAD note count.

They'll be wrong for basic block 2 in the second insn dump above
because update_life_info() will put the necessary REG_DEAD note
into insn 836 for %icc, so the count won't match up.

I know what's wrong, but who is responsible for putting the necessary
REG_DEAD notes where we need them during if_conversion?  I guess what
needs to happen is that if we truly generate any conditional traps
we call update_life_info()?

if_convert() seems to have logic that tries to fix up the life
information if any basic blocks are deleted, so I suppose making
it also do this fixup if any conditional traps are generated?
I wrote a patch which does that and it's at the end of this email.
(No basic blocks are delete in the conditional trap emitting done
 above, the final else branch of find_cond_trap() is what we
 do, emitting a new jump instruction instead of deleting or merging
 blocks)

Anyways, is this fix correct?

2003-10-10  David S. Miller  <davem@redhat.com>

	* ifcvt.c (num_conditional_traps): New.
	(find_cond_trap): Increment it.
	(if_convert): Initialize to zero and rebuild life data
	if non-zero.

--- ifcvt.c.~1~	Wed Oct  8 10:08:51 2003
+++ ifcvt.c	Fri Oct 10 05:01:33 2003
@@ -78,6 +78,9 @@ static int num_updated_if_blocks;
 /* # of basic blocks that were removed.  */
 static int num_removed_blocks;
 
+/* # of conditional traps that were generated.  */
+static int num_conditional_traps;
+
 /* Whether conditional execution changes were made.  */
 static int cond_exec_changed_p;
 
@@ -2544,6 +2547,8 @@ find_cond_trap (basic_block test_bb, edg
   if (seq == NULL)
     return FALSE;
 
+  num_conditional_traps++;
+
   /* Emit the new insns before cond_earliest.  */
   emit_insn_before_setloc (seq, cond_earliest, INSN_LOCATOR (trap));
 
@@ -3113,6 +3118,7 @@ if_convert (int x_life_data_ok)
   num_possible_if_blocks = 0;
   num_updated_if_blocks = 0;
   num_removed_blocks = 0;
+  num_conditional_traps = 0;
   life_data_ok = (x_life_data_ok != 0);
 
   if (! (* targetm.cannot_modify_jumps_p) ())
@@ -3173,7 +3179,8 @@ if_convert (int x_life_data_ok)
   clear_aux_for_blocks ();
 
   /* Rebuild life info for basic blocks that require it.  */
-  if (num_removed_blocks && life_data_ok)
+  if ((num_removed_blocks || num_conditional_traps)
+      && life_data_ok)
     {
       /* If we allocated new pseudos, we must resize the array for sched1.  */
       if (max_regno < max_reg_num ())
@@ -3198,6 +3205,9 @@ if_convert (int x_life_data_ok)
       fprintf (rtl_dump_file,
 	       "%d basic blocks deleted.\n\n\n",
 	       num_removed_blocks);
+      fprintf (rtl_dump_file,
+	       "%d conditional traps emitted.\n\n\n",
+	       num_conditional_traps);
     }
 
 #ifdef ENABLE_CHECKING