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)?
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;
+ */
PR middle-end/51752
* tree-ssa-pre.c (compute_avail): Disable PRE movements inside a
transaction.
(execute_pre): Compute transaction bits.
Index: testsuite/gcc.dg/tm/pr51752.c
===================================================================
--- testsuite/gcc.dg/tm/pr51752.c (revision 0)
+++ testsuite/gcc.dg/tm/pr51752.c (revision 0)
@@ -0,0 +1,23 @@
+/* { dg-do compile } */
+/* { dg-options "-fgnu-tm -fdump-tree-pre -O2" } */
+
+int flag, hoist, y, z, george;
+
+void
+foo (void)
+{
+ __transaction_atomic
+ {
+ if (george)
+ {
+ if (flag)
+ y = hoist + 4;
+ else
+ flag = 888;
+ z = hoist + 4;
+ }
+ }
+}
+
+/* { dg-final { scan-tree-dump-times "pretmp.*= hoist" 0 "pre" } } */
+/* { dg-final { cleanup-tree-dump "pre" } } */
Index: tree-ssa-pre.c
===================================================================
--- tree-ssa-pre.c (revision 184935)
+++ tree-ssa-pre.c (working copy)
@@ -3986,6 +3986,54 @@ compute_avail (void)
|| stmt_could_throw_p (stmt))
continue;
+ /* 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;
+ */
+ 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;
+ }
+
switch (gimple_code (stmt))
{
case GIMPLE_RETURN:
@@ -4896,6 +4944,9 @@ execute_pre (bool do_fre)
init_pre (do_fre);
scev_initialize ();
+ if (flag_tm)
+ compute_transaction_bits ();
+
/* Collect and value number expressions computed in each basic block. */
compute_avail ();
