| Message ID | 52795DBF.5040806@redhat.com |
|---|---|
| State | New |
| Headers | show |
On 11/05/13 14:06, Andrew MacLeod wrote: > Looks like another location of convenience perhaps... > > Anyway, block_in_transaction (bb) really belongs in basic-block.h... The > only oddity is that it also checks flag_tm... Is this really > necessary? One would think the flag would never be set if flag_tm wasn't > on... > > In any case, basic-block.h is already picking options.h up through > function.h which includes tm.h. And regardless, it does belong here... > > Bootstraps on x86_64-unknown-linux-gnu and current running regressions. > Assuming its clean, OK? OK. I wouldn't lose any sleep of that test were removed. In fact, please remove it :-) I wouldn't want someone to see that code and think "hey, BB_IN_TRANSACTION is guarded, let's reuse that bit for something else when not compiling for TM. We've done far too much of that through the years ;( Jeff
On Tue, Nov 5, 2013 at 10:08 PM, Jeff Law <law@redhat.com> wrote: > On 11/05/13 14:06, Andrew MacLeod wrote: >> >> Looks like another location of convenience perhaps... >> >> Anyway, block_in_transaction (bb) really belongs in basic-block.h... The >> only oddity is that it also checks flag_tm... Is this really >> necessary? One would think the flag would never be set if flag_tm wasn't >> on... >> >> In any case, basic-block.h is already picking options.h up through >> function.h which includes tm.h. And regardless, it does belong here... >> >> Bootstraps on x86_64-unknown-linux-gnu and current running regressions. >> Assuming its clean, OK? > > OK. I wouldn't lose any sleep of that test were removed. In fact, please > remove it :-) I wouldn't want someone to see that code and think "hey, > BB_IN_TRANSACTION is guarded, let's reuse that bit for something else when > not compiling for TM. We've done far too much of that through the years ;( I think it's not computed for !flag_tm, but I agree. Richard. > > Jeff
* gimple.h (block_in_transaction): Move to basic-block.h and rename. (gimple_in_transaction): Use bb_in_transaction. * basic-block.h (bb_in_transaction): Relocate here and rename. * tree-ssa-loop-im.c (execute_sm): Use bb_in_transaction. Index: gimple.h =================================================================== *** gimple.h (revision 204420) --- gimple.h (working copy) *************** gimple_set_has_volatile_ops (gimple stmt *** 1564,1583 **** stmt->gsbase.has_volatile_ops = (unsigned) volatilep; } - /* Return true if BB is in a transaction. */ - - static inline bool - block_in_transaction (basic_block bb) - { - return flag_tm && bb->flags & BB_IN_TRANSACTION; - } - /* Return true if STMT is in a transaction. */ static inline bool gimple_in_transaction (gimple stmt) { ! return block_in_transaction (gimple_bb (stmt)); } /* Return true if statement STMT may access memory. */ --- 1564,1575 ---- stmt->gsbase.has_volatile_ops = (unsigned) volatilep; } /* Return true if STMT is in a transaction. */ static inline bool gimple_in_transaction (gimple stmt) { ! return bb_in_transaction (gimple_bb (stmt)); } /* Return true if statement STMT may access memory. */ Index: basic-block.h =================================================================== *** basic-block.h (revision 204420) --- basic-block.h (working copy) *************** struct loop *get_loop_copy (struct loop *** 897,902 **** --- 897,910 ---- #include "cfghooks.h" + /* Return true if BB is in a transaction. */ + + static inline bool + bb_in_transaction (basic_block bb) + { + return flag_tm && bb->flags & BB_IN_TRANSACTION; + } + /* Return true when one of the predecessor edges of BB is marked with EDGE_EH. */ static inline bool bb_has_eh_pred (basic_block bb) Index: tree-ssa-loop-im.c =================================================================== *** tree-ssa-loop-im.c (revision 204420) --- tree-ssa-loop-im.c (working copy) *************** execute_sm (struct loop *loop, vec<edge> *** 1948,1954 **** fmt_data.orig_loop = loop; for_each_index (&ref->mem.ref, force_move_till, &fmt_data); ! if (block_in_transaction (loop_preheader_edge (loop)->src) || !PARAM_VALUE (PARAM_ALLOW_STORE_DATA_RACES)) multi_threaded_model_p = true; --- 1948,1954 ---- fmt_data.orig_loop = loop; for_each_index (&ref->mem.ref, force_move_till, &fmt_data); ! if (bb_in_transaction (loop_preheader_edge (loop)->src) || !PARAM_VALUE (PARAM_ALLOW_STORE_DATA_RACES)) multi_threaded_model_p = true;