This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

[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


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]