On 06/24/2015 10:50 AM, Ramana Radhakrishnan wrote:
On 12/06/15 21:50, Abe Skolnik wrote:
Hi everybody!
In the current implementation of if conversion, loads and stores are
if-converted in a thread-unsafe way:
* loads were always executed, even when they should have not been.
Some source code could be rendered invalid due to null pointers
that were OK in the original program because they were never
dereferenced.
* writes were if-converted via load/maybe-modify/store, which
renders some code multithreading-unsafe.
This patch reimplements if-conversion of loads and stores in a safe
way using a scratchpad allocated by the compiler on the stack:
* loads are done through an indirection, reading either the correct
data from the correct source [if the condition is true] or reading
from the scratchpad and later ignoring this read result [if the
condition is false].
* writes are also done through an indirection, writing either to the
correct destination [if the condition is true] or to the
scratchpad [if the condition is false].
Vectorization of "if-cvt-stores-vect-ifcvt-18.c" disabled because the
old if-conversion resulted in unsafe code that could fail under
multithreading even though the as-written code _was_ thread-safe.
Passed regression testing and bootstrap on amd64-linux.
Is this OK to commit to trunk?
I can't approve or reject but this certainly looks like an improvement
compared to where we are as we get rid of the data races.
Right. I was going to assume the primary purpose to to address
correctness issues, not increase the amount of if-conversion for
optimization purposes.
I have a couple of high level concerns around the scratchpad usage
(aliasing, write-write hazards), but until I dig into the patch I don't
know if they're real issues or not.
Jeff
