On 12/15/23 01:52, Kewen.Lin wrote:
Hi,

PR112995 exposed one issue in current try_replace_dest_reg
that the result rtx insn after replace_dest_with_reg_in_expr
is probably unable to match any constraints.  Although there
are some checks on the changes onto dest or src of orig_insn,
none is performed on the EXPR_INSN_RTX.

This patch is to add the check before actually replacing dest
in expr with reg.

Bootstrapped and regtested on x86_64-redhat-linux and
powerpc64{,le}-linux-gnu.

Is it ok for trunk?

BR,
Kewen
-----
        PR rtl-optimization/112995

gcc/ChangeLog:

        * sel-sched.cc (try_replace_dest_reg): Check the validity of the
        replaced insn before actually replacing dest in expr.

gcc/testsuite/ChangeLog:

        * gcc.target/powerpc/pr112995.c: New test.
Setting aside whether or not we should just deprecate/remove sel-sched for now....



From the PR:
with moving up, we have:

(insn 46 0 0 (set (reg:DI 64 0 [135])
        (sign_extend:DI (reg/v:SI 64 0 [orig:119 c ] [119]))) 31 {extendsidi2}
     (expr_list:REG_DEAD (reg/v:SI 9 9 [orig:119 c ] [119])
        (nil)))

in try_replace_dest_reg, we updated the above EXPR_INSN_RTX to:

(insn 48 0 0 (set (reg:DI 32 0)
        (sign_extend:DI (reg/v:SI 64 0 [orig:119 c ] [119]))) 31 {extendsidi2}
     (nil))

This doesn't match any constraint and it's an unexpected modification.


It would have been helpful to include that in the patch, along with the fact that (reg 32) and (reg 64) are FP and VREGs respectively. That makes it clearer why the constraints might not match after the change.

OK for the trunk.
jeff

Reply via email to