On Tue, Oct 16, 2012 at 10:19:54AM -0700, Ian Lance Taylor wrote: > On Mon, Oct 15, 2012 at 5:28 AM, Aldy Hernandez <al...@redhat.com> wrote: > I don't see a reason to add this new function. I would just inline it > into noce_can_store_speculate_p, replacing the call to > memory_modified_in_p. And I'm not sure I see a reason to change the > comment for memory_modified_in_p, it seems to already be accurate. > > Clearly we could consider the possibility of a PARALLEL of SET insns, > but of course most the compiler won't handle that anyhow. I suppose > that would be a reason to use memory_surely_modified_in_insn_p, but in > that case you might as well handle the PARALLEL case now.
I see another problem with noce_can_store_speculate_p, the return false; checks. The search for a store to exactly that location can be done just on post dominator bbs, but I wonder how you can avoid looking for volatile insns or non-const/pure calls (or insns that may modify the address) also in other bbs. What if there is: location of considered if-conversion on mem if (some_condition) call (); mem = something; ? We only walk immediate post-dominators, so look at the bb containing mem = something;, but I think a call/volatile insn/modification of mem's address is problematic in any bb on any path in between test_bb and the post-dominator on which the write has been found. Jakub