This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
[path] PR 54900: store data race in if-conversion pass
- From: Aldy Hernandez <aldyh at redhat dot com>
- To: 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: Mon, 15 Oct 2012 08:28:15 -0400
- Subject: [path] PR 54900: store data race in if-conversion pass
[Ian, I am copying you because this is originally your code. Richard, I
am copying you because you are all things aliased :-). And Andrew is
here because I am unable to come up with a suitable test for the
simulate-thread harness.].
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 thought of restricting the speculation for "--param
allow-store-data-race=0" or transactional code, but IMO the speculation
is incorrect as is.
I am having a bit of a problem coming up with a generic testcase.
Perhaps Andrew or others have an idea.
The attached testcase fails to trigger without the patch, because AFAICT
we have no way of testing an addition of zero to a memory location:
cmpl $1, flag(%rip)
setb %al
addl %eax, dont_write(%rip)
In the simulate-thread harness I can test the environment before an
instruction, and after an instruction, but adding 0 to *dont_write
produces no measurable effects, particularly in a back-end independent
manner. Ideas?
Bootstrap and regtested on x86-64 Linux.
Patch OK? (Except the test itself.)
Attachment:
curr
Description: Text document