This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: PING^(n+1) fwprop merge (ping 6 or 7 by now)
On 07 May 2006 13:04:40 -0700, Ian Lance Taylor <ian@airs.com> wrote:
> +/* Return a pointer to one of the occurrences of FIND in *PX. */
> +
> +rtx *
> +find_occurrence (rtx *px, rtx find)
I think it would be preferable to write this in terms for
for_each_rtx, unless there is some reason not to.
IIRC the reason for not using for_each_rtx was that we can't return a
pointer to FIND. Suggestions for doing the same thing with
for_each_rtx are welcome, but I couldn't think of a way to achieve the
same thing with for_each_rtx :-(
I count six uses of DF_INSN_UID_GET in df-core.c. You are fixing
three of them. Is there any reason not to fix the other three?
Actually I think that we shouldn't have to fix any of them, or at
least not this way. What happens is that we segfault in this loop in
e.g. df_bb_regno_last_def_find
FOR_BB_INSNS_REVERSE (bb, insn)
{
unsigned int uid = INSN_UID (insn);
for (def = DF_INSN_UID_GET (df, uid)->defs; def; def = def->next_ref)
if (DF_REF_REGNO (def) == regno)
return def;
}
If we do not find a DEF setting REGNO, we end up looking for
DF_INSN_UID_GET on a NOTE:
Breakpoint 3, df_bb_regno_last_def_find (df=0xc7b140,
bb=0x2aaaaafac300, regno=62)
at df-core.c:957
957 FOR_BB_INSNS_REVERSE (bb, insn)
(gdb) p debug_bb(bb)
;; basic block 42, loop depth 2, count 0
;; prev block 38, next block 39
;; pred: 38 [100.0%] (fallthru) 3 [100.0%]
;; succ: 4 [97.5%] 39 [2.5%] (fallthru,loop_exit)
;; Registers live at start: (nil)
(code_label 261 225 260 42 38 "" [1 uses])
(note 260 261 227 42 [bb 42] NOTE_INSN_BASIC_BLOCK)
(insn 227 260 228 42 (set (reg:QI 70 [ D.1933 ])
(mem:QI (reg/v/f:DI 62 [ s.48 ]) [0 S1 A8])) 55 {*movqi_1} (nil)
(nil))
(insn 228 227 229 42 (set (reg:CCZ 17 flags)
(compare:CCZ (reg:QI 70 [ D.1933 ])
(const_int 0 [0x0]))) 9 {*cmpqi_ccno_1} (nil)
(nil))
(jump_insn 229 228 231 42 (set (pc)
(if_then_else (ne (reg:CCZ 17 flags)
(const_int 0 [0x0]))
(label_ref 23)
(pc))) 511 {*jcc_1} (nil)
(expr_list:REG_BR_PROB (const_int 9750 [0x2616])
(nil)))
;; Registers live at end: (nil)
$13 = void
(gdb) p regno
$14 = 62
(gdb) cont
Continuing.
Program received signal SIGSEGV, Segmentation fault.
0x0000000000575927 in df_bb_regno_last_def_find (df=0xc7b140,
bb=<value optimized out>,
regno=62) at df-core.c:961
961 for (def = DF_INSN_UID_GET (df, uid)->defs; def; def =
def->next_ref)
(gdb)
Dan, what do you think about this? Should we make sure in
df_bb_regno_last_def_find that we are looking at insns, i.e. something
like this:
FOR_BB_INSNS_REVERSE (bb, insn)
{
- unsigned int uid = INSN_UID (insn);
+ unsigned int uid;
+ if (! INSN_P (insn))
+ continue;
+
+ uid = INSN_UID (insn);
for (def = DF_INSN_UID_GET (df, uid)->defs; def; def = def->next_ref)
if (DF_REF_REGNO (def) == regno)
return def;
}
It seems to me that getting DF_INSN_UID_GET on a NOTE is always wrong...?
I note that
the code on dataflow-branch still assumes that DF_INSN_UID_GET is
always non-NULL.
Ehm, yes...
I also note that fwprop is not enabled by default on the dataflow branch :-/
> +/* Returns a canonical version of X for the address, from the point of view,
> + that all multiplications are represented as MULT instead of the multiply
> + by a power of 2 being represented as ASHIFT.
> +
> + Every ASHIFT we find has been made by simplify_gen_binary and was not
> + there before, so it is not shared. So we can do this in place. */
How can you be sure of that? Doesn't that assume that no target will
use ASHIFT when representing an address?
Using ASHIFT in an address is non-canonical RTL. That means we can
make that assumption, no?
(And, yes, that's actually a problem for targets like e.g. ARM, which
has instructions for manipulating addresses with SHIFTs instead of
MULs ;-).
Thanks for your review. I'm still looking at/working on the other
issues you mentioned.
Gr.
Steven