On 11/20/2015 07:08 AM, Bernd Schmidt wrote:
BZ27313 is marked as fixed by the introduction of the tree cselim pass,
thus the problem won't even be seen at RTL level.
Cool.
I'm undecided on whether cs-elim is safe wrt the store speculation vs
locks concerns raised in the thread discussing Ian's
noce_can_store_speculate_p, but that's not something we have to consider
to solve the problem at hand.
I don't think cs-elim is safe WRT locks and such in multi-threaded code.
In particular it replaces this:
bb0:
if (cond) goto bb2; else goto bb1;
bb1:
*p = RHS;
bb2:
with
bb0:
if (cond) goto bb1; else goto bb2;
bb1:
condtmp' = *p;
bb2:
condtmp = PHI <RHS, condtmp'>
*p = condtmp;
If *p is a shared memory location, then there may be another writer. If
that writer happens to store something in that location after the load
of *p, but before the store to *p, then that store will get lost in the
transformed pseudo code.
That seems to introduce a data race. Presumably one would call the
original code ill-formed WRT the C11/C++11 memory model since the shared
location is not marked as such.
I'm willing to consider this an independent problem.
As far as I can tell hmmer and the 27313 testcase are unaffected at -O2
(if anything, hmmer was very slightly faster afterwards). The run wasn't
super-scientific, but I wouldn't have expected anything else given the
existence of cs-elim.
Cool. It doesn't have to be super-scientific. Good to see it didn't
harm anything -- thanks for going the extra mile on this one.
Ok to do the above, removing all the bits made unnecessary (including
memory_must_be_modified_in_insn_p in alias.c)?
Yup. Zap it.
jeff
