This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [RFA] A patch for PR 25115


On Mon, 28 Nov 2005, Paolo Bonzini wrote:
> 2005-11-28  Paolo Bonzini  <bonzini@gnu.org>
>
>	PR 25115
>	* gcse.c (pre_insert_copy_insn): Try to make the new insn
>	out of a REG_EQUAL or REG_EQUIV note.  Check with expr_equiv_p
>	even if PAT is not a PARALLEL.

I don't think that adding expr_equiv_p tests is the correct way to
go.  Instead we should rely on the constraint on REG_EQUAL/REG_EQUIV
notes that they may only occur on instructions containing exactly one
set.

In the case of Kaz's failure:

(insn 37 36 39 2 (parallel [
            (set (reg:SI 167)
                (unspec:SI [
                        (const (unspec [
                                    (symbol_ref:SI ("res") [flags 0x58] <var_decl 0xb7f16540 res>)
                                ] 24))
                    ] 22))
            (use (reg:SI 144 gbr))
            (use (reg:SI 12 r12))
            (clobber (reg:SI 0 r0))
        ]) 256 {tls_initial_exec} (nil)
    (expr_list:REG_EQUAL (symbol_ref:SI ("res") [flags 0x58] <var_decl 0xb7f16540 res>)
        (nil)))

even though expr_equiv_p isn't strong enough to show that the UNSPEC is
equivalent to the SYMBOL_REF, we know from the semantics of these reg
notes that it must be the assignmnent to reg:SI 167 that we're interested
in.

Therefore I think the correct approach is to assume that if the
pattern is a SET, then that is the "set" we're interested in (as we
do currently).  For, the PARALLEL case, I propose looping over the
elements of the parallel looking for a single set... If there's only one,
this must be the one we're interested in.  Otherwise, we should loop
across the parallel using the existing expr_equiv_p logic.  This final
fallback should never be required in the case of values taken from notes.

Hence set should never be NULL after the loop, so you should never
need your new gen_move_insn clause, and instead should be able to
just use "gcc_assert (set);" immediately before the statement
"if (REG_P (SET_DEST (set))".


But whilst we're on the topic, please remember to repeat that a
patch has been bootstraped and regression tested when its posted and
list the target triples.  And the "PR 25115" in the ChangeLog entry
should be written as "PR rtl-optimization/25115".

Thanks to Steven B for bringing this patch to my attention.

Roger
--


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]