This is the mail archive of the
mailing list for the GCC project.
Re: [PATCH] Make CSE path following use the CFG
> You really are an "I can always do better" kinda guy, no? :-)
I presume so. :-) More seriously, substantial rewrites of cse.c are pretty
rare so I think that we can be extra careful when one of them is being made.
> If you look at set_unique_reg_note, you see that there are no cases
> where cse_insn can call it and expect a NULL_RTX result.
> There is the n_sets == 1 check in cse_insn, so the multiple sets check
> in s_u_r_n can never trigger; and the datum argument to s_u_r_n can
> never be a ASM_OPERANDS because cse_insn only calls it to put a
> REG_EQUAL note with a constant.
I think that there is a (theoritical) hole for patterns like
[(set (reg) (src)
(set (pc) (label)]
because n_sets == 1 for CSE in this case but multiple_sets will return true.
> Calling may_trap_p on the insn at the start of cse_insn, and using
> that later in the apply_change_group case (i.e. the code you quoted),
> that would work. Not calling may_trap_p in the new REG_EQUAL case
> would also work. I have no preference either way.
I don't have either, I'd just use the same idiom in both cases.
> In fact, I considered that earlier to fix the Java problem I had after
> the TER changes, and again to fix you Ada bug report. I opted for
> updating the CFG in cse_insn because it seems to me that this is the
> most transparent: Make all changes necessary after an insn change as
> close as possible to the code where the insn change occurs.
Then why not updating it at the very end of cse_insn once for all?
> Should more cases surface of this assert being triggerd, I guess we'll
> just have to update the CFG in cse_extended_basic_block after all.
How many more cases? :-)