Hi all,
I would like to know if the attached patch resource-check.patch is a
good sanity check or not.
I have been working in reorg.c and running into trouble with the target
info cache in mark_target_live_regs, so I decided to write a patch that
checks consistency between cached values and recomputed values. Soon I
started running into issues that were unrelated to my changes in reorg.c.
As an example of the kind of situations the sanity check detects,
consider fixed-bit.c.212r.alignments. During pass_machine_reorg, the
following scenario happens:
1. a copy of insn 78 is imported into the delay slot of jump_insn 14.
(insn 106 13 49
/scratch/vries/henry7/mips/src/gcc-trunk-4.5/libgcc/../gcc/config/fixed-bit.c:183
(sequence [
(jump_insn 14 13 78
/scratch/vries/henry7/mips/src/gcc-trunk-4.5/libgcc/../gcc/config/fixed-bit.c:183
(set (pc)
(if_then_else (ne (reg:SI 2 $2 [200])
(const_int 0 [0x0]))
(label_ref:SI 105)
(pc))) 442 {*branch_equalitysi_micromips}
(expr_list:REG_DEAD (reg:SI 2 $2 [200])
(expr_list:REG_BR_PROB (const_int 6100 [0x17d4])
(nil)))
-> 105)
(insn/s 78 14 49 (set (reg:SI 3 $3 [orig:196 z+4 ] [196])
(const_int 0 [0x0])) 284 {*movsi_internal} (nil))
]) -1 (nil))
It's a copy since jump_insn 14 does not own the thread containing insn
78. The copy is marked with INSN_FROM_TARGET_P (insn/s) to indicate that
it was imported from the target of the jump.
2. we call mark_target_live_regs for insn 17. We scan all insn from the
start of the function to insn 17. We ignore however the copy of insn 78,
since it's marked with INSN_FROM_TARGET_P, so we conclude that $3 is not
live at insn 17. This info is stored in the target info cache.
3. redundant_insn finds that the original insn 78 is redundant (since
the copy of insn 78 occurs on all paths towards insn 78) and decides to
remove the original. The INSN_FROM_TARGET_P of the copy is cleared, to
indicate that the $3 is now live on the fall-through path of jump_insn
14 as well.
4. we call mark_target_live_regs for insn 17 again. We get the value
from the cache and conclude that $3 is not live at insn 17. Then my
patch recomputes the live info, which now takes the copy of insn 78 into
account since INSN_FROM_TARGET_P has been cleared, and concludes that $3
is live. And we assert.
The following fix makes sure that the cached live info is invalidated:
...
@@ -1865,6 +1880,7 @@ redundant_insn (rtx insn, rtx target, rt
{
/* Show that this insn will be used in the sequel. */
INSN_FROM_TARGET_P (candidate) = 0;
+ incr_ticks_for_insn (candidate);
return candidate;
}
...
and the assert is not triggered anymore.
So my questions are:
- is the consistency check correct? Does it make sense to fix all the
cases where it triggers?
- Is my analysis of the example and the fix correct?
Thanks,
- Tom
Index: gcc/resource.c
===================================================================
--- gcc/resource.c (revision 310935)
+++ gcc/resource.c (working copy)
@@ -878,6 +878,12 @@ mark_target_live_regs (rtx insns, rtx ta
rtx jump_insn = 0;
rtx jump_target;
HARD_REG_SET scratch;
+#if defined ENABLE_RUNTIME_CHECKING
+ HARD_REG_SET cached;
+ bool cached_valid = false;
+ int fb = -1;
+#endif
+
struct resources set, needed;
/* Handle end of function. */
@@ -918,6 +924,13 @@ mark_target_live_regs (rtx insns, rtx ta
if (b == -1)
b = find_basic_block (target, MAX_DELAY_SLOT_LIVE_SEARCH);
+#if defined ENABLE_RUNTIME_CHECKING
+ else
+ {
+ fb = find_basic_block (target, MAX_DELAY_SLOT_LIVE_SEARCH);
+ gcc_assert (fb == -1 || fb == b);
+ }
+#endif
if (target_hash_table != NULL)
{
@@ -927,8 +940,13 @@ mark_target_live_regs (rtx insns, rtx ta
update it below. */
if (b == tinfo->block && b != -1 && tinfo->bb_tick == bb_ticks[b])
{
+#if defined ENABLE_RUNTIME_CHECKING
+ cached_valid = true;
+ COPY_HARD_REG_SET (cached, tinfo->live_regs);
+#else
COPY_HARD_REG_SET (res->regs, tinfo->live_regs);
return;
+#endif
}
}
else
@@ -1126,6 +1144,10 @@ mark_target_live_regs (rtx insns, rtx ta
{
COPY_HARD_REG_SET (tinfo->live_regs, res->regs);
}
+#if defined ENABLE_RUNTIME_CHECKING
+ if (cached_valid)
+ gcc_assert (hard_reg_set_equal_p (cached, tinfo->live_regs));
+#endif
}
/* Initialize the resources required by mark_target_live_regs ().
;; Function __ussubudq3 (__ussubudq3)
(note 1 0 6 NOTE_INSN_DELETED)
(note 6 1 80 2 [bb 2] NOTE_INSN_BASIC_BLOCK)
(note 80 6 4 2 NOTE_INSN_PROLOGUE_END)
(note 4 80 9 2 NOTE_INSN_FUNCTION_BEG)
(debug_insn 9 4 11 2
/scratch/vries/henry7/mips/src/gcc-trunk-4.5/libgcc/../gcc/config/fixed-bit.c:180
(var_location:DI x (concatn:DI [
(reg:SI 4 $4 [orig:209 x.0 ] [209])
(reg:SI 5 $5 [orig:210 x.0+4 ] [210])
])) -1 (nil))
(debug_insn 11 9 12 2
/scratch/vries/henry7/mips/src/gcc-trunk-4.5/libgcc/../gcc/config/fixed-bit.c:181
(var_location:DI y (concatn:DI [
(reg:SI 6 $6 [orig:211 y.1 ] [211])
(reg:SI 7 $7 [orig:212 y.1+4 ] [212])
])) -1 (nil))
(debug_insn 12 11 13 2
/scratch/vries/henry7/mips/src/gcc-trunk-4.5/libgcc/../gcc/config/fixed-bit.c:182
(var_location:DI z (minus:DI (concatn:DI [
(reg:SI 4 $4 [orig:209 x.0 ] [209])
(reg:SI 5 $5 [orig:210 x.0+4 ] [210])
])
(concatn:DI [
(reg:SI 6 $6 [orig:211 y.1 ] [211])
(reg:SI 7 $7 [orig:212 y.1+4 ] [212])
]))) -1 (nil))
(insn 13 12 14 2
/scratch/vries/henry7/mips/src/gcc-trunk-4.5/libgcc/../gcc/config/fixed-bit.c:183
(set (reg:SI 2 $2 [200])
(gtu:SI (reg:SI 6 $6 [orig:211 y.1 ] [211])
(reg:SI 4 $4 [orig:209 x.0 ] [209]))) 479 {*sgtu_sisi} (nil))
(jump_insn 14 13 49 2
/scratch/vries/henry7/mips/src/gcc-trunk-4.5/libgcc/../gcc/config/fixed-bit.c:183
(set (pc)
(if_then_else (ne (reg:SI 2 $2 [200])
(const_int 0 [0x0]))
(label_ref:SI 48)
(pc))) 442 {*branch_equalitysi_micromips} (expr_list:REG_DEAD
(reg:SI 2 $2 [200])
(expr_list:REG_BR_PROB (const_int 6100 [0x17d4])
(nil)))
-> 48)
(note 49 14 16 3 [bb 3] NOTE_INSN_BASIC_BLOCK)
(jump_insn 16 49 50 3
/scratch/vries/henry7/mips/src/gcc-trunk-4.5/libgcc/../gcc/config/fixed-bit.c:183
(set (pc)
(if_then_else (ne (reg:SI 6 $6 [orig:211 y.1 ] [211])
(reg:SI 4 $4 [orig:209 x.0 ] [209]))
(label_ref:SI 55)
(pc))) 442 {*branch_equalitysi_micromips} (expr_list:REG_BR_PROB
(const_int 3900 [0xf3c])
(nil))
-> 55)
(note 50 16 17 4 [bb 4] NOTE_INSN_BASIC_BLOCK)
(insn 17 50 18 4
/scratch/vries/henry7/mips/src/gcc-trunk-4.5/libgcc/../gcc/config/fixed-bit.c:183
(set (reg:SI 2 $2 [202])
(gtu:SI (reg:SI 7 $7 [orig:212 y.1+4 ] [212])
(reg:SI 5 $5 [orig:210 x.0+4 ] [210]))) 479 {*sgtu_sisi} (nil))
(jump_insn 18 17 55 4
/scratch/vries/henry7/mips/src/gcc-trunk-4.5/libgcc/../gcc/config/fixed-bit.c:183
(set (pc)
(if_then_else (ne (reg:SI 2 $2 [202])
(const_int 0 [0x0]))
(label_ref:SI 48)
(pc))) 442 {*branch_equalitysi_micromips} (expr_list:REG_DEAD
(reg:SI 2 $2 [202])
(expr_list:REG_BR_PROB (const_int 6100 [0x17d4])
(nil)))
-> 48)
(code_label 55 18 24 5 5 "" [1 uses])
(note 24 55 29 5 [bb 5] NOTE_INSN_BASIC_BLOCK)
(note 29 24 26 5 NOTE_INSN_DELETED)
(insn 26 29 27 5
/scratch/vries/henry7/mips/src/gcc-trunk-4.5/libgcc/../gcc/config/fixed-bit.c:182
(set (reg:SI 7 $7 [orig:214+4 ] [214])
(minus:SI (reg:SI 5 $5 [orig:210 x.0+4 ] [210])
(reg:SI 7 $7 [orig:212 y.1+4 ] [212]))) 23 {subsi3} (nil))
(insn 27 26 28 5
/scratch/vries/henry7/mips/src/gcc-trunk-4.5/libgcc/../gcc/config/fixed-bit.c:182
(set (reg:SI 5 $5 [206])
(gtu:SI (reg:SI 7 $7 [orig:214+4 ] [214])
(reg:SI 5 $5 [orig:210 x.0+4 ] [210]))) 479 {*sgtu_sisi} (nil))
(insn 28 27 62 5
/scratch/vries/henry7/mips/src/gcc-trunk-4.5/libgcc/../gcc/config/fixed-bit.c:182
(set (reg:SI 6 $6 [213])
(minus:SI (reg:SI 4 $4 [orig:209 x.0 ] [209])
(reg:SI 6 $6 [orig:211 y.1 ] [211]))) 23 {subsi3}
(expr_list:REG_DEAD (reg:SI 4 $4 [orig:209 x.0 ] [209])
(nil)))
(insn 62 28 74 5
/scratch/vries/henry7/mips/src/gcc-trunk-4.5/libgcc/../gcc/config/fixed-bit.c:182
(set (reg:SI 9 $9 [orig:220 z ] [220])
(minus:SI (reg:SI 6 $6 [213])
(reg:SI 5 $5 [206]))) 23 {subsi3} (expr_list:REG_DEAD (reg:SI 6 $6
[213])
(expr_list:REG_DEAD (reg:SI 5 $5 [206])
(nil))))
(insn 74 62 75 5
/scratch/vries/henry7/mips/src/gcc-trunk-4.5/libgcc/../gcc/config/fixed-bit.c:182
(set (reg:SI 2 $2 [ z ])
(reg:SI 9 $9 [orig:220 z ] [220])) 284 {*movsi_internal}
(expr_list:REG_DEAD (reg:SI 9 $9 [orig:220 z ] [220])
(nil)))
(insn 75 74 81 5
/scratch/vries/henry7/mips/src/gcc-trunk-4.5/libgcc/../gcc/config/fixed-bit.c:182
(set (reg:SI 3 $3 [orig:222 z+4 ] [222])
(reg:SI 7 $7 [orig:221 z+4 ] [221])) 284 {*movsi_internal}
(expr_list:REG_DEAD (reg:SI 7 $7 [orig:221 z+4 ] [221])
(nil)))
(jump_insn 81 75 67 5 (return) 564 {*return} (nil)
-> return)
(barrier 67 81 48)
(code_label 48 67 47 6 4 "" [2 uses])
(note 47 48 78 6 [bb 6] NOTE_INSN_BASIC_BLOCK)
(insn 78 47 79 6
/scratch/vries/henry7/mips/src/gcc-trunk-4.5/libgcc/../gcc/config/fixed-bit.c:184
(set (reg:SI 3 $3 [orig:196 z+4 ] [196])
(const_int 0 [0x0])) 284 {*movsi_internal} (nil))
(insn 79 78 34 6
/scratch/vries/henry7/mips/src/gcc-trunk-4.5/libgcc/../gcc/config/fixed-bit.c:184
(set (reg:SI 2 $2 [ z ])
(const_int 0 [0x0])) 284 {*movsi_internal} (nil))
(debug_insn 34 79 35 6 (var_location:DI z (reg/v:DI 2 $2 [orig:196 z ] [196]))
-1 (nil))
(debug_insn 35 34 43 6
/scratch/vries/henry7/mips/src/gcc-trunk-4.5/libgcc/../gcc/config/fixed-bit.c:189
(var_location:UDQ c (subreg:UDQ (reg/v:DI 2 $2 [orig:196 z ] [196]) 0)) -1
(nil))
(insn 43 35 83 6
/scratch/vries/henry7/mips/src/gcc-trunk-4.5/libgcc/../gcc/config/fixed-bit.c:191
(use (reg/i:UDQ 2 $2)) -1 (nil))
(jump_insn 83 43 82 6
/scratch/vries/henry7/mips/src/gcc-trunk-4.5/libgcc/../gcc/config/fixed-bit.c:191
(return) 564 {*return} (nil)
-> return)
(barrier 82 83 77)
(note 77 82 0 NOTE_INSN_DELETED)
