Re: [PATCH]: Fix SSA constant propagation

[Daniel Berlin <dan at cgsoftware dot com>]

law@redhat.com writes:

>   In message <87bsl31gne.fsf@cgsoftware.com>you write:
>   > Of course, I meant to emit a new move, rather than try to set the
>   > pattern of the old def to the result of gen_move_insn.
>   > 
>   > I'll commit the second part of the patch as obvious, however, now that
>   > i think about it more, i'm not sure about the first one in all cases.
>   > 
>   > The before code (I.E. what is there now, not what i changed it to) is
>   > clearly wrong.
>   > 
>   > I'm pretty sure the code i've changed it to won't handle parallels or
>   > sequences in some cases (since we'll delete the entire insn, rather
>   > than just nullify the set in question).
>   > What the heck is the right thing to do here?
> These should have been dealt with earlier.
How so?
We didn't know it was constant before.

>
>   > Should i have it just nullify the set (rather than delete the whole
>   > insn), but otherwise leave the code as i've got it written below?
>   > 
>   > 2001-08-25  Daniel Berlin  <dan@cgsoftware.com>
>   >              
>   >         * ssa-ccp.c: Include expr.h for gen_move_insn prototype.
>   >         (ssa_ccp_substitute_constants): We can't just replace
>   >         SET_SRC, we have no guarantee the result will be valid.
> Err, no.  Do not check this in.  As I've said before this code is dependent
> on having a higher level RTL in place.  We are not going to contaminate
> the SSA optimizers with that kind of stuff.

Contaminate it with what?
You are simply doing a SET_SRC (x) = constant.
This is incorrect.
At the very least, you should use a validate function, since it's not
guaranteed to be valid.

Unless you plan on introducing a middle rtl where (set (reg:si 57)
(any constant)) is always valid?

I'm just confused as to why you think this is contamination.

>
>
>
>
>   > 2001-08-25  Daniel Berlin  <dan@cgsoftware.co>
>   >  
>   >         * ssa-ccp.c (ssa_ccp_df_delete_unreachable_insns):
>   >         find_unreachable_blocks uses BB_REACHABLE stored in the flags, not
>   >         the aux pointer.
> This is OK.
> jeff

-- 
"Four years ago...  No, it was yesterday.
Today I...  No, that wasn't me.
Sometimes I...  No, I don't.
"-Steven Wright