This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [path] PR 54900: store data race in if-conversion pass
- From: Ian Lance Taylor <iant at google dot com>
- To: Aldy Hernandez <aldyh at redhat dot com>
- Cc: gcc-patches <gcc-patches at gcc dot gnu dot org>, Richard Guenther <richard dot guenther at gmail dot com>, Ian Lance Taylor <ian at airs dot com>, Jakub Jelinek <jakub at redhat dot com>, Andrew MacLeod <amacleod at redhat dot com>
- Date: Tue, 16 Oct 2012 10:19:54 -0700
- Subject: Re: [path] PR 54900: store data race in if-conversion pass
- References: <507C015F.2090508@redhat.com>
On Mon, Oct 15, 2012 at 5:28 AM, Aldy Hernandez <aldyh@redhat.com> wrote:
>
> Here we have a store data race because noce_can_store_speculate_p()
> incorrectly returns true. The problem is that memory_modified_in_insn_p()
> returns true if an instruction *MAY* modify a memory location, but the store
> can only be speculated if we are absolutely sure the store will happen on
> all subsequent paths.
>
> My approach is to implement a memory_SURELY_modified_in_insn_p(), which will
> trigger this optimization less often, but at least it will be correct.
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.
Ian