On Fri, Mar 9, 2012 at 10:48 PM, Aldy Hernandez <[email protected]> wrote:
>
>> Note that partial PRE (enabled at -O3) can insert expressions into paths
>> that did _not_ execute the expression. For regular PRE you are right.
>>
>> Richard.
>
>
> I've thought about this some more, and Torvald's comment makes a lot of
> sense. PRE can make things completely redundant, and a later pass may move
> things before its publication. I believe it's wise to disable PRE inside
> transactions as originally discussed. The attached patch does so.
>
> Below is an example (from my patch) with an explanation of what may go
> wrong.
>
> Torvald is this what you were thinking of?
>
> Richards, is this OK for the 4.7 branch and trunk (pending tests)?
+ if (flag_tm
+ && gimple_in_transaction (stmt)
+ && gimple_assign_single_p (stmt))
+ {
+ tree rhs = gimple_assign_rhs1 (stmt);
+ if (DECL_P (rhs) && is_global_var (rhs))
+ continue;
this does not cover a read like 'a.b', nor a '*p' read. I think in some other
thread I sketched some ref_may_refer_to_global function, maybe you
remember.
Richard.
> Thanks.
> Aldy
>
> + /* Non local loads in a transaction cannot be hoisted out,
> + because they may make partially redundant expressions
> + totally redundant, which a later pass may move before its
> + publication by another thread.
> +
> + For example:
> +
> + __transaction_atomic {
> + if (flag)
> + y = x + 4;
> + else
> + // stuff
> + z = x + 4;
> + }
> +
> + PRE can rewrite this into:
> +
> + __transaction_atomic {
> + if (flag) {
> + tmp = x + 4;
> + y = tmp;
> + } else {
> + // stuff
> + tmp = x + 4;
> + }
> + z = tmp;
> + }
> +
> + A later pass can move the now totally redundant [x + 4]
> + before its publication predicated by "flag":
> +
> + __transaction_atomic {
> + tmp = x + 4;
> + if (flag) {
> + } else {
> + // stuff
> + }
> + z = tmp;
> + */
