Re: [PATCH] Add "Implicit sets" to GCSE's cprop (take 2)

[Roger Sayle <roger at www dot eyesopen dot com>]

On Thu, 6 Feb 2003, Richard Henderson wrote:
> I believe I've found the problem.  Before gcse we had
>
> (insn 4062 9281 4065 383 0x20000053188 (set (reg/v:DI 82 [ swapped ])
>         (eq:DI (reg/v:DI 82 [ swapped ])
>             (const_int 0 [0x0]))) 149 {*setcc_internal} (nil)
>     (nil))
> (jump_insn 4065 4062 9282 383 0x20000053188 (set (pc)
>         (if_then_else (eq (reg/v:DI 82 [ swapped ])
>                 (const_int 0 [0x0]))
>             (label_ref 4223)
>             (pc))) 174 {*bcc_normal} (nil)
>     (expr_list:REG_BR_PRED (concat (const_int 20 [0x14])
>             (const_int 7000 [0x1b58]))
>         (nil)))
>
> Note that reg 82 is modified and then tested.  Our use of
> get_condition looked back through the first instruction to
> return (ne:DI (reg:DI 82) (const_int 0)).  So we wound up
> inferring the wrong condition.

Excellent catch!

I'd completely failed to anticipate that the "setcc" instruction
immediately prior to the jump instruction, could mention it's
destination in its source.  As you point out, substituting the
set in the jump, then leads to "find_implicit_sets" inferring
the wrong equivalence.


I've one question.  What's the behaviour of fis_get_condition
on your example above?  Trying to follow the logic in that
routine, it looks like it determines that the condition is
modified between earliest and jump, and returns NULL_RTX.


If so, I'll add fis_canonicalize_condition to my long-term TODO
list.  We should eventually teach it to ignore the condition
modifying instruction before the jump, and return just the
condition in the jump insn.  For your example above, this should
enable us to infer that REG:82 is zero on the other branch, rather
than punt completely.


Very many thanks once again.  I owe you one for tracking this
down and my apologies for the incovenience this has caused.

Roger
--