public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [patch] 19/n: trans-mem: middle end/misc patches (LAST PATCH)
@ 2011-11-03 19:58 Aldy Hernandez
  2011-11-04 11:22 ` Richard Guenther
  0 siblings, 1 reply; 20+ messages in thread
From: Aldy Hernandez @ 2011-11-03 19:58 UTC (permalink / raw)
  To: gcc-patches

This is everything else that doesn't fit neatly into any other category. 
  Here are the middle end changes, as well as pass ordering code, along 
with varasm and a potpourri of other small changes.

This is the last patch.  Please let me know if there is anything else 
(reasonable) you would like me to post.

Index: gcc/cgraph.h
===================================================================
--- gcc/cgraph.h	(.../trunk)	(revision 180744)
+++ gcc/cgraph.h	(.../branches/transactional-memory)	(revision 180773)
@@ -98,6 +98,9 @@ struct GTY(()) cgraph_local_info {
    /* True when the function has been originally extern inline, but it is
       redefined now.  */
    unsigned redefined_extern_inline : 1;
+
+  /* True if the function may enter serial irrevocable mode.  */
+  unsigned tm_may_enter_irr : 1;
  };

  /* Information about the function that needs to be computed globally
@@ -565,6 +568,8 @@ void verify_cgraph_node (struct cgraph_n
  void cgraph_build_static_cdtor (char which, tree body, int priority);
  void cgraph_reset_static_var_maps (void);
  void init_cgraph (void);
+struct cgraph_node * cgraph_copy_node_for_versioning (struct cgraph_node *,
+		tree, VEC(cgraph_edge_p,heap)*, bitmap);
  struct cgraph_node *cgraph_function_versioning (struct cgraph_node *,
  						VEC(cgraph_edge_p,heap)*,
  						VEC(ipa_replace_map_p,gc)*,
Index: gcc/tree-pass.h
===================================================================
--- gcc/tree-pass.h	(.../trunk)	(revision 180744)
+++ gcc/tree-pass.h	(.../branches/transactional-memory)	(revision 180773)
@@ -447,6 +447,12 @@ extern struct gimple_opt_pass pass_build
  extern struct gimple_opt_pass pass_local_pure_const;
  extern struct gimple_opt_pass pass_tracer;
  extern struct gimple_opt_pass pass_warn_unused_result;
+extern struct gimple_opt_pass pass_diagnose_tm_blocks;
+extern struct gimple_opt_pass pass_lower_tm;
+extern struct gimple_opt_pass pass_tm_init;
+extern struct gimple_opt_pass pass_tm_mark;
+extern struct gimple_opt_pass pass_tm_memopt;
+extern struct gimple_opt_pass pass_tm_edges;
  extern struct gimple_opt_pass pass_split_functions;
  extern struct gimple_opt_pass pass_feedback_split_functions;

@@ -469,6 +475,7 @@ extern struct ipa_opt_pass_d pass_ipa_pu
  extern struct simple_ipa_opt_pass pass_ipa_pta;
  extern struct ipa_opt_pass_d pass_ipa_lto_wpa_fixup;
  extern struct ipa_opt_pass_d pass_ipa_lto_finish_out;
+extern struct simple_ipa_opt_pass pass_ipa_tm;
  extern struct ipa_opt_pass_d pass_ipa_profile;
  extern struct ipa_opt_pass_d pass_ipa_cdtor_merge;

Index: gcc/rtlanal.c
===================================================================
--- gcc/rtlanal.c	(.../trunk)	(revision 180744)
+++ gcc/rtlanal.c	(.../branches/transactional-memory)	(revision 180773)
@@ -1918,6 +1918,7 @@ alloc_reg_note (enum reg_note kind, rtx
      case REG_CC_USER:
      case REG_LABEL_TARGET:
      case REG_LABEL_OPERAND:
+    case REG_TM:
        /* These types of register notes use an INSN_LIST rather than an
  	 EXPR_LIST, so that copying is done right and dumps look
  	 better.  */
Index: gcc/omp-low.c
===================================================================
--- gcc/omp-low.c	(.../trunk)	(revision 180744)
+++ gcc/omp-low.c	(.../branches/transactional-memory)	(revision 180773)
@@ -139,6 +139,7 @@ static tree scan_omp_1_op (tree *, int *
      case GIMPLE_TRY: \
      case GIMPLE_CATCH: \
      case GIMPLE_EH_FILTER: \
+    case GIMPLE_TRANSACTION: \
        /* The sub-statements for these should be walked.  */ \
        *handled_ops_p = false; \
        break;
Index: gcc/toplev.c
===================================================================
--- gcc/toplev.c	(.../trunk)	(revision 180744)
+++ gcc/toplev.c	(.../branches/transactional-memory)	(revision 180773)
@@ -599,6 +599,7 @@ compile_file (void)

        output_shared_constant_pool ();
        output_object_blocks ();
+  finish_tm_clone_pairs ();

        /* Write out any pending weak symbol declarations.  */
        weak_finish ();
Index: gcc/cgraphunit.c
===================================================================
--- gcc/cgraphunit.c	(.../trunk)	(revision 180744)
+++ gcc/cgraphunit.c	(.../branches/transactional-memory)	(revision 180773)
@@ -2272,7 +2272,7 @@ update_call_expr (struct cgraph_node *ne
     was copied to prevent duplications of calls that are dead
     in the clone.  */

-static struct cgraph_node *
+struct cgraph_node *
  cgraph_copy_node_for_versioning (struct cgraph_node *old_version,
  				 tree new_decl,
  				 VEC(cgraph_edge_p,heap) *redirect_callers,
@@ -2286,7 +2286,7 @@ cgraph_copy_node_for_versioning (struct

     new_version = cgraph_create_node (new_decl);

-   new_version->analyzed = true;
+   new_version->analyzed = old_version->analyzed;
     new_version->local = old_version->local;
     new_version->local.externally_visible = false;
     new_version->local.local = true;
@@ -2294,6 +2294,7 @@ cgraph_copy_node_for_versioning (struct
     new_version->rtl = old_version->rtl;
     new_version->reachable = true;
     new_version->count = old_version->count;
+   new_version->lowered = true;

     for (e = old_version->callees; e; e=e->next_callee)
       if (!bbs_to_copy
@@ -2389,7 +2390,6 @@ cgraph_function_versioning (struct cgrap
    DECL_VIRTUAL_P (new_version_node->decl) = 0;
    new_version_node->local.externally_visible = 0;
    new_version_node->local.local = 1;
-  new_version_node->lowered = true;

    /* Update the call_expr on the edges to call the new version node. */
    update_call_expr (new_version_node);
Index: gcc/tree-ssa-alias.c
===================================================================
--- gcc/tree-ssa-alias.c	(.../trunk)	(revision 180744)
+++ gcc/tree-ssa-alias.c	(.../branches/transactional-memory)	(revision 
180773)
@@ -1182,6 +1182,8 @@ ref_maybe_used_by_call_p_1 (gimple call,
  	case BUILT_IN_MEMPCPY:
  	case BUILT_IN_STPCPY:
  	case BUILT_IN_STPNCPY:
+        case BUILT_IN_TM_MEMCPY:
+        case BUILT_IN_TM_MEMMOVE:
  	  {
  	    ao_ref dref;
  	    tree size = NULL_TREE;
@@ -1228,6 +1230,32 @@ ref_maybe_used_by_call_p_1 (gimple call,
  					   size);
  	    return refs_may_alias_p_1 (&dref, ref, false);
  	  }
+
+        /* The following functions read memory pointed to by their
+	   first argument.  */
+	CASE_BUILT_IN_TM_LOAD (1):
+	CASE_BUILT_IN_TM_LOAD (2):
+	CASE_BUILT_IN_TM_LOAD (4):
+	CASE_BUILT_IN_TM_LOAD (8):
+        CASE_BUILT_IN_TM_LOAD (FLOAT):
+	CASE_BUILT_IN_TM_LOAD (DOUBLE):
+	CASE_BUILT_IN_TM_LOAD (LDOUBLE):
+	CASE_BUILT_IN_TM_LOAD (M64):
+	CASE_BUILT_IN_TM_LOAD (M128):
+	CASE_BUILT_IN_TM_LOAD (M256):
+        case BUILT_IN_TM_LOG:
+        case BUILT_IN_TM_LOG_1:
+        case BUILT_IN_TM_LOG_2:
+        case BUILT_IN_TM_LOG_4:
+        case BUILT_IN_TM_LOG_8:
+        case BUILT_IN_TM_LOG_FLOAT:
+        case BUILT_IN_TM_LOG_DOUBLE:
+        case BUILT_IN_TM_LOG_LDOUBLE:
+        case BUILT_IN_TM_LOG_M64:
+        case BUILT_IN_TM_LOG_M128:
+        case BUILT_IN_TM_LOG_M256:
+	  return ptr_deref_may_alias_ref_p_1 (gimple_call_arg (call, 0), ref);
+
  	/* These read memory pointed to by the first argument.  */
  	case BUILT_IN_STRDUP:
  	case BUILT_IN_STRNDUP:
@@ -1250,6 +1278,7 @@ ref_maybe_used_by_call_p_1 (gimple call,
  	case BUILT_IN_STACK_SAVE:
  	case BUILT_IN_STACK_RESTORE:
  	case BUILT_IN_MEMSET:
+        case BUILT_IN_TM_MEMSET:
  	case BUILT_IN_MEMSET_CHK:
  	case BUILT_IN_FREXP:
  	case BUILT_IN_FREXPF:
@@ -1480,6 +1509,19 @@ call_may_clobber_ref_p_1 (gimple call, a
  	case BUILT_IN_STRCAT:
  	case BUILT_IN_STRNCAT:
  	case BUILT_IN_MEMSET:
+        case BUILT_IN_TM_MEMSET:
+        CASE_BUILT_IN_TM_STORE (1):
+        CASE_BUILT_IN_TM_STORE (2):
+        CASE_BUILT_IN_TM_STORE (4):
+        CASE_BUILT_IN_TM_STORE (8):
+        CASE_BUILT_IN_TM_STORE (FLOAT):
+        CASE_BUILT_IN_TM_STORE (DOUBLE):
+        CASE_BUILT_IN_TM_STORE (LDOUBLE):
+        CASE_BUILT_IN_TM_STORE (M64):
+        CASE_BUILT_IN_TM_STORE (M128):
+        CASE_BUILT_IN_TM_STORE (M256):
+        case BUILT_IN_TM_MEMCPY:
+        case BUILT_IN_TM_MEMMOVE:
  	  {
  	    ao_ref dref;
  	    tree size = NULL_TREE;
Index: gcc/ipa-inline.c
===================================================================
--- gcc/ipa-inline.c	(.../trunk)	(revision 180744)
+++ gcc/ipa-inline.c	(.../branches/transactional-memory)	(revision 180773)
@@ -284,6 +284,15 @@ can_inline_edge_p (struct cgraph_edge *e
        e->inline_failed = CIF_EH_PERSONALITY;
        inlinable = false;
      }
+  /* TM pure functions should not get inlined if the outer function is
+     a TM safe function.  */
+  else if (flag_tm
+	   && is_tm_pure (callee->decl)
+	   && is_tm_safe (e->caller->decl))
+    {
+      e->inline_failed = CIF_UNSPECIFIED;
+      inlinable = false;
+    }
    /* Don't inline if the callee can throw non-call exceptions but the
       caller cannot.
       FIXME: this is obviously wrong for LTO where STRUCT_FUNCTION is 
missing.
Index: gcc/crtstuff.c
===================================================================
--- gcc/crtstuff.c	(.../trunk)	(revision 180744)
+++ gcc/crtstuff.c	(.../branches/transactional-memory)	(revision 180773)
@@ -162,6 +162,9 @@ extern void __do_global_ctors_1 (void);
  /* Likewise for _Jv_RegisterClasses.  */
  extern void _Jv_RegisterClasses (void *) TARGET_ATTRIBUTE_WEAK;

+extern void _ITM_registerTMCloneTable (void *, size_t) 
TARGET_ATTRIBUTE_WEAK;
+extern void _ITM_deregisterTMCloneTable (void *) TARGET_ATTRIBUTE_WEAK;
+
  #ifdef OBJECT_FORMAT_ELF

  /*  Declare a pointer to void function type.  */
@@ -241,6 +244,11 @@ STATIC void *__JCR_LIST__[]
    = { };
  #endif /* JCR_SECTION_NAME */

+STATIC func_ptr __TMC_LIST__[]
+  __attribute__((unused, section(".tm_clone_table"), 
aligned(sizeof(void*))))
+  = { };
+extern func_ptr __TMC_END__[] __attribute__((__visibility__ ("hidden")));
+
  #if defined(INIT_SECTION_ASM_OP) || defined(INIT_ARRAY_SECTION_ASM_OP)

  #ifdef OBJECT_FORMAT_ELF
@@ -330,6 +338,13 @@ __do_global_dtors_aux (void)
    }
  #endif /* !defined(FINI_ARRAY_SECTION_ASM_OP) */

+  if (_ITM_deregisterTMCloneTable)
+    {
+      size_t size = (size_t)(__TMC_END__ - __TMC_LIST__) / 2;
+      if (size > 0)
+	_ITM_deregisterTMCloneTable (__TMC_LIST__);
+    }
+
  #ifdef USE_EH_FRAME_REGISTRY
  #ifdef CRT_GET_RFIB_DATA
    /* If we used the new __register_frame_info_bases interface,
@@ -391,6 +406,12 @@ frame_dummy (void)
  	register_classes (__JCR_LIST__);
      }
  #endif /* JCR_SECTION_NAME */
+  if (_ITM_registerTMCloneTable)
+    {
+      size_t size = (size_t)(__TMC_END__ - __TMC_LIST__) / 2;
+      if (size > 0)
+	_ITM_registerTMCloneTable (__TMC_LIST__, size);
+    }
  }

  #ifdef INIT_SECTION_ASM_OP
@@ -457,6 +478,13 @@ __do_global_dtors (void)
    for (p = __DTOR_LIST__ + 1; (f = *p); p++)
      f ();

+  if (_ITM_deregisterTMCloneTable)
+    {
+      size_t size = (size_t)(__TMC_END__ - __TMC_LIST__) / 2;
+      if (size > 0)
+	_ITM_deregisterTMCloneTable (__TMC_LIST__);
+    }
+
  #ifdef USE_EH_FRAME_REGISTRY
    if (__deregister_frame_info)
      __deregister_frame_info (__EH_FRAME_BEGIN__);
@@ -570,6 +598,11 @@ STATIC void *__JCR_END__[1]
     = { 0 };
  #endif /* JCR_SECTION_NAME */

+func_ptr __TMC_END__[]
+  __attribute__((unused, section(".tm_clone_table"), 
aligned(sizeof(void *)),
+		 __visibility__ ("hidden")))
+  = { };
+
  #ifdef INIT_ARRAY_SECTION_ASM_OP

  /* If we are using .init_array, there is nothing to do.  */
Index: gcc/cfgbuild.c
===================================================================
--- gcc/cfgbuild.c	(.../trunk)	(revision 180744)
+++ gcc/cfgbuild.c	(.../branches/transactional-memory)	(revision 180773)
@@ -338,18 +338,30 @@ make_edges (basic_block min, basic_block
  	  /* Add any appropriate EH edges.  */
  	  rtl_make_eh_edge (edge_cache, bb, insn);

-	  if (code == CALL_INSN && nonlocal_goto_handler_labels)
+	  if (code == CALL_INSN)
  	    {
-	      /* ??? This could be made smarter: in some cases it's possible
-		 to tell that certain calls will not do a nonlocal goto.
-		 For example, if the nested functions that do the nonlocal
-		 gotos do not have their addresses taken, then only calls to
-		 those functions or to other nested functions that use them
-		 could possibly do nonlocal gotos.  */
  	      if (can_nonlocal_goto (insn))
-		for (x = nonlocal_goto_handler_labels; x; x = XEXP (x, 1))
-		  make_label_edge (edge_cache, bb, XEXP (x, 0),
-				   EDGE_ABNORMAL | EDGE_ABNORMAL_CALL);
+		{
+		  /* ??? This could be made smarter: in some cases it's
+		     possible to tell that certain calls will not do a
+		     nonlocal goto.  For example, if the nested functions
+		     that do the nonlocal gotos do not have their addresses
+		     taken, then only calls to those functions or to other
+		     nested functions that use them could possibly do
+		     nonlocal gotos.  */
+		  for (x = nonlocal_goto_handler_labels; x; x = XEXP (x, 1))
+		    make_label_edge (edge_cache, bb, XEXP (x, 0),
+				     EDGE_ABNORMAL | EDGE_ABNORMAL_CALL);
+		}
+
+	      if (flag_tm)
+		{
+		  rtx note;
+		  for (note = REG_NOTES (insn); note; note = XEXP (note, 1))
+		    if (REG_NOTE_KIND (note) == REG_TM)
+		      make_label_edge (edge_cache, bb, XEXP (note, 0),
+				       EDGE_ABNORMAL | EDGE_ABNORMAL_CALL);
+		}
  	    }
  	}

Index: gcc/timevar.def
===================================================================
--- gcc/timevar.def	(.../trunk)	(revision 180744)
+++ gcc/timevar.def	(.../branches/transactional-memory)	(revision 180773)
@@ -184,6 +184,7 @@ DEFTIMEVAR (TV_TREE_COPY_RENAME	     , "
  DEFTIMEVAR (TV_TREE_SSA_VERIFY       , "tree SSA verifier")
  DEFTIMEVAR (TV_TREE_STMT_VERIFY      , "tree STMT verifier")
  DEFTIMEVAR (TV_TREE_SWITCH_CONVERSION, "tree switch initialization 
conversion")
+DEFTIMEVAR (TV_TRANS_MEM             , "transactional memory")
  DEFTIMEVAR (TV_TREE_STRLEN           , "tree strlen optimization")
  DEFTIMEVAR (TV_CGRAPH_VERIFY         , "callgraph verifier")
  DEFTIMEVAR (TV_DOM_FRONTIERS         , "dominance frontiers")
Index: gcc/recog.c
===================================================================
--- gcc/recog.c	(.../trunk)	(revision 180744)
+++ gcc/recog.c	(.../branches/transactional-memory)	(revision 180773)
@@ -3287,6 +3287,7 @@ peep2_attempt (basic_block bb, rtx insn,
  	  {
  	  case REG_NORETURN:
  	  case REG_SETJMP:
+	  case REG_TM:
  	    add_reg_note (new_insn, REG_NOTE_KIND (note),
  			  XEXP (note, 0));
  	    break;
Index: gcc/function.h
===================================================================
--- gcc/function.h	(.../trunk)	(revision 180744)
+++ gcc/function.h	(.../branches/transactional-memory)	(revision 180773)
@@ -467,6 +467,14 @@ extern GTY(()) struct rtl_data x_rtl;
     want to do differently.  */
  #define crtl (&x_rtl)

+/* This structure is used to map a gimple statement to a label,
+   or list of labels to represent transaction restart.  */
+
+struct GTY(()) tm_restart_node {
+  gimple stmt;
+  tree label_or_list;
+};
+
  struct GTY(()) stack_usage
  {
    /* # of bytes of static stack space allocated by the function.  */
@@ -518,6 +526,10 @@ struct GTY(()) function {
    /* Value histograms attached to particular statements.  */
    htab_t GTY((skip)) value_histograms;

+  /* Map gimple stmt to tree label (or list of labels) for transaction
+     restart and abort.  */
+  htab_t GTY ((param_is (struct tm_restart_node))) tm_restart;
+
    /* For function.c.  */

    /* Points to the FUNCTION_DECL of this function.  */
Index: gcc/emit-rtl.c
===================================================================
--- gcc/emit-rtl.c	(.../trunk)	(revision 180744)
+++ gcc/emit-rtl.c	(.../branches/transactional-memory)	(revision 180773)
@@ -3595,6 +3595,7 @@ try_split (rtx pat, rtx trial, int last)

  	case REG_NORETURN:
  	case REG_SETJMP:
+	case REG_TM:
  	  for (insn = insn_last; insn != NULL_RTX; insn = PREV_INSN (insn))
  	    {
  	      if (CALL_P (insn))
Index: gcc/cfgexpand.c
===================================================================
--- gcc/cfgexpand.c	(.../trunk)	(revision 180744)
+++ gcc/cfgexpand.c	(.../branches/transactional-memory)	(revision 180773)
@@ -2096,6 +2096,32 @@ expand_gimple_stmt (gimple stmt)
  	}
      }

+  /* Mark all calls that can have a transaction restart.  */
+  if (cfun->tm_restart && is_gimple_call (stmt))
+    {
+      struct tm_restart_node dummy;
+      void **slot;
+
+      dummy.stmt = stmt;
+      slot = htab_find_slot (cfun->tm_restart, &dummy, NO_INSERT);
+      if (slot)
+	{
+	  struct tm_restart_node *n = (struct tm_restart_node *) *slot;
+	  tree list = n->label_or_list;
+	  rtx insn;
+
+	  for (insn = next_real_insn (last); !CALL_P (insn);
+	       insn = next_real_insn (insn))
+	    continue;
+
+	  if (TREE_CODE (list) == LABEL_DECL)
+	    add_reg_note (insn, REG_TM, label_rtx (list));
+	  else
+	    for (; list ; list = TREE_CHAIN (list))
+	      add_reg_note (insn, REG_TM, label_rtx (TREE_VALUE (list)));
+	}
+    }
+
    return last;
  }

@@ -4455,6 +4481,10 @@ gimple_expand_cfg (void)
    /* After expanding, the return labels are no longer needed. */
    return_label = NULL;
    naked_return_label = NULL;
+
+  /* After expanding, the tm_restart map is no longer needed.  */
+  cfun->tm_restart = NULL;
+
    /* Tag the blocks with a depth number so that change_scope can find
       the common parent easily.  */
    set_block_levels (DECL_INITIAL (cfun->decl), 0);
Index: gcc/varasm.c
===================================================================
--- gcc/varasm.c	(.../trunk)	(revision 180744)
+++ gcc/varasm.c	(.../branches/transactional-memory)	(revision 180773)
@@ -5859,6 +5859,103 @@ assemble_alias (tree decl, tree target)
      }
  }

+/* Record and output a table of translations from original function
+   to its transaction aware clone.  Note that tm_pure functions are
+   considered to be their own clone.  */
+
+static GTY((if_marked ("tree_map_marked_p"), param_is (struct tree_map)))
+     htab_t tm_clone_pairs;
+
+void
+record_tm_clone_pair (tree o, tree n)
+{
+  struct tree_map **slot, *h;
+
+  if (tm_clone_pairs == NULL)
+    tm_clone_pairs = htab_create_ggc (32, tree_map_hash, tree_map_eq, 0);
+
+  h = ggc_alloc_tree_map ();
+  h->hash = htab_hash_pointer (o);
+  h->base.from = o;
+  h->to = n;
+
+  slot = (struct tree_map **)
+    htab_find_slot_with_hash (tm_clone_pairs, h, h->hash, INSERT);
+  *slot = h;
+}
+
+tree
+get_tm_clone_pair (tree o)
+{
+  if (tm_clone_pairs)
+    {
+      struct tree_map *h, in;
+
+      in.base.from = o;
+      in.hash = htab_hash_pointer (o);
+      h = (struct tree_map *) htab_find_with_hash (tm_clone_pairs,
+						   &in, in.hash);
+      if (h)
+	return h->to;
+    }
+  return NULL_TREE;
+}
+
+/* Helper function for finish_tm_clone_pairs.  Dump the clone table.  */
+
+int
+finish_tm_clone_pairs_1 (void **slot, void *info ATTRIBUTE_UNUSED)
+{
+  struct tree_map *map = (struct tree_map *) *slot;
+  bool *switched = (bool *) info;
+  tree src = map->base.from;
+  tree dst = map->to;
+  struct cgraph_node *src_n = cgraph_get_node (src);
+  struct cgraph_node *dst_n = cgraph_get_node (dst);
+
+  /* The function ipa_tm_create_version() marks the clone as needed if
+     the original function was needed.  But we also mark the clone as
+     needed if we ever called the clone indirectly through
+     TM_GETTMCLONE.  If neither of these are true, we didn't generate
+     a clone, and we didn't call it indirectly... no sense keeping it
+     in the clone table.  */
+  if (!dst_n || !dst_n->needed)
+    return 1;
+
+  /* This covers the case where we have optimized the original
+     function away, and only access the transactional clone.  */
+  if (!src_n || !src_n->needed)
+    return 1;
+
+  if (!*switched)
+    {
+      switch_to_section (get_named_section (NULL, ".tm_clone_table", 3));
+      assemble_align (POINTER_SIZE);
+      *switched = true;
+    }
+
+  assemble_integer (XEXP (DECL_RTL (src), 0),
+		    POINTER_SIZE / BITS_PER_UNIT, POINTER_SIZE, 1);
+  assemble_integer (XEXP (DECL_RTL (dst), 0),
+		    POINTER_SIZE / BITS_PER_UNIT, POINTER_SIZE, 1);
+  return 1;
+}
+
+void
+finish_tm_clone_pairs (void)
+{
+  bool switched = false;
+
+  if (tm_clone_pairs == NULL)
+    return;
+
+  htab_traverse_noresize (tm_clone_pairs, finish_tm_clone_pairs_1,
+			  (void *) &switched);
+  htab_delete (tm_clone_pairs);
+  tm_clone_pairs = NULL;
+}
+
+
  /* Emit an assembler directive to set symbol for DECL visibility to
     the visibility type VIS, which must not be VISIBILITY_DEFAULT.  */

Index: gcc/output.h
===================================================================
--- gcc/output.h	(.../trunk)	(revision 180744)
+++ gcc/output.h	(.../branches/transactional-memory)	(revision 180773)
@@ -606,6 +606,11 @@ extern bool unlikely_text_section_p (sec
  extern void switch_to_section (section *);
  extern void output_section_asm_op (const void *);

+extern void record_tm_clone_pair (tree, tree);
+extern void finish_tm_clone_pairs (void);
+extern int finish_tm_clone_pairs_1 (void **, void *);
+extern tree get_tm_clone_pair (tree);
+
  extern void default_asm_output_source_filename (FILE *, const char *);
  extern void output_file_directive (FILE *, const char *);

Index: gcc/combine.c
===================================================================
--- gcc/combine.c	(.../trunk)	(revision 180744)
+++ gcc/combine.c	(.../branches/transactional-memory)	(revision 180773)
@@ -13286,6 +13286,7 @@ distribute_notes (rtx notes, rtx from_in

  	case REG_NORETURN:
  	case REG_SETJMP:
+	case REG_TM:
  	  /* These notes must remain with the call.  It should not be
  	     possible for both I2 and I3 to be a call.  */
  	  if (CALL_P (i3))
Index: gcc/tree-flow.h
===================================================================
--- gcc/tree-flow.h	(.../trunk)	(revision 180744)
+++ gcc/tree-flow.h	(.../branches/transactional-memory)	(revision 180773)
@@ -778,6 +778,9 @@ extern bool maybe_duplicate_eh_stmt (gim
  extern bool verify_eh_edges (gimple);
  extern bool verify_eh_dispatch_edge (gimple);

+/* In gtm-low.c  */
+extern bool is_transactional_stmt (const_gimple);
+
  /* In tree-ssa-pre.c  */
  struct pre_expr_d;
  void add_to_value (unsigned int, struct pre_expr_d *);
Index: gcc/tree-ssa-structalias.c
===================================================================
--- gcc/tree-ssa-structalias.c	(.../trunk)	(revision 180744)
+++ gcc/tree-ssa-structalias.c	(.../branches/transactional-memory) 
(revision 180773)
@@ -4024,6 +4024,8 @@ find_func_aliases_for_builtin_call (gimp
        case BUILT_IN_STPCPY_CHK:
        case BUILT_IN_STRCAT_CHK:
        case BUILT_IN_STRNCAT_CHK:
+      case BUILT_IN_TM_MEMCPY:
+      case BUILT_IN_TM_MEMMOVE:
  	{
  	  tree res = gimple_call_lhs (t);
  	  tree dest = gimple_call_arg (t, (DECL_FUNCTION_CODE (fndecl)
@@ -4056,6 +4058,7 @@ find_func_aliases_for_builtin_call (gimp
  	}
        case BUILT_IN_MEMSET:
        case BUILT_IN_MEMSET_CHK:
+      case BUILT_IN_TM_MEMSET:
  	{
  	  tree res = gimple_call_lhs (t);
  	  tree dest = gimple_call_arg (t, 0);
@@ -4197,6 +4200,50 @@ find_func_aliases_for_builtin_call (gimp
  	    }
  	  return true;
  	}
+      CASE_BUILT_IN_TM_STORE (1):
+      CASE_BUILT_IN_TM_STORE (2):
+      CASE_BUILT_IN_TM_STORE (4):
+      CASE_BUILT_IN_TM_STORE (8):
+      CASE_BUILT_IN_TM_STORE (FLOAT):
+      CASE_BUILT_IN_TM_STORE (DOUBLE):
+      CASE_BUILT_IN_TM_STORE (LDOUBLE):
+      CASE_BUILT_IN_TM_STORE (M64):
+      CASE_BUILT_IN_TM_STORE (M128):
+      CASE_BUILT_IN_TM_STORE (M256):
+	{
+	  tree addr = gimple_call_arg (t, 0);
+	  tree src = gimple_call_arg (t, 1);
+
+	  get_constraint_for (addr, &lhsc);
+	  do_deref (&lhsc);
+	  get_constraint_for (src, &rhsc);
+	  process_all_all_constraints (lhsc, rhsc);
+	  VEC_free (ce_s, heap, lhsc);
+	  VEC_free (ce_s, heap, rhsc);
+	  return true;
+	}
+      CASE_BUILT_IN_TM_LOAD (1):
+      CASE_BUILT_IN_TM_LOAD (2):
+      CASE_BUILT_IN_TM_LOAD (4):
+      CASE_BUILT_IN_TM_LOAD (8):
+      CASE_BUILT_IN_TM_LOAD (FLOAT):
+      CASE_BUILT_IN_TM_LOAD (DOUBLE):
+      CASE_BUILT_IN_TM_LOAD (LDOUBLE):
+      CASE_BUILT_IN_TM_LOAD (M64):
+      CASE_BUILT_IN_TM_LOAD (M128):
+      CASE_BUILT_IN_TM_LOAD (M256):
+        {
+	  tree dest = gimple_call_lhs (t);
+	  tree addr = gimple_call_arg (t, 0);
+
+	  get_constraint_for (dest, &lhsc);
+	  get_constraint_for (addr, &rhsc);
+	  do_deref (&rhsc);
+	  process_all_all_constraints (lhsc, rhsc);
+	  VEC_free (ce_s, heap, lhsc);
+	  VEC_free (ce_s, heap, rhsc);
+	  return true;
+        }
        /* Variadic argument handling needs to be handled in IPA
  	 mode as well.  */
        case BUILT_IN_VA_START:
Index: gcc/tree-cfg.c
===================================================================
--- gcc/tree-cfg.c	(.../trunk)	(revision 180744)
+++ gcc/tree-cfg.c	(.../branches/transactional-memory)	(revision 180773)
@@ -666,6 +666,15 @@ make_edges (void)
  		}
  	      break;

+	    case GIMPLE_TRANSACTION:
+	      {
+		tree abort_label = gimple_transaction_label (last);
+		if (abort_label)
+		  make_edge (bb, label_to_block (abort_label), 0);
+		fallthru = true;
+	      }
+	      break;
+
  	    default:
  	      gcc_assert (!stmt_ends_bb_p (last));
  	      fallthru = true;
@@ -1196,22 +1205,30 @@ cleanup_dead_labels (void)
    FOR_EACH_BB (bb)
      {
        gimple stmt = last_stmt (bb);
+      tree label, new_label;
+
        if (!stmt)
  	continue;

        switch (gimple_code (stmt))
  	{
  	case GIMPLE_COND:
-	  {
-	    tree true_label = gimple_cond_true_label (stmt);
-	    tree false_label = gimple_cond_false_label (stmt);
+	  label = gimple_cond_true_label (stmt);
+	  if (label)
+	    {
+	      new_label = main_block_label (label);
+	      if (new_label != label)
+		gimple_cond_set_true_label (stmt, new_label);
+	    }

-	    if (true_label)
-	      gimple_cond_set_true_label (stmt, main_block_label (true_label));
-	    if (false_label)
-	      gimple_cond_set_false_label (stmt, main_block_label (false_label));
-	    break;
-	  }
+	  label = gimple_cond_false_label (stmt);
+	  if (label)
+	    {
+	      new_label = main_block_label (label);
+	      if (new_label != label)
+		gimple_cond_set_false_label (stmt, new_label);
+	    }
+	  break;

  	case GIMPLE_SWITCH:
  	  {
@@ -1221,8 +1238,10 @@ cleanup_dead_labels (void)
  	    for (i = 0; i < n; ++i)
  	      {
  		tree case_label = gimple_switch_label (stmt, i);
-		tree label = main_block_label (CASE_LABEL (case_label));
-		CASE_LABEL (case_label) = label;
+		label = CASE_LABEL (case_label);
+		new_label = main_block_label (label);
+		if (new_label != label)
+		  CASE_LABEL (case_label) = new_label;
  	      }
  	    break;
  	  }
@@ -1243,13 +1262,27 @@ cleanup_dead_labels (void)
  	/* We have to handle gotos until they're removed, and we don't
  	   remove them until after we've created the CFG edges.  */
  	case GIMPLE_GOTO:
-          if (!computed_goto_p (stmt))
+	  if (!computed_goto_p (stmt))
  	    {
-	      tree new_dest = main_block_label (gimple_goto_dest (stmt));
-	      gimple_goto_set_dest (stmt, new_dest);
+	      label = gimple_goto_dest (stmt);
+	      new_label = main_block_label (label);
+	      if (new_label != label)
+		gimple_goto_set_dest (stmt, new_label);
  	    }
  	  break;

+	case GIMPLE_TRANSACTION:
+	  {
+	    tree label = gimple_transaction_label (stmt);
+	    if (label)
+	      {
+		tree new_label = main_block_label (label);
+		if (new_label != label)
+		  gimple_transaction_set_label (stmt, new_label);
+	      }
+	  }
+	  break;
+
  	default:
  	  break;
        }
@@ -2263,6 +2296,13 @@ is_ctrl_altering_stmt (gimple t)
  	if (flags & ECF_NORETURN)
  	  return true;

+	/* TM ending statements have backedges out of the transaction.
+	   Return true so we split the basic block containing
+	   them.  */
+	if ((flags & ECF_TM_OPS)
+	    && is_tm_ending_fndecl (gimple_call_fndecl (t)))
+	  return true;
+
  	/* BUILT_IN_RETURN call is same as return statement.  */
  	if (gimple_call_builtin_p (t, BUILT_IN_RETURN))
  	  return true;
@@ -2284,6 +2324,10 @@ is_ctrl_altering_stmt (gimple t)
        /* OpenMP directives alter control flow.  */
        return true;

+    case GIMPLE_TRANSACTION:
+      /* A transaction start alters control flow.  */
+      return true;
+
      default:
        break;
      }
@@ -4054,6 +4098,17 @@ verify_gimple_switch (gimple stmt)
    return false;
  }

+/* Verify the contents of a GIMPLE_TRANSACTION.  Returns true if there
+   is a problem, otherwise false.  */
+
+static bool
+verify_gimple_transaction (gimple stmt)
+{
+  tree lab = gimple_transaction_label (stmt);
+  if (lab != NULL && TREE_CODE (lab) != LABEL_DECL)
+    return true;
+  return false;
+}

  /* Verify a gimple debug statement STMT.
     Returns true if anything is wrong.  */
@@ -4155,6 +4210,9 @@ verify_gimple_stmt (gimple stmt)
      case GIMPLE_ASM:
        return false;

+    case GIMPLE_TRANSACTION:
+      return verify_gimple_transaction (stmt);
+
      /* Tuples that do not have tree operands.  */
      case GIMPLE_NOP:
      case GIMPLE_PREDICT:
@@ -4271,10 +4329,19 @@ verify_gimple_in_seq_2 (gimple_seq stmts
  	  err |= verify_gimple_in_seq_2 (gimple_eh_filter_failure (stmt));
  	  break;

+	case GIMPLE_EH_ELSE:
+	  err |= verify_gimple_in_seq_2 (gimple_eh_else_n_body (stmt));
+	  err |= verify_gimple_in_seq_2 (gimple_eh_else_e_body (stmt));
+	  break;
+
  	case GIMPLE_CATCH:
  	  err |= verify_gimple_in_seq_2 (gimple_catch_handler (stmt));
  	  break;

+	case GIMPLE_TRANSACTION:
+	  err |= verify_gimple_in_seq_2 (gimple_transaction_body (stmt));
+	  break;
+
  	default:
  	  {
  	    bool err2 = verify_gimple_stmt (stmt);
@@ -5052,6 +5119,14 @@ gimple_redirect_edge_and_branch (edge e,
  	redirect_eh_dispatch_edge (stmt, e, dest);
        break;

+    case GIMPLE_TRANSACTION:
+      /* The ABORT edge has a stored label associated with it, otherwise
+	 the edges are simply redirectable.  */
+      /* ??? We don't really need this label after the cfg is created.  */
+      if (e->flags == 0)
+	gimple_transaction_set_label (stmt, gimple_block_label (dest));
+      break;
+
      default:
        /* Otherwise it must be a fallthru edge, and we don't need to
  	 do anything besides redirecting it.  */
@@ -6428,8 +6503,10 @@ dump_function_to_file (tree fn, FILE *fi
    bool ignore_topmost_bind = false, any_var = false;
    basic_block bb;
    tree chain;
+  bool tmclone = TREE_CODE (fn) == FUNCTION_DECL && DECL_IS_TM_CLONE (fn);

-  fprintf (file, "%s (", lang_hooks.decl_printable_name (fn, 2));
+  fprintf (file, "%s %s(", lang_hooks.decl_printable_name (fn, 2),
+	   tmclone ? "[tm-clone] " : "");

    arg = DECL_ARGUMENTS (fn);
    while (arg)
Index: gcc/passes.c
===================================================================
--- gcc/passes.c	(.../trunk)	(revision 180744)
+++ gcc/passes.c	(.../branches/transactional-memory)	(revision 180773)
@@ -1174,9 +1174,11 @@ init_optimization_passes (void)
    p = &all_lowering_passes;
    NEXT_PASS (pass_warn_unused_result);
    NEXT_PASS (pass_diagnose_omp_blocks);
+  NEXT_PASS (pass_diagnose_tm_blocks);
    NEXT_PASS (pass_mudflap_1);
    NEXT_PASS (pass_lower_omp);
    NEXT_PASS (pass_lower_cf);
+  NEXT_PASS (pass_lower_tm);
    NEXT_PASS (pass_refactor_eh);
    NEXT_PASS (pass_lower_eh);
    NEXT_PASS (pass_build_cfg);
@@ -1241,6 +1243,7 @@ init_optimization_passes (void)
      }
    NEXT_PASS (pass_ipa_increase_alignment);
    NEXT_PASS (pass_ipa_matrix_reorg);
+  NEXT_PASS (pass_ipa_tm);
    NEXT_PASS (pass_ipa_lower_emutls);
    *p = NULL;

@@ -1400,6 +1403,13 @@ init_optimization_passes (void)
        NEXT_PASS (pass_uncprop);
        NEXT_PASS (pass_local_pure_const);
      }
+  NEXT_PASS (pass_tm_init);
+    {
+      struct opt_pass **p = &pass_tm_init.pass.sub;
+      NEXT_PASS (pass_tm_mark);
+      NEXT_PASS (pass_tm_memopt);
+      NEXT_PASS (pass_tm_edges);
+    }
    NEXT_PASS (pass_lower_complex_O0);
    NEXT_PASS (pass_cleanup_eh);
    NEXT_PASS (pass_lower_resx);
Index: gcc/reg-notes.def
===================================================================
--- gcc/reg-notes.def	(.../trunk)	(revision 180744)
+++ gcc/reg-notes.def	(.../branches/transactional-memory)	(revision 180773)
@@ -203,6 +203,11 @@ REG_NOTE (CROSSING_JUMP)
     functions that can return twice.  */
  REG_NOTE (SETJMP)

+/* This kind of note is generated at each transactional memory
+   builtin, to indicate we need to generate transaction restart
+   edges for this insn.  */
+REG_NOTE (TM)
+
  /* Indicates the cumulative offset of the stack pointer accounting
     for pushed arguments.  This will only be generated when
     ACCUMULATE_OUTGOING_ARGS is false.  */
Index: gcc/cfgrtl.c
===================================================================
--- gcc/cfgrtl.c	(.../trunk)	(revision 180744)
+++ gcc/cfgrtl.c	(.../branches/transactional-memory)	(revision 180773)
@@ -2246,6 +2246,8 @@ purge_dead_edges (basic_block bb)
  	    ;
  	  else if ((e->flags & EDGE_EH) && can_throw_internal (insn))
  	    ;
+	  else if (flag_tm && find_reg_note (insn, REG_TM, NULL))
+	    ;
  	  else
  	    remove = true;
  	}
Index: gcc/params.def
===================================================================
--- gcc/params.def	(.../trunk)	(revision 180744)
+++ gcc/params.def	(.../branches/transactional-memory)	(revision 180773)
@@ -872,6 +872,13 @@ DEFPARAM (PARAM_IPA_SRA_PTR_GROWTH_FACTO
  	  "a pointer to an aggregate with",
  	  2, 0, 0)

+DEFPARAM (PARAM_TM_MAX_AGGREGATE_SIZE,
+	  "tm-max-aggregate-size",
+	  "Size in bytes after which thread-local aggregates should be "
+	  "instrumented with the logging functions instead of save/restore "
+	  "pairs",
+	  9, 0, 0)
+
  DEFPARAM (PARAM_IPA_CP_VALUE_LIST_SIZE,
  	  "ipa-cp-value-list-size",
  	  "Maximum size of a list of values associated with each parameter for "

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [patch] 19/n: trans-mem: middle end/misc patches (LAST PATCH)
  2011-11-03 19:58 [patch] 19/n: trans-mem: middle end/misc patches (LAST PATCH) Aldy Hernandez
@ 2011-11-04 11:22 ` Richard Guenther
  2011-11-06 19:07   ` Aldy Hernandez
  0 siblings, 1 reply; 20+ messages in thread
From: Richard Guenther @ 2011-11-04 11:22 UTC (permalink / raw)
  To: Aldy Hernandez; +Cc: gcc-patches

On Thu, Nov 3, 2011 at 8:32 PM, Aldy Hernandez <aldyh@redhat.com> wrote:
> This is everything else that doesn't fit neatly into any other category.
>  Here are the middle end changes, as well as pass ordering code, along with
> varasm and a potpourri of other small changes.
>
> This is the last patch.  Please let me know if there is anything else
> (reasonable) you would like me to post.
>
> Index: gcc/cgraph.h
> ===================================================================
> --- gcc/cgraph.h        (.../trunk)     (revision 180744)
> +++ gcc/cgraph.h        (.../branches/transactional-memory)     (revision
> 180773)
> @@ -98,6 +98,9 @@ struct GTY(()) cgraph_local_info {
>   /* True when the function has been originally extern inline, but it is
>      redefined now.  */
>   unsigned redefined_extern_inline : 1;
> +
> +  /* True if the function may enter serial irrevocable mode.  */
> +  unsigned tm_may_enter_irr : 1;
>  };
>
>  /* Information about the function that needs to be computed globally
> @@ -565,6 +568,8 @@ void verify_cgraph_node (struct cgraph_n
>  void cgraph_build_static_cdtor (char which, tree body, int priority);
>  void cgraph_reset_static_var_maps (void);
>  void init_cgraph (void);
> +struct cgraph_node * cgraph_copy_node_for_versioning (struct cgraph_node *,
> +               tree, VEC(cgraph_edge_p,heap)*, bitmap);
>  struct cgraph_node *cgraph_function_versioning (struct cgraph_node *,
>                                                VEC(cgraph_edge_p,heap)*,
>                                                VEC(ipa_replace_map_p,gc)*,
> Index: gcc/tree-pass.h
> ===================================================================
> --- gcc/tree-pass.h     (.../trunk)     (revision 180744)
> +++ gcc/tree-pass.h     (.../branches/transactional-memory)     (revision
> 180773)
> @@ -447,6 +447,12 @@ extern struct gimple_opt_pass pass_build
>  extern struct gimple_opt_pass pass_local_pure_const;
>  extern struct gimple_opt_pass pass_tracer;
>  extern struct gimple_opt_pass pass_warn_unused_result;
> +extern struct gimple_opt_pass pass_diagnose_tm_blocks;
> +extern struct gimple_opt_pass pass_lower_tm;
> +extern struct gimple_opt_pass pass_tm_init;
> +extern struct gimple_opt_pass pass_tm_mark;
> +extern struct gimple_opt_pass pass_tm_memopt;
> +extern struct gimple_opt_pass pass_tm_edges;
>  extern struct gimple_opt_pass pass_split_functions;
>  extern struct gimple_opt_pass pass_feedback_split_functions;
>
> @@ -469,6 +475,7 @@ extern struct ipa_opt_pass_d pass_ipa_pu
>  extern struct simple_ipa_opt_pass pass_ipa_pta;
>  extern struct ipa_opt_pass_d pass_ipa_lto_wpa_fixup;
>  extern struct ipa_opt_pass_d pass_ipa_lto_finish_out;
> +extern struct simple_ipa_opt_pass pass_ipa_tm;
>  extern struct ipa_opt_pass_d pass_ipa_profile;
>  extern struct ipa_opt_pass_d pass_ipa_cdtor_merge;
>
> Index: gcc/rtlanal.c
> ===================================================================
> --- gcc/rtlanal.c       (.../trunk)     (revision 180744)
> +++ gcc/rtlanal.c       (.../branches/transactional-memory)     (revision
> 180773)
> @@ -1918,6 +1918,7 @@ alloc_reg_note (enum reg_note kind, rtx
>     case REG_CC_USER:
>     case REG_LABEL_TARGET:
>     case REG_LABEL_OPERAND:
> +    case REG_TM:
>       /* These types of register notes use an INSN_LIST rather than an
>         EXPR_LIST, so that copying is done right and dumps look
>         better.  */
> Index: gcc/omp-low.c
> ===================================================================
> --- gcc/omp-low.c       (.../trunk)     (revision 180744)
> +++ gcc/omp-low.c       (.../branches/transactional-memory)     (revision
> 180773)
> @@ -139,6 +139,7 @@ static tree scan_omp_1_op (tree *, int *
>     case GIMPLE_TRY: \
>     case GIMPLE_CATCH: \
>     case GIMPLE_EH_FILTER: \
> +    case GIMPLE_TRANSACTION: \
>       /* The sub-statements for these should be walked.  */ \
>       *handled_ops_p = false; \
>       break;
> Index: gcc/toplev.c
> ===================================================================
> --- gcc/toplev.c        (.../trunk)     (revision 180744)
> +++ gcc/toplev.c        (.../branches/transactional-memory)     (revision
> 180773)
> @@ -599,6 +599,7 @@ compile_file (void)
>
>       output_shared_constant_pool ();
>       output_object_blocks ();
> +  finish_tm_clone_pairs ();
>       /* Write out any pending weak symbol declarations.  */
>       weak_finish ();
> Index: gcc/cgraphunit.c
> ===================================================================
> --- gcc/cgraphunit.c    (.../trunk)     (revision 180744)
> +++ gcc/cgraphunit.c    (.../branches/transactional-memory)     (revision
> 180773)
> @@ -2272,7 +2272,7 @@ update_call_expr (struct cgraph_node *ne
>    was copied to prevent duplications of calls that are dead
>    in the clone.  */
>
> -static struct cgraph_node *
> +struct cgraph_node *
>  cgraph_copy_node_for_versioning (struct cgraph_node *old_version,
>                                 tree new_decl,
>                                 VEC(cgraph_edge_p,heap) *redirect_callers,
> @@ -2286,7 +2286,7 @@ cgraph_copy_node_for_versioning (struct
>
>    new_version = cgraph_create_node (new_decl);
>
> -   new_version->analyzed = true;
> +   new_version->analyzed = old_version->analyzed;

Hm?  analyzed means "with body", sure you have a body if you clone.

>    new_version->local = old_version->local;
>    new_version->local.externally_visible = false;
>    new_version->local.local = true;
> @@ -2294,6 +2294,7 @@ cgraph_copy_node_for_versioning (struct
>    new_version->rtl = old_version->rtl;
>    new_version->reachable = true;
>    new_version->count = old_version->count;
> +   new_version->lowered = true;

OTOH this isn't necessary true.  cgraph exists before lowering.

>    for (e = old_version->callees; e; e=e->next_callee)
>      if (!bbs_to_copy
> @@ -2389,7 +2390,6 @@ cgraph_function_versioning (struct cgrap
>   DECL_VIRTUAL_P (new_version_node->decl) = 0;
>   new_version_node->local.externally_visible = 0;
>   new_version_node->local.local = 1;
> -  new_version_node->lowered = true;
>
>   /* Update the call_expr on the edges to call the new version node. */
>   update_call_expr (new_version_node);
> Index: gcc/tree-ssa-alias.c
> ===================================================================
> --- gcc/tree-ssa-alias.c        (.../trunk)     (revision 180744)
> +++ gcc/tree-ssa-alias.c        (.../branches/transactional-memory)
> (revision 180773)
> @@ -1182,6 +1182,8 @@ ref_maybe_used_by_call_p_1 (gimple call,
>        case BUILT_IN_MEMPCPY:
>        case BUILT_IN_STPCPY:
>        case BUILT_IN_STPNCPY:
> +        case BUILT_IN_TM_MEMCPY:
> +        case BUILT_IN_TM_MEMMOVE:
>          {
>            ao_ref dref;
>            tree size = NULL_TREE;
> @@ -1228,6 +1230,32 @@ ref_maybe_used_by_call_p_1 (gimple call,
>                                           size);
>            return refs_may_alias_p_1 (&dref, ref, false);
>          }
> +
> +        /* The following functions read memory pointed to by their
> +          first argument.  */
> +       CASE_BUILT_IN_TM_LOAD (1):
> +       CASE_BUILT_IN_TM_LOAD (2):
> +       CASE_BUILT_IN_TM_LOAD (4):
> +       CASE_BUILT_IN_TM_LOAD (8):
> +        CASE_BUILT_IN_TM_LOAD (FLOAT):
> +       CASE_BUILT_IN_TM_LOAD (DOUBLE):
> +       CASE_BUILT_IN_TM_LOAD (LDOUBLE):
> +       CASE_BUILT_IN_TM_LOAD (M64):
> +       CASE_BUILT_IN_TM_LOAD (M128):
> +       CASE_BUILT_IN_TM_LOAD (M256):
> +        case BUILT_IN_TM_LOG:
> +        case BUILT_IN_TM_LOG_1:
> +        case BUILT_IN_TM_LOG_2:
> +        case BUILT_IN_TM_LOG_4:
> +        case BUILT_IN_TM_LOG_8:
> +        case BUILT_IN_TM_LOG_FLOAT:
> +        case BUILT_IN_TM_LOG_DOUBLE:
> +        case BUILT_IN_TM_LOG_LDOUBLE:
> +        case BUILT_IN_TM_LOG_M64:
> +        case BUILT_IN_TM_LOG_M128:
> +        case BUILT_IN_TM_LOG_M256:
> +         return ptr_deref_may_alias_ref_p_1 (gimple_call_arg (call, 0),
> ref);
> +
>        /* These read memory pointed to by the first argument.  */
>        case BUILT_IN_STRDUP:
>        case BUILT_IN_STRNDUP:
> @@ -1250,6 +1278,7 @@ ref_maybe_used_by_call_p_1 (gimple call,
>        case BUILT_IN_STACK_SAVE:
>        case BUILT_IN_STACK_RESTORE:
>        case BUILT_IN_MEMSET:
> +        case BUILT_IN_TM_MEMSET:
>        case BUILT_IN_MEMSET_CHK:
>        case BUILT_IN_FREXP:
>        case BUILT_IN_FREXPF:
> @@ -1480,6 +1509,19 @@ call_may_clobber_ref_p_1 (gimple call, a
>        case BUILT_IN_STRCAT:
>        case BUILT_IN_STRNCAT:
>        case BUILT_IN_MEMSET:
> +        case BUILT_IN_TM_MEMSET:
> +        CASE_BUILT_IN_TM_STORE (1):
> +        CASE_BUILT_IN_TM_STORE (2):
> +        CASE_BUILT_IN_TM_STORE (4):
> +        CASE_BUILT_IN_TM_STORE (8):
> +        CASE_BUILT_IN_TM_STORE (FLOAT):
> +        CASE_BUILT_IN_TM_STORE (DOUBLE):
> +        CASE_BUILT_IN_TM_STORE (LDOUBLE):
> +        CASE_BUILT_IN_TM_STORE (M64):
> +        CASE_BUILT_IN_TM_STORE (M128):
> +        CASE_BUILT_IN_TM_STORE (M256):
> +        case BUILT_IN_TM_MEMCPY:
> +        case BUILT_IN_TM_MEMMOVE:
>          {
>            ao_ref dref;
>            tree size = NULL_TREE;
> Index: gcc/ipa-inline.c
> ===================================================================
> --- gcc/ipa-inline.c    (.../trunk)     (revision 180744)
> +++ gcc/ipa-inline.c    (.../branches/transactional-memory)     (revision
> 180773)
> @@ -284,6 +284,15 @@ can_inline_edge_p (struct cgraph_edge *e
>       e->inline_failed = CIF_EH_PERSONALITY;
>       inlinable = false;
>     }
> +  /* TM pure functions should not get inlined if the outer function is
> +     a TM safe function.  */
> +  else if (flag_tm

Please move flag checks into the respective prediates.  Any reason
why the is_tm_pure () predicate wouldn't already do the correct thing
with !flag_tm?

> +          && is_tm_pure (callee->decl)
> +          && is_tm_safe (e->caller->decl))
> +    {
> +      e->inline_failed = CIF_UNSPECIFIED;
> +      inlinable = false;
> +    }
>   /* Don't inline if the callee can throw non-call exceptions but the
>      caller cannot.
>      FIXME: this is obviously wrong for LTO where STRUCT_FUNCTION is
> missing.
> Index: gcc/crtstuff.c
> ===================================================================
> --- gcc/crtstuff.c      (.../trunk)     (revision 180744)
> +++ gcc/crtstuff.c      (.../branches/transactional-memory)     (revision
> 180773)
> @@ -162,6 +162,9 @@ extern void __do_global_ctors_1 (void);
>  /* Likewise for _Jv_RegisterClasses.  */
>  extern void _Jv_RegisterClasses (void *) TARGET_ATTRIBUTE_WEAK;
>
> +extern void _ITM_registerTMCloneTable (void *, size_t)
> TARGET_ATTRIBUTE_WEAK;
> +extern void _ITM_deregisterTMCloneTable (void *) TARGET_ATTRIBUTE_WEAK;
> +
>  #ifdef OBJECT_FORMAT_ELF
>
>  /*  Declare a pointer to void function type.  */
> @@ -241,6 +244,11 @@ STATIC void *__JCR_LIST__[]
>   = { };
>  #endif /* JCR_SECTION_NAME */
>
> +STATIC func_ptr __TMC_LIST__[]
> +  __attribute__((unused, section(".tm_clone_table"),
> aligned(sizeof(void*))))
> +  = { };
> +extern func_ptr __TMC_END__[] __attribute__((__visibility__ ("hidden")));
> +
>  #if defined(INIT_SECTION_ASM_OP) || defined(INIT_ARRAY_SECTION_ASM_OP)
>
>  #ifdef OBJECT_FORMAT_ELF
> @@ -330,6 +338,13 @@ __do_global_dtors_aux (void)
>   }
>  #endif /* !defined(FINI_ARRAY_SECTION_ASM_OP) */
>
> +  if (_ITM_deregisterTMCloneTable)
> +    {
> +      size_t size = (size_t)(__TMC_END__ - __TMC_LIST__) / 2;
> +      if (size > 0)
> +       _ITM_deregisterTMCloneTable (__TMC_LIST__);
> +    }
> +
>  #ifdef USE_EH_FRAME_REGISTRY
>  #ifdef CRT_GET_RFIB_DATA
>   /* If we used the new __register_frame_info_bases interface,
> @@ -391,6 +406,12 @@ frame_dummy (void)
>        register_classes (__JCR_LIST__);
>     }
>  #endif /* JCR_SECTION_NAME */
> +  if (_ITM_registerTMCloneTable)
> +    {
> +      size_t size = (size_t)(__TMC_END__ - __TMC_LIST__) / 2;
> +      if (size > 0)
> +       _ITM_registerTMCloneTable (__TMC_LIST__, size);
> +    }
>  }
>
>  #ifdef INIT_SECTION_ASM_OP
> @@ -457,6 +478,13 @@ __do_global_dtors (void)
>   for (p = __DTOR_LIST__ + 1; (f = *p); p++)
>     f ();
>
> +  if (_ITM_deregisterTMCloneTable)
> +    {
> +      size_t size = (size_t)(__TMC_END__ - __TMC_LIST__) / 2;
> +      if (size > 0)
> +       _ITM_deregisterTMCloneTable (__TMC_LIST__);
> +    }
> +
>  #ifdef USE_EH_FRAME_REGISTRY
>   if (__deregister_frame_info)
>     __deregister_frame_info (__EH_FRAME_BEGIN__);
> @@ -570,6 +598,11 @@ STATIC void *__JCR_END__[1]
>    = { 0 };
>  #endif /* JCR_SECTION_NAME */
>
> +func_ptr __TMC_END__[]
> +  __attribute__((unused, section(".tm_clone_table"), aligned(sizeof(void
> *)),
> +                __visibility__ ("hidden")))
> +  = { };
> +
>  #ifdef INIT_ARRAY_SECTION_ASM_OP
>
>  /* If we are using .init_array, there is nothing to do.  */
> Index: gcc/cfgbuild.c
> ===================================================================
> --- gcc/cfgbuild.c      (.../trunk)     (revision 180744)
> +++ gcc/cfgbuild.c      (.../branches/transactional-memory)     (revision
> 180773)
> @@ -338,18 +338,30 @@ make_edges (basic_block min, basic_block
>          /* Add any appropriate EH edges.  */
>          rtl_make_eh_edge (edge_cache, bb, insn);
>
> -         if (code == CALL_INSN && nonlocal_goto_handler_labels)
> +         if (code == CALL_INSN)
>            {
> -             /* ??? This could be made smarter: in some cases it's possible
> -                to tell that certain calls will not do a nonlocal goto.
> -                For example, if the nested functions that do the nonlocal
> -                gotos do not have their addresses taken, then only calls to
> -                those functions or to other nested functions that use them
> -                could possibly do nonlocal gotos.  */
>              if (can_nonlocal_goto (insn))
> -               for (x = nonlocal_goto_handler_labels; x; x = XEXP (x, 1))
> -                 make_label_edge (edge_cache, bb, XEXP (x, 0),
> -                                  EDGE_ABNORMAL | EDGE_ABNORMAL_CALL);
> +               {
> +                 /* ??? This could be made smarter: in some cases it's
> +                    possible to tell that certain calls will not do a
> +                    nonlocal goto.  For example, if the nested functions
> +                    that do the nonlocal gotos do not have their addresses
> +                    taken, then only calls to those functions or to other
> +                    nested functions that use them could possibly do
> +                    nonlocal gotos.  */
> +                 for (x = nonlocal_goto_handler_labels; x; x = XEXP (x, 1))
> +                   make_label_edge (edge_cache, bb, XEXP (x, 0),
> +                                    EDGE_ABNORMAL | EDGE_ABNORMAL_CALL);
> +               }
> +
> +             if (flag_tm)
> +               {
> +                 rtx note;
> +                 for (note = REG_NOTES (insn); note; note = XEXP (note, 1))
> +                   if (REG_NOTE_KIND (note) == REG_TM)
> +                     make_label_edge (edge_cache, bb, XEXP (note, 0),
> +                                      EDGE_ABNORMAL | EDGE_ABNORMAL_CALL);
> +               }
>            }
>        }
>
> Index: gcc/timevar.def
> ===================================================================
> --- gcc/timevar.def     (.../trunk)     (revision 180744)
> +++ gcc/timevar.def     (.../branches/transactional-memory)     (revision
> 180773)
> @@ -184,6 +184,7 @@ DEFTIMEVAR (TV_TREE_COPY_RENAME          , "
>  DEFTIMEVAR (TV_TREE_SSA_VERIFY       , "tree SSA verifier")
>  DEFTIMEVAR (TV_TREE_STMT_VERIFY      , "tree STMT verifier")
>  DEFTIMEVAR (TV_TREE_SWITCH_CONVERSION, "tree switch initialization
> conversion")
> +DEFTIMEVAR (TV_TRANS_MEM             , "transactional memory")
>  DEFTIMEVAR (TV_TREE_STRLEN           , "tree strlen optimization")
>  DEFTIMEVAR (TV_CGRAPH_VERIFY         , "callgraph verifier")
>  DEFTIMEVAR (TV_DOM_FRONTIERS         , "dominance frontiers")
> Index: gcc/recog.c
> ===================================================================
> --- gcc/recog.c (.../trunk)     (revision 180744)
> +++ gcc/recog.c (.../branches/transactional-memory)     (revision 180773)
> @@ -3287,6 +3287,7 @@ peep2_attempt (basic_block bb, rtx insn,
>          {
>          case REG_NORETURN:
>          case REG_SETJMP:
> +         case REG_TM:
>            add_reg_note (new_insn, REG_NOTE_KIND (note),
>                          XEXP (note, 0));
>            break;
> Index: gcc/function.h
> ===================================================================
> --- gcc/function.h      (.../trunk)     (revision 180744)
> +++ gcc/function.h      (.../branches/transactional-memory)     (revision
> 180773)
> @@ -467,6 +467,14 @@ extern GTY(()) struct rtl_data x_rtl;
>    want to do differently.  */
>  #define crtl (&x_rtl)
>
> +/* This structure is used to map a gimple statement to a label,
> +   or list of labels to represent transaction restart.  */
> +
> +struct GTY(()) tm_restart_node {
> +  gimple stmt;
> +  tree label_or_list;
> +};
> +
>  struct GTY(()) stack_usage
>  {
>   /* # of bytes of static stack space allocated by the function.  */
> @@ -518,6 +526,10 @@ struct GTY(()) function {
>   /* Value histograms attached to particular statements.  */
>   htab_t GTY((skip)) value_histograms;
>
> +  /* Map gimple stmt to tree label (or list of labels) for transaction
> +     restart and abort.  */
> +  htab_t GTY ((param_is (struct tm_restart_node))) tm_restart;
> +

As this maps 'gimple' to tree shouldn't this go to fn->gimple_df instead?
That way you avoid growing generic struct function.  Or in to eh_status,
if that looks like a better fit.

>   /* For function.c.  */
>
>   /* Points to the FUNCTION_DECL of this function.  */
> Index: gcc/emit-rtl.c
> ===================================================================
> --- gcc/emit-rtl.c      (.../trunk)     (revision 180744)
> +++ gcc/emit-rtl.c      (.../branches/transactional-memory)     (revision
> 180773)
> @@ -3595,6 +3595,7 @@ try_split (rtx pat, rtx trial, int last)
>
>        case REG_NORETURN:
>        case REG_SETJMP:
> +       case REG_TM:
>          for (insn = insn_last; insn != NULL_RTX; insn = PREV_INSN (insn))
>            {
>              if (CALL_P (insn))
> Index: gcc/cfgexpand.c
> ===================================================================
> --- gcc/cfgexpand.c     (.../trunk)     (revision 180744)
> +++ gcc/cfgexpand.c     (.../branches/transactional-memory)     (revision
> 180773)
> @@ -2096,6 +2096,32 @@ expand_gimple_stmt (gimple stmt)
>        }
>     }
>
> +  /* Mark all calls that can have a transaction restart.  */

Why isn't this done when we expand the call?  This walking of the
RTL sequence looks like a hack (an easy one, albeit).

> +  if (cfun->tm_restart && is_gimple_call (stmt))
> +    {
> +      struct tm_restart_node dummy;
> +      void **slot;
> +
> +      dummy.stmt = stmt;
> +      slot = htab_find_slot (cfun->tm_restart, &dummy, NO_INSERT);
> +      if (slot)
> +       {
> +         struct tm_restart_node *n = (struct tm_restart_node *) *slot;
> +         tree list = n->label_or_list;
> +         rtx insn;
> +
> +         for (insn = next_real_insn (last); !CALL_P (insn);
> +              insn = next_real_insn (insn))
> +           continue;
> +
> +         if (TREE_CODE (list) == LABEL_DECL)
> +           add_reg_note (insn, REG_TM, label_rtx (list));
> +         else
> +           for (; list ; list = TREE_CHAIN (list))
> +             add_reg_note (insn, REG_TM, label_rtx (TREE_VALUE (list)));
> +       }
> +    }
> +
>   return last;
>  }
>
> @@ -4455,6 +4481,10 @@ gimple_expand_cfg (void)
>   /* After expanding, the return labels are no longer needed. */
>   return_label = NULL;
>   naked_return_label = NULL;
> +
> +  /* After expanding, the tm_restart map is no longer needed.  */
> +  cfun->tm_restart = NULL;

You should still free it, to not confuse the statistics code I think.

> +
>   /* Tag the blocks with a depth number so that change_scope can find
>      the common parent easily.  */
>   set_block_levels (DECL_INITIAL (cfun->decl), 0);
> Index: gcc/varasm.c
> ===================================================================
> --- gcc/varasm.c        (.../trunk)     (revision 180744)
> +++ gcc/varasm.c        (.../branches/transactional-memory)     (revision
> 180773)
> @@ -5859,6 +5859,103 @@ assemble_alias (tree decl, tree target)
>     }
>  }
>
> +/* Record and output a table of translations from original function
> +   to its transaction aware clone.  Note that tm_pure functions are
> +   considered to be their own clone.  */
> +
> +static GTY((if_marked ("tree_map_marked_p"), param_is (struct tree_map)))
> +     htab_t tm_clone_pairs;
> +
> +void
> +record_tm_clone_pair (tree o, tree n)
> +{
> +  struct tree_map **slot, *h;
> +
> +  if (tm_clone_pairs == NULL)
> +    tm_clone_pairs = htab_create_ggc (32, tree_map_hash, tree_map_eq, 0);
> +
> +  h = ggc_alloc_tree_map ();
> +  h->hash = htab_hash_pointer (o);
> +  h->base.from = o;
> +  h->to = n;
> +
> +  slot = (struct tree_map **)
> +    htab_find_slot_with_hash (tm_clone_pairs, h, h->hash, INSERT);
> +  *slot = h;
> +}
> +
> +tree
> +get_tm_clone_pair (tree o)
> +{
> +  if (tm_clone_pairs)
> +    {
> +      struct tree_map *h, in;
> +
> +      in.base.from = o;
> +      in.hash = htab_hash_pointer (o);
> +      h = (struct tree_map *) htab_find_with_hash (tm_clone_pairs,
> +                                                  &in, in.hash);
> +      if (h)
> +       return h->to;
> +    }
> +  return NULL_TREE;
> +}
> +
> +/* Helper function for finish_tm_clone_pairs.  Dump the clone table.  */
> +
> +int
> +finish_tm_clone_pairs_1 (void **slot, void *info ATTRIBUTE_UNUSED)
> +{
> +  struct tree_map *map = (struct tree_map *) *slot;
> +  bool *switched = (bool *) info;
> +  tree src = map->base.from;
> +  tree dst = map->to;
> +  struct cgraph_node *src_n = cgraph_get_node (src);
> +  struct cgraph_node *dst_n = cgraph_get_node (dst);
> +
> +  /* The function ipa_tm_create_version() marks the clone as needed if
> +     the original function was needed.  But we also mark the clone as
> +     needed if we ever called the clone indirectly through
> +     TM_GETTMCLONE.  If neither of these are true, we didn't generate
> +     a clone, and we didn't call it indirectly... no sense keeping it
> +     in the clone table.  */
> +  if (!dst_n || !dst_n->needed)
> +    return 1;
> +
> +  /* This covers the case where we have optimized the original
> +     function away, and only access the transactional clone.  */
> +  if (!src_n || !src_n->needed)
> +    return 1;
> +
> +  if (!*switched)
> +    {
> +      switch_to_section (get_named_section (NULL, ".tm_clone_table", 3));
> +      assemble_align (POINTER_SIZE);
> +      *switched = true;
> +    }
> +
> +  assemble_integer (XEXP (DECL_RTL (src), 0),
> +                   POINTER_SIZE / BITS_PER_UNIT, POINTER_SIZE, 1);
> +  assemble_integer (XEXP (DECL_RTL (dst), 0),
> +                   POINTER_SIZE / BITS_PER_UNIT, POINTER_SIZE, 1);
> +  return 1;
> +}
> +
> +void
> +finish_tm_clone_pairs (void)
> +{
> +  bool switched = false;
> +
> +  if (tm_clone_pairs == NULL)
> +    return;
> +
> +  htab_traverse_noresize (tm_clone_pairs, finish_tm_clone_pairs_1,
> +                         (void *) &switched);

This makes the generated table dependent on memory layout.  You
need to walk the pairs in some deterministic order.  In fact why not
walk all cgraph_nodes looking for the pairs - they should be still
in the list of clones for a node and you've marked it with DECL_TM_CLONE.
You can then sort them by cgraph node uid.

Did you check bootstrapping GCC with TM enabled and address-space
randomization turned on?

> +  htab_delete (tm_clone_pairs);
> +  tm_clone_pairs = NULL;
> +}
> +
> +
>  /* Emit an assembler directive to set symbol for DECL visibility to
>    the visibility type VIS, which must not be VISIBILITY_DEFAULT.  */
>
> Index: gcc/output.h
> ===================================================================
> --- gcc/output.h        (.../trunk)     (revision 180744)
> +++ gcc/output.h        (.../branches/transactional-memory)     (revision
> 180773)
> @@ -606,6 +606,11 @@ extern bool unlikely_text_section_p (sec
>  extern void switch_to_section (section *);
>  extern void output_section_asm_op (const void *);
>
> +extern void record_tm_clone_pair (tree, tree);
> +extern void finish_tm_clone_pairs (void);
> +extern int finish_tm_clone_pairs_1 (void **, void *);
> +extern tree get_tm_clone_pair (tree);
> +
>  extern void default_asm_output_source_filename (FILE *, const char *);
>  extern void output_file_directive (FILE *, const char *);
>
> Index: gcc/combine.c
> ===================================================================
> --- gcc/combine.c       (.../trunk)     (revision 180744)
> +++ gcc/combine.c       (.../branches/transactional-memory)     (revision
> 180773)
> @@ -13286,6 +13286,7 @@ distribute_notes (rtx notes, rtx from_in
>
>        case REG_NORETURN:
>        case REG_SETJMP:
> +       case REG_TM:
>          /* These notes must remain with the call.  It should not be
>             possible for both I2 and I3 to be a call.  */
>          if (CALL_P (i3))
> Index: gcc/tree-flow.h
> ===================================================================
> --- gcc/tree-flow.h     (.../trunk)     (revision 180744)
> +++ gcc/tree-flow.h     (.../branches/transactional-memory)     (revision
> 180773)
> @@ -778,6 +778,9 @@ extern bool maybe_duplicate_eh_stmt (gim
>  extern bool verify_eh_edges (gimple);
>  extern bool verify_eh_dispatch_edge (gimple);
>
> +/* In gtm-low.c  */
> +extern bool is_transactional_stmt (const_gimple);
> +

gimple.h please.  looks like a gimple predicate as well, so the implementation
should be in gimple.c?

>  /* In tree-ssa-pre.c  */
>  struct pre_expr_d;
>  void add_to_value (unsigned int, struct pre_expr_d *);
> Index: gcc/tree-ssa-structalias.c
> ===================================================================
> --- gcc/tree-ssa-structalias.c  (.../trunk)     (revision 180744)
> +++ gcc/tree-ssa-structalias.c  (.../branches/transactional-memory)
> (revision 180773)
> @@ -4024,6 +4024,8 @@ find_func_aliases_for_builtin_call (gimp
>       case BUILT_IN_STPCPY_CHK:
>       case BUILT_IN_STRCAT_CHK:
>       case BUILT_IN_STRNCAT_CHK:
> +      case BUILT_IN_TM_MEMCPY:
> +      case BUILT_IN_TM_MEMMOVE:
>        {
>          tree res = gimple_call_lhs (t);
>          tree dest = gimple_call_arg (t, (DECL_FUNCTION_CODE (fndecl)
> @@ -4056,6 +4058,7 @@ find_func_aliases_for_builtin_call (gimp
>        }
>       case BUILT_IN_MEMSET:
>       case BUILT_IN_MEMSET_CHK:
> +      case BUILT_IN_TM_MEMSET:
>        {
>          tree res = gimple_call_lhs (t);
>          tree dest = gimple_call_arg (t, 0);
> @@ -4197,6 +4200,50 @@ find_func_aliases_for_builtin_call (gimp
>            }
>          return true;
>        }
> +      CASE_BUILT_IN_TM_STORE (1):
> +      CASE_BUILT_IN_TM_STORE (2):
> +      CASE_BUILT_IN_TM_STORE (4):
> +      CASE_BUILT_IN_TM_STORE (8):
> +      CASE_BUILT_IN_TM_STORE (FLOAT):
> +      CASE_BUILT_IN_TM_STORE (DOUBLE):
> +      CASE_BUILT_IN_TM_STORE (LDOUBLE):
> +      CASE_BUILT_IN_TM_STORE (M64):
> +      CASE_BUILT_IN_TM_STORE (M128):
> +      CASE_BUILT_IN_TM_STORE (M256):
> +       {
> +         tree addr = gimple_call_arg (t, 0);
> +         tree src = gimple_call_arg (t, 1);
> +
> +         get_constraint_for (addr, &lhsc);
> +         do_deref (&lhsc);
> +         get_constraint_for (src, &rhsc);
> +         process_all_all_constraints (lhsc, rhsc);
> +         VEC_free (ce_s, heap, lhsc);
> +         VEC_free (ce_s, heap, rhsc);
> +         return true;
> +       }
> +      CASE_BUILT_IN_TM_LOAD (1):
> +      CASE_BUILT_IN_TM_LOAD (2):
> +      CASE_BUILT_IN_TM_LOAD (4):
> +      CASE_BUILT_IN_TM_LOAD (8):
> +      CASE_BUILT_IN_TM_LOAD (FLOAT):
> +      CASE_BUILT_IN_TM_LOAD (DOUBLE):
> +      CASE_BUILT_IN_TM_LOAD (LDOUBLE):
> +      CASE_BUILT_IN_TM_LOAD (M64):
> +      CASE_BUILT_IN_TM_LOAD (M128):
> +      CASE_BUILT_IN_TM_LOAD (M256):
> +        {
> +         tree dest = gimple_call_lhs (t);
> +         tree addr = gimple_call_arg (t, 0);
> +
> +         get_constraint_for (dest, &lhsc);
> +         get_constraint_for (addr, &rhsc);
> +         do_deref (&rhsc);
> +         process_all_all_constraints (lhsc, rhsc);
> +         VEC_free (ce_s, heap, lhsc);
> +         VEC_free (ce_s, heap, rhsc);
> +         return true;
> +        }
>       /* Variadic argument handling needs to be handled in IPA
>         mode as well.  */
>       case BUILT_IN_VA_START:
> Index: gcc/tree-cfg.c
> ===================================================================
> --- gcc/tree-cfg.c      (.../trunk)     (revision 180744)
> +++ gcc/tree-cfg.c      (.../branches/transactional-memory)     (revision
> 180773)
> @@ -666,6 +666,15 @@ make_edges (void)
>                }
>              break;
>
> +           case GIMPLE_TRANSACTION:
> +             {
> +               tree abort_label = gimple_transaction_label (last);
> +               if (abort_label)
> +                 make_edge (bb, label_to_block (abort_label), 0);
> +               fallthru = true;
> +             }
> +             break;
> +
>            default:
>              gcc_assert (!stmt_ends_bb_p (last));
>              fallthru = true;
> @@ -1196,22 +1205,30 @@ cleanup_dead_labels (void)
>   FOR_EACH_BB (bb)
>     {
>       gimple stmt = last_stmt (bb);
> +      tree label, new_label;
> +
>       if (!stmt)
>        continue;
>
>       switch (gimple_code (stmt))
>        {
>        case GIMPLE_COND:
> -         {
> -           tree true_label = gimple_cond_true_label (stmt);
> -           tree false_label = gimple_cond_false_label (stmt);
> +         label = gimple_cond_true_label (stmt);
> +         if (label)
> +           {
> +             new_label = main_block_label (label);
> +             if (new_label != label)
> +               gimple_cond_set_true_label (stmt, new_label);
> +           }
>
> -           if (true_label)
> -             gimple_cond_set_true_label (stmt, main_block_label
> (true_label));
> -           if (false_label)
> -             gimple_cond_set_false_label (stmt, main_block_label
> (false_label));
> -           break;
> -         }
> +         label = gimple_cond_false_label (stmt);
> +         if (label)
> +           {
> +             new_label = main_block_label (label);
> +             if (new_label != label)
> +               gimple_cond_set_false_label (stmt, new_label);
> +           }
> +         break;
>
>        case GIMPLE_SWITCH:
>          {
> @@ -1221,8 +1238,10 @@ cleanup_dead_labels (void)
>            for (i = 0; i < n; ++i)
>              {
>                tree case_label = gimple_switch_label (stmt, i);
> -               tree label = main_block_label (CASE_LABEL (case_label));
> -               CASE_LABEL (case_label) = label;
> +               label = CASE_LABEL (case_label);
> +               new_label = main_block_label (label);
> +               if (new_label != label)
> +                 CASE_LABEL (case_label) = new_label;
>              }
>            break;
>          }
> @@ -1243,13 +1262,27 @@ cleanup_dead_labels (void)
>        /* We have to handle gotos until they're removed, and we don't
>           remove them until after we've created the CFG edges.  */
>        case GIMPLE_GOTO:
> -          if (!computed_goto_p (stmt))
> +         if (!computed_goto_p (stmt))
>            {
> -             tree new_dest = main_block_label (gimple_goto_dest (stmt));
> -             gimple_goto_set_dest (stmt, new_dest);
> +             label = gimple_goto_dest (stmt);
> +             new_label = main_block_label (label);
> +             if (new_label != label)
> +               gimple_goto_set_dest (stmt, new_label);

What's the reason for this changes?  Optimization?

>            }
>          break;
>
> +       case GIMPLE_TRANSACTION:
> +         {
> +           tree label = gimple_transaction_label (stmt);
> +           if (label)
> +             {
> +               tree new_label = main_block_label (label);
> +               if (new_label != label)
> +                 gimple_transaction_set_label (stmt, new_label);
> +             }
> +         }
> +         break;
> +
>        default:
>          break;
>       }
> @@ -2263,6 +2296,13 @@ is_ctrl_altering_stmt (gimple t)
>        if (flags & ECF_NORETURN)
>          return true;
>
> +       /* TM ending statements have backedges out of the transaction.
> +          Return true so we split the basic block containing
> +          them.  */
> +       if ((flags & ECF_TM_OPS)
> +           && is_tm_ending_fndecl (gimple_call_fndecl (t)))
> +         return true;
> +
>        /* BUILT_IN_RETURN call is same as return statement.  */
>        if (gimple_call_builtin_p (t, BUILT_IN_RETURN))
>          return true;
> @@ -2284,6 +2324,10 @@ is_ctrl_altering_stmt (gimple t)
>       /* OpenMP directives alter control flow.  */
>       return true;
>
> +    case GIMPLE_TRANSACTION:
> +      /* A transaction start alters control flow.  */
> +      return true;
> +
>     default:
>       break;
>     }
> @@ -4054,6 +4098,17 @@ verify_gimple_switch (gimple stmt)
>   return false;
>  }
>
> +/* Verify the contents of a GIMPLE_TRANSACTION.  Returns true if there
> +   is a problem, otherwise false.  */
> +
> +static bool
> +verify_gimple_transaction (gimple stmt)
> +{
> +  tree lab = gimple_transaction_label (stmt);
> +  if (lab != NULL && TREE_CODE (lab) != LABEL_DECL)
> +    return true;

ISTR this has substatements, so you should handle this in
verify_gimple_in_seq_2 and make sure to verify those substatements.

> +  return false;
> +}
>
>  /* Verify a gimple debug statement STMT.
>    Returns true if anything is wrong.  */
> @@ -4155,6 +4210,9 @@ verify_gimple_stmt (gimple stmt)
>     case GIMPLE_ASM:
>       return false;
>
> +    case GIMPLE_TRANSACTION:
> +      return verify_gimple_transaction (stmt);
> +

Not here.

>     /* Tuples that do not have tree operands.  */
>     case GIMPLE_NOP:
>     case GIMPLE_PREDICT:
> @@ -4271,10 +4329,19 @@ verify_gimple_in_seq_2 (gimple_seq stmts
>          err |= verify_gimple_in_seq_2 (gimple_eh_filter_failure (stmt));
>          break;
>
> +       case GIMPLE_EH_ELSE:
> +         err |= verify_gimple_in_seq_2 (gimple_eh_else_n_body (stmt));
> +         err |= verify_gimple_in_seq_2 (gimple_eh_else_e_body (stmt));
> +         break;
> +
>        case GIMPLE_CATCH:
>          err |= verify_gimple_in_seq_2 (gimple_catch_handler (stmt));
>          break;
>
> +       case GIMPLE_TRANSACTION:
> +         err |= verify_gimple_in_seq_2 (gimple_transaction_body (stmt));
> +         break;
> +

Ah, you do.  But you'll never call your label verification code.

>        default:
>          {
>            bool err2 = verify_gimple_stmt (stmt);
> @@ -5052,6 +5119,14 @@ gimple_redirect_edge_and_branch (edge e,
>        redirect_eh_dispatch_edge (stmt, e, dest);
>       break;
>
> +    case GIMPLE_TRANSACTION:
> +      /* The ABORT edge has a stored label associated with it, otherwise
> +        the edges are simply redirectable.  */
> +      /* ??? We don't really need this label after the cfg is created.  */
> +      if (e->flags == 0)
> +       gimple_transaction_set_label (stmt, gimple_block_label (dest));

So why set it (and thus keep it live)?

> +      break;
> +
>     default:
>       /* Otherwise it must be a fallthru edge, and we don't need to
>         do anything besides redirecting it.  */
> @@ -6428,8 +6503,10 @@ dump_function_to_file (tree fn, FILE *fi
>   bool ignore_topmost_bind = false, any_var = false;
>   basic_block bb;
>   tree chain;
> +  bool tmclone = TREE_CODE (fn) == FUNCTION_DECL && DECL_IS_TM_CLONE (fn);
>
> -  fprintf (file, "%s (", lang_hooks.decl_printable_name (fn, 2));
> +  fprintf (file, "%s %s(", lang_hooks.decl_printable_name (fn, 2),
> +          tmclone ? "[tm-clone] " : "");
>
>   arg = DECL_ARGUMENTS (fn);
>   while (arg)
> Index: gcc/passes.c
> ===================================================================
> --- gcc/passes.c        (.../trunk)     (revision 180744)
> +++ gcc/passes.c        (.../branches/transactional-memory)     (revision
> 180773)
> @@ -1174,9 +1174,11 @@ init_optimization_passes (void)
>   p = &all_lowering_passes;
>   NEXT_PASS (pass_warn_unused_result);
>   NEXT_PASS (pass_diagnose_omp_blocks);
> +  NEXT_PASS (pass_diagnose_tm_blocks);
>   NEXT_PASS (pass_mudflap_1);
>   NEXT_PASS (pass_lower_omp);
>   NEXT_PASS (pass_lower_cf);
> +  NEXT_PASS (pass_lower_tm);
>   NEXT_PASS (pass_refactor_eh);
>   NEXT_PASS (pass_lower_eh);
>   NEXT_PASS (pass_build_cfg);
> @@ -1241,6 +1243,7 @@ init_optimization_passes (void)
>     }
>   NEXT_PASS (pass_ipa_increase_alignment);
>   NEXT_PASS (pass_ipa_matrix_reorg);
> +  NEXT_PASS (pass_ipa_tm);
>   NEXT_PASS (pass_ipa_lower_emutls);
>   *p = NULL;
>
> @@ -1400,6 +1403,13 @@ init_optimization_passes (void)
>       NEXT_PASS (pass_uncprop);
>       NEXT_PASS (pass_local_pure_const);
>     }
> +  NEXT_PASS (pass_tm_init);
> +    {
> +      struct opt_pass **p = &pass_tm_init.pass.sub;
> +      NEXT_PASS (pass_tm_mark);
> +      NEXT_PASS (pass_tm_memopt);
> +      NEXT_PASS (pass_tm_edges);
> +    }
>   NEXT_PASS (pass_lower_complex_O0);
>   NEXT_PASS (pass_cleanup_eh);
>   NEXT_PASS (pass_lower_resx);
> Index: gcc/reg-notes.def
> ===================================================================
> --- gcc/reg-notes.def   (.../trunk)     (revision 180744)
> +++ gcc/reg-notes.def   (.../branches/transactional-memory)     (revision
> 180773)
> @@ -203,6 +203,11 @@ REG_NOTE (CROSSING_JUMP)
>    functions that can return twice.  */
>  REG_NOTE (SETJMP)
>
> +/* This kind of note is generated at each transactional memory
> +   builtin, to indicate we need to generate transaction restart
> +   edges for this insn.  */
> +REG_NOTE (TM)
> +
>  /* Indicates the cumulative offset of the stack pointer accounting
>    for pushed arguments.  This will only be generated when
>    ACCUMULATE_OUTGOING_ARGS is false.  */
> Index: gcc/cfgrtl.c
> ===================================================================
> --- gcc/cfgrtl.c        (.../trunk)     (revision 180744)
> +++ gcc/cfgrtl.c        (.../branches/transactional-memory)     (revision
> 180773)
> @@ -2246,6 +2246,8 @@ purge_dead_edges (basic_block bb)
>            ;
>          else if ((e->flags & EDGE_EH) && can_throw_internal (insn))
>            ;
> +         else if (flag_tm && find_reg_note (insn, REG_TM, NULL))
> +           ;
>          else
>            remove = true;
>        }
> Index: gcc/params.def
> ===================================================================
> --- gcc/params.def      (.../trunk)     (revision 180744)
> +++ gcc/params.def      (.../branches/transactional-memory)     (revision
> 180773)
> @@ -872,6 +872,13 @@ DEFPARAM (PARAM_IPA_SRA_PTR_GROWTH_FACTO
>          "a pointer to an aggregate with",
>          2, 0, 0)
>
> +DEFPARAM (PARAM_TM_MAX_AGGREGATE_SIZE,
> +         "tm-max-aggregate-size",
> +         "Size in bytes after which thread-local aggregates should be "
> +         "instrumented with the logging functions instead of save/restore "
> +         "pairs",
> +         9, 0, 0)
> +
>  DEFPARAM (PARAM_IPA_CP_VALUE_LIST_SIZE,
>          "ipa-cp-value-list-size",
>          "Maximum size of a list of values associated with each parameter
> for "
>

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [patch] 19/n: trans-mem: middle end/misc patches (LAST PATCH)
  2011-11-04 11:22 ` Richard Guenther
@ 2011-11-06 19:07   ` Aldy Hernandez
  2011-11-06 20:47     ` Richard Henderson
                       ` (2 more replies)
  0 siblings, 3 replies; 20+ messages in thread
From: Aldy Hernandez @ 2011-11-06 19:07 UTC (permalink / raw)
  To: Richard Guenther; +Cc: gcc-patches, Richard Henderson

[-- Attachment #1: Type: text/plain, Size: 6698 bytes --]

[rth, more comments for you below]

On 11/04/11 04:14, Richard Guenther wrote:

>>     new_version = cgraph_create_node (new_decl);
>>
>> -   new_version->analyzed = true;
>> +   new_version->analyzed = old_version->analyzed;
>
> Hm?  analyzed means "with body", sure you have a body if you clone.
>
>>     new_version->local = old_version->local;
>>     new_version->local.externally_visible = false;
>>     new_version->local.local = true;
>> @@ -2294,6 +2294,7 @@ cgraph_copy_node_for_versioning (struct
>>     new_version->rtl = old_version->rtl;
>>     new_version->reachable = true;
>>     new_version->count = old_version->count;
>> +   new_version->lowered = true;
>
> OTOH this isn't necessary true.  cgraph exists before lowering.

I don't understand what you want me to do on either of these two 
comments.  Could you elaborate?

>> +  /* TM pure functions should not get inlined if the outer function is
>> +     a TM safe function.  */
>> +  else if (flag_tm
>
> Please move flag checks into the respective prediates.  Any reason
> why the is_tm_pure () predicate wouldn't already do the correct thing
> with !flag_tm?

Done.

>> +  /* Map gimple stmt to tree label (or list of labels) for transaction
>> +     restart and abort.  */
>> +  htab_t GTY ((param_is (struct tm_restart_node))) tm_restart;
>> +
>
> As this maps 'gimple' to tree shouldn't this go to fn->gimple_df instead?
> That way you avoid growing generic struct function.  Or in to eh_status,
> if that looks like a better fit.

Done.

>> +  /* Mark all calls that can have a transaction restart.  */
>
> Why isn't this done when we expand the call?  This walking of the
> RTL sequence looks like a hack (an easy one, albeit).
>
>> +  if (cfun->tm_restart&&  is_gimple_call (stmt))
>> +    {
>> +      struct tm_restart_node dummy;
>> +      void **slot;
>> +
>> +      dummy.stmt = stmt;
>> +      slot = htab_find_slot (cfun->tm_restart,&dummy, NO_INSERT);
>> +      if (slot)
>> +       {
>> +         struct tm_restart_node *n = (struct tm_restart_node *) *slot;
>> +         tree list = n->label_or_list;
>> +         rtx insn;
>> +
>> +         for (insn = next_real_insn (last); !CALL_P (insn);
>> +              insn = next_real_insn (insn))
>> +           continue;
>> +
>> +         if (TREE_CODE (list) == LABEL_DECL)
>> +           add_reg_note (insn, REG_TM, label_rtx (list));
>> +         else
>> +           for (; list ; list = TREE_CHAIN (list))
>> +             add_reg_note (insn, REG_TM, label_rtx (TREE_VALUE (list)));
>> +       }
>> +    }

I can certainly move this to expand_call_stmt() if you prefer.  Do you 
have an objection to the RTL walk?  This isn't my code, but I'm open to 
suggestions on an alternative to implement.

>> +  /* After expanding, the tm_restart map is no longer needed.  */
>> +  cfun->tm_restart = NULL;
>
> You should still free it, to not confuse the statistics code I think.

Done.

>> +finish_tm_clone_pairs (void)
>> +{
>> +  bool switched = false;
>> +
>> +  if (tm_clone_pairs == NULL)
>> +    return;
>> +
>> +  htab_traverse_noresize (tm_clone_pairs, finish_tm_clone_pairs_1,
>> +                         (void *)&switched);
>
> This makes the generated table dependent on memory layout.  You
> need to walk the pairs in some deterministic order.  In fact why not
> walk all cgraph_nodes looking for the pairs - they should be still
> in the list of clones for a node and you've marked it with DECL_TM_CLONE.
> You can then sort them by cgraph node uid.
>
> Did you check bootstrapping GCC with TM enabled and address-space
> randomization turned on?

Actually, the table organization is irrelevant, because upon registering 
of the table in the runtime, we qsort the entire thing.  We then do a 
binary search to find items.  See _ITM_registerTMCloneTable() and 
find_clone() in the libitm runtime.

>> +/* In gtm-low.c  */
>> +extern bool is_transactional_stmt (const_gimple);
>> +
>
> gimple.h please.  looks like a gimple predicate as well, so the implementation
> should be in gimple.c?

Woo hoo!  Unused function.  I've removed it altogether.

>>         case GIMPLE_GOTO:
>> -          if (!computed_goto_p (stmt))
>> +         if (!computed_goto_p (stmt))
>>             {
>> -             tree new_dest = main_block_label (gimple_goto_dest (stmt));
>> -             gimple_goto_set_dest (stmt, new_dest);
>> +             label = gimple_goto_dest (stmt);
>> +             new_label = main_block_label (label);
>> +             if (new_label != label)
>> +               gimple_goto_set_dest (stmt, new_label);
>
> What's the reason for this changes?  Optimization?

Yes.  Rth can elaborate if you deem necessary.

>> +/* Verify the contents of a GIMPLE_TRANSACTION.  Returns true if there
>> +   is a problem, otherwise false.  */
>> +
>> +static bool
>> +verify_gimple_transaction (gimple stmt)
>> +{
>> +  tree lab = gimple_transaction_label (stmt);
>> +  if (lab != NULL&&  TREE_CODE (lab) != LABEL_DECL)
>> +    return true;
>
> ISTR this has substatements, so you should handle this in
> verify_gimple_in_seq_2 and make sure to verify those substatements.

I have added verification for the substatements in 
verify_gimple_transaction()...

>> @@ -4155,6 +4210,9 @@ verify_gimple_stmt (gimple stmt)
>>      case GIMPLE_ASM:
>>        return false;
>>
>> +    case GIMPLE_TRANSACTION:
>> +      return verify_gimple_transaction (stmt);
>> +
>
> Not here.

...but we still need to check the transaction in verify_gimple_stmt() 
(as well as in verify_gimple_in_seq_2), because verify_gimple_in_cfg() 
will verify a gimple_transaction by calling verify_gimple_stmt, so we 
must handle GIMPLE_TRANSACTION in verify_gimple_stmt also.

>> +       case GIMPLE_TRANSACTION:
>> +         err |= verify_gimple_in_seq_2 (gimple_transaction_body (stmt));
>> +         break;

I am calling verify_gimple_transaction() now.  See patch.

>> +    case GIMPLE_TRANSACTION:
>> +      /* The ABORT edge has a stored label associated with it, otherwise
>> +        the edges are simply redirectable.  */
>> +      /* ??? We don't really need this label after the cfg is created.  */
>> +      if (e->flags == 0)
>> +       gimple_transaction_set_label (stmt, gimple_block_label (dest));
>
> So why set it (and thus keep it live)?

This seems like leftovers from a previous incantation.  However, I'm not 
100% sure, so I have disabled the code, but left it in a comment.  A 
full bootstrap/regtest revealed no regressions.

rth, do you have any objections to remove this?

Tested on x86-64 Linux.

OK for branch?

p.s. This changelog entry is for ChangeLog.tm, but I will adapt a merged 
ChangeLog entry for ChangeLog.tm-merge as well.  And will do so from now on.

[-- Attachment #2: curr --]
[-- Type: text/plain, Size: 9333 bytes --]

	* tree-cfg.c (verify_gimple_transaction): Verify body.  Move down.
	(verify_gimple_in_seq_2): Verify the label in a
	GIMPLE_TRANSACTION.
	* function.h (struct function): Move tm_restart field to struct
	gimple_df in tree-flow.h
	Move tm_restart_node to tree-flow.h
	* tree-flow.h (struct gimple_df): New location for tm_restart
	field.
	New location for tm_restart_node.
	(is_transactional_stmt): Remove.
	* trans-mem.c (is_transactional_stmt): Remove.
	(make_tm_edge): Field tm_restart is now in gimple_df.
	* cfgexpand.c (gimple_expand_cfg): Field tm_restart is now in
	cfun->gimple_df.
	Free tm_restart.
	* cfgexpand.c (expand_gimple_stmt): Field tm_restart is now in
	gimple_df.
	* ipa-inline.c (can_inline_edge_p): Do not check flag_tm.
	* trans-mem.c (is_tm_pure): Check flag_tm.
	(is_tm_safe): Same.

Index: ipa-inline.c
===================================================================
--- ipa-inline.c	(revision 181028)
+++ ipa-inline.c	(working copy)
@@ -286,8 +286,7 @@ can_inline_edge_p (struct cgraph_edge *e
     }
   /* TM pure functions should not get inlined if the outer function is
      a TM safe function.  */
-  else if (flag_tm
-	   && is_tm_pure (callee->decl)
+  else if (is_tm_pure (callee->decl)
 	   && is_tm_safe (e->caller->decl))
     {
       e->inline_failed = CIF_UNSPECIFIED;
Index: function.h
===================================================================
--- function.h	(revision 181028)
+++ function.h	(working copy)
@@ -467,14 +467,6 @@ extern GTY(()) struct rtl_data x_rtl;
    want to do differently.  */
 #define crtl (&x_rtl)
 
-/* This structure is used to map a gimple statement to a label,
-   or list of labels to represent transaction restart.  */
-
-struct GTY(()) tm_restart_node {
-  gimple stmt;
-  tree label_or_list;
-};
-
 struct GTY(()) stack_usage
 {
   /* # of bytes of static stack space allocated by the function.  */
@@ -526,10 +518,6 @@ struct GTY(()) function {
   /* Value histograms attached to particular statements.  */
   htab_t GTY((skip)) value_histograms;
 
-  /* Map gimple stmt to tree label (or list of labels) for transaction
-     restart and abort.  */
-  htab_t GTY ((param_is (struct tm_restart_node))) tm_restart;
-
   /* For function.c.  */
 
   /* Points to the FUNCTION_DECL of this function.  */
Index: trans-mem.c
===================================================================
--- trans-mem.c	(revision 181028)
+++ trans-mem.c	(working copy)
@@ -172,9 +172,13 @@ get_attrs_for (const_tree x)
 bool
 is_tm_pure (const_tree x)
 {
-  tree attrs = get_attrs_for (x);
-  if (attrs)
-    return lookup_attribute ("transaction_pure", attrs) != NULL;
+  if (flag_tm)
+    {
+      tree attrs = get_attrs_for (x);
+      if (attrs)
+	return lookup_attribute ("transaction_pure", attrs) != NULL;
+      return false;
+    }
   return false;
 }
 
@@ -205,16 +209,17 @@ is_tm_irrevocable (tree x)
 bool
 is_tm_safe (const_tree x)
 {
-  tree attrs = get_attrs_for (x);
-
-  if (attrs)
+  if (flag_tm)
     {
-      if (lookup_attribute ("transaction_safe", attrs))
-	return true;
-      if (lookup_attribute ("transaction_may_cancel_outer", attrs))
-	return true;
+      tree attrs = get_attrs_for (x);
+      if (attrs)
+	{
+	  if (lookup_attribute ("transaction_safe", attrs))
+	    return true;
+	  if (lookup_attribute ("transaction_may_cancel_outer", attrs))
+	    return true;
+	}
     }
-
   return false;
 }
 
@@ -270,22 +275,6 @@ is_tm_may_cancel_outer (tree x)
   return false;
 }
 
-/* Return true if STMT may alter control flow via a transactional edge.  */
-
-bool
-is_transactional_stmt (const_gimple stmt)
-{
-  switch (gimple_code (stmt))
-    {
-    case GIMPLE_CALL:
-      return (gimple_call_flags (stmt) & ECF_TM_OPS) != 0;
-    case GIMPLE_TRANSACTION:
-      return true;
-    default:
-      return false;
-    }
-}
-
 /* Return true for built in functions that "end" a transaction.   */
 
 bool
@@ -2470,13 +2459,13 @@ make_tm_edge (gimple stmt, basic_block b
   void **slot;
   struct tm_restart_node *n, dummy;
 
-  if (cfun->tm_restart == NULL)
-    cfun->tm_restart = htab_create_ggc (31, struct_ptr_hash,
-					struct_ptr_eq, ggc_free);
+  if (cfun->gimple_df->tm_restart == NULL)
+    cfun->gimple_df->tm_restart = htab_create_ggc (31, struct_ptr_hash,
+						   struct_ptr_eq, ggc_free);
 
   dummy.stmt = stmt;
   dummy.label_or_list = gimple_block_label (region->entry_block); 
-  slot = htab_find_slot (cfun->tm_restart, &dummy, INSERT);
+  slot = htab_find_slot (cfun->gimple_df->tm_restart, &dummy, INSERT);
   n = (struct tm_restart_node *) *slot;
   if (n == NULL)
     {
Index: cfgexpand.c
===================================================================
--- cfgexpand.c	(revision 181028)
+++ cfgexpand.c	(working copy)
@@ -2097,13 +2097,13 @@ expand_gimple_stmt (gimple stmt)
     }
 
   /* Mark all calls that can have a transaction restart.  */
-  if (cfun->tm_restart && is_gimple_call (stmt))
+  if (cfun->gimple_df->tm_restart && is_gimple_call (stmt))
     {
       struct tm_restart_node dummy;
       void **slot;
 
       dummy.stmt = stmt;
-      slot = htab_find_slot (cfun->tm_restart, &dummy, NO_INSERT);
+      slot = htab_find_slot (cfun->gimple_df->tm_restart, &dummy, NO_INSERT);
       if (slot)
 	{
 	  struct tm_restart_node *n = (struct tm_restart_node *) *slot;
@@ -4483,7 +4483,11 @@ gimple_expand_cfg (void)
   naked_return_label = NULL;
 
   /* After expanding, the tm_restart map is no longer needed.  */
-  cfun->tm_restart = NULL;
+  if (cfun->gimple_df->tm_restart)
+    {
+      htab_delete (cfun->gimple_df->tm_restart);
+      cfun->gimple_df->tm_restart = NULL;
+    }
 
   /* Tag the blocks with a depth number so that change_scope can find
      the common parent easily.  */
Index: tree-flow.h
===================================================================
--- tree-flow.h	(revision 181028)
+++ tree-flow.h	(working copy)
@@ -33,6 +33,14 @@ along with GCC; see the file COPYING3.  
 #include "tree-ssa-alias.h"
 
 
+/* This structure is used to map a gimple statement to a label,
+   or list of labels to represent transaction restart.  */
+
+struct GTY(()) tm_restart_node {
+  gimple stmt;
+  tree label_or_list;
+};
+
 /* Gimple dataflow datastructure. All publicly available fields shall have
    gimple_ accessor defined in tree-flow-inline.h, all publicly modifiable
    fields should have gimple_set accessor.  */
@@ -80,6 +88,10 @@ struct GTY(()) gimple_df {
   unsigned int ipa_pta : 1;
 
   struct ssa_operands ssa_operands;
+
+  /* Map gimple stmt to tree label (or list of labels) for transaction
+     restart and abort.  */
+  htab_t GTY ((param_is (struct tm_restart_node))) tm_restart;
 };
 
 /* Accessors for internal use only.  Generic code should use abstraction
@@ -778,9 +790,6 @@ extern bool maybe_duplicate_eh_stmt (gim
 extern bool verify_eh_edges (gimple);
 extern bool verify_eh_dispatch_edge (gimple);
 
-/* In gtm-low.c  */
-extern bool is_transactional_stmt (const_gimple);
-
 /* In tree-ssa-pre.c  */
 struct pre_expr_d;
 void add_to_value (unsigned int, struct pre_expr_d *);
Index: tree-cfg.c
===================================================================
--- tree-cfg.c	(revision 181028)
+++ tree-cfg.c	(working copy)
@@ -117,6 +117,7 @@ static int gimple_verify_flow_info (void
 static void gimple_make_forwarder_block (edge);
 static void gimple_cfg2vcg (FILE *);
 static gimple first_non_label_stmt (basic_block);
+static bool verify_gimple_transaction (gimple);
 
 /* Flowgraph optimization and cleanup.  */
 static void gimple_merge_blocks (basic_block, basic_block);
@@ -4098,18 +4099,6 @@ verify_gimple_switch (gimple stmt)
   return false;
 }
 
-/* Verify the contents of a GIMPLE_TRANSACTION.  Returns true if there
-   is a problem, otherwise false.  */
-
-static bool
-verify_gimple_transaction (gimple stmt)
-{
-  tree lab = gimple_transaction_label (stmt);
-  if (lab != NULL && TREE_CODE (lab) != LABEL_DECL)
-    return true;
-  return false;
-}
-
 /* Verify a gimple debug statement STMT.
    Returns true if anything is wrong.  */
 
@@ -4339,7 +4328,7 @@ verify_gimple_in_seq_2 (gimple_seq stmts
 	  break;
 
 	case GIMPLE_TRANSACTION:
-	  err |= verify_gimple_in_seq_2 (gimple_transaction_body (stmt));
+	  err |= verify_gimple_transaction (stmt);
 	  break;
 
 	default:
@@ -4355,6 +4344,18 @@ verify_gimple_in_seq_2 (gimple_seq stmts
   return err;
 }
 
+/* Verify the contents of a GIMPLE_TRANSACTION.  Returns true if there
+   is a problem, otherwise false.  */
+
+static bool
+verify_gimple_transaction (gimple stmt)
+{
+  tree lab = gimple_transaction_label (stmt);
+  if (lab != NULL && TREE_CODE (lab) != LABEL_DECL)
+    return true;
+  return verify_gimple_in_seq_2 (gimple_transaction_body (stmt));
+}
+
 
 /* Verify the GIMPLE statements inside the statement list STMTS.  */
 
@@ -5122,9 +5123,10 @@ gimple_redirect_edge_and_branch (edge e,
     case GIMPLE_TRANSACTION:
       /* The ABORT edge has a stored label associated with it, otherwise
 	 the edges are simply redirectable.  */
-      /* ??? We don't really need this label after the cfg is created.  */
-      if (e->flags == 0)
+      /* ??? We don't really need this label after the cfg is created.
+      if (&e->flags == 0)
 	gimple_transaction_set_label (stmt, gimple_block_label (dest));
+      */
       break;
 
     default:

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [patch] 19/n: trans-mem: middle end/misc patches (LAST PATCH)
  2011-11-06 19:07   ` Aldy Hernandez
@ 2011-11-06 20:47     ` Richard Henderson
  2011-11-06 22:01       ` Aldy Hernandez
                         ` (2 more replies)
  2011-11-06 21:50     ` Richard Henderson
  2011-11-07  9:48     ` Richard Guenther
  2 siblings, 3 replies; 20+ messages in thread
From: Richard Henderson @ 2011-11-06 20:47 UTC (permalink / raw)
  To: Aldy Hernandez; +Cc: Richard Guenther, gcc-patches

On 11/06/2011 10:53 AM, Aldy Hernandez wrote:
>> Did you check bootstrapping GCC with TM enabled and address-space
>> randomization turned on?
> 
> Actually, the table organization is irrelevant, because upon
> registering of the table in the runtime, we qsort the entire thing.

False.  You get the equivalent of bootstrap comparison mismatches.
If we actually used tm during the bootstrap.

The simplest thing to do is to change the hash this table uses.
E.g. use the DECL_UID right from the start, rather than the pointer.

>>> -          if (!computed_goto_p (stmt))
>>> +         if (!computed_goto_p (stmt))
>>>             {
>>> -             tree new_dest = main_block_label (gimple_goto_dest (stmt));
>>> -             gimple_goto_set_dest (stmt, new_dest);
>>> +             label = gimple_goto_dest (stmt);
>>> +             new_label = main_block_label (label);
>>> +             if (new_label != label)
>>> +               gimple_goto_set_dest (stmt, new_label);
>>
>> What's the reason for this changes?  Optimization?
> 
> Yes.  Rth can elaborate if you deem necessary.

Really?  I have no idea what this change achieves.
I actually wonder if this is a merge error.

>>> +    case GIMPLE_TRANSACTION:
>>> +      /* The ABORT edge has a stored label associated with it, otherwise
>>> +        the edges are simply redirectable.  */
>>> +      /* ??? We don't really need this label after the cfg is created.  */
>>> +      if (e->flags == 0)
>>> +       gimple_transaction_set_label (stmt, gimple_block_label (dest));
>>
>> So why set it (and thus keep it live)?
> 
> This seems like leftovers from a previous incantation.  However, I'm not 100% sure, so I have disabled the code, but left it in a comment.  A full bootstrap/regtest revealed no regressions.
> 
> rth, do you have any objections to remove this?

I think that the comment is wrong.  We need that edge, and the label updated until pass_tm_edges, at which point the GIMPLE_TRANSACTION itself goes away.  Thus that label is live throughout the live of the GIMPLE_TRANSACTION node.

Delete that ??? comment instead.

Patch is otherwise ok.


r~

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [patch] 19/n: trans-mem: middle end/misc patches (LAST PATCH)
  2011-11-06 19:07   ` Aldy Hernandez
  2011-11-06 20:47     ` Richard Henderson
@ 2011-11-06 21:50     ` Richard Henderson
  2011-11-07  9:48     ` Richard Guenther
  2 siblings, 0 replies; 20+ messages in thread
From: Richard Henderson @ 2011-11-06 21:50 UTC (permalink / raw)
  To: Aldy Hernandez; +Cc: Richard Guenther, gcc-patches

> On 11/04/11 04:14, Richard Guenther wrote:
>>>     new_version = cgraph_create_node (new_decl);
>>>
>>> -   new_version->analyzed = true;
>>> +   new_version->analyzed = old_version->analyzed;
>>
>> Hm?  analyzed means "with body", sure you have a body if you clone.

Incidentally, for TM we also clone functions that do NOT have a body.

An external declaration with __attribute__((transaction_callable))
is an assertion by the user that the transactional clone exists
(or alternately, a directive from the user to generate such a clone
in the file that contains the function).

>>> @@ -2294,6 +2294,7 @@ cgraph_copy_node_for_versioning (struct
>>>     new_version->rtl = old_version->rtl;
>>>     new_version->reachable = true;
>>>     new_version->count = old_version->count;
>>> +   new_version->lowered = true;
>>
>> OTOH this isn't necessary true.  cgraph exists before lowering.

But no clones are created before lowering.


r~

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [patch] 19/n: trans-mem: middle end/misc patches (LAST PATCH)
  2011-11-06 20:47     ` Richard Henderson
@ 2011-11-06 22:01       ` Aldy Hernandez
  2011-11-07  4:25       ` Aldy Hernandez
  2011-11-07  7:53       ` Aldy Hernandez
  2 siblings, 0 replies; 20+ messages in thread
From: Aldy Hernandez @ 2011-11-06 22:01 UTC (permalink / raw)
  To: Richard Henderson; +Cc: Richard Guenther, gcc-patches

[-- Attachment #1: Type: text/plain, Size: 2279 bytes --]


>> Actually, the table organization is irrelevant, because upon
>> registering of the table in the runtime, we qsort the entire thing.
>
> False.  You get the equivalent of bootstrap comparison mismatches.
> If we actually used tm during the bootstrap.
>
> The simplest thing to do is to change the hash this table uses.
> E.g. use the DECL_UID right from the start, rather than the pointer.

Argh, will fix in a followup patch.

>>>> -          if (!computed_goto_p (stmt))
>>>> +         if (!computed_goto_p (stmt))
>>>>              {
>>>> -             tree new_dest = main_block_label (gimple_goto_dest (stmt));
>>>> -             gimple_goto_set_dest (stmt, new_dest);
>>>> +             label = gimple_goto_dest (stmt);
>>>> +             new_label = main_block_label (label);
>>>> +             if (new_label != label)
>>>> +               gimple_goto_set_dest (stmt, new_label);
>>>
>>> What's the reason for this changes?  Optimization?
>>
>> Yes.  Rth can elaborate if you deem necessary.
>
> Really?  I have no idea what this change achieves.
> I actually wonder if this is a merge error.

I won't complain :).  I have reverted the original patch and am 
including it in the final (attched) version I will commit.

>>>> +    case GIMPLE_TRANSACTION:
>>>> +      /* The ABORT edge has a stored label associated with it, otherwise
>>>> +        the edges are simply redirectable.  */
>>>> +      /* ??? We don't really need this label after the cfg is created.  */
>>>> +      if (e->flags == 0)
>>>> +       gimple_transaction_set_label (stmt, gimple_block_label (dest));
>>>
>>> So why set it (and thus keep it live)?
>>
>> This seems like leftovers from a previous incantation.  However, I'm not 100% sure, so I have disabled the code, but left it in a comment.  A full bootstrap/regtest revealed no regressions.
>>
>> rth, do you have any objections to remove this?
>
> I think that the comment is wrong.  We need that edge, and the label updated until pass_tm_edges, at which point the GIMPLE_TRANSACTION itself goes away.  Thus that label is live throughout the live of the GIMPLE_TRANSACTION node.
>
> Delete that ??? comment instead.

Done.

> Patch is otherwise ok.

Attched is the final revision of the patch.  I will commit once tests 
finish.

thank you.

[-- Attachment #2: curr --]
[-- Type: text/plain, Size: 10144 bytes --]

	* tree-cfg.c (verify_gimple_transaction): Verify body.  Move down.
	(verify_gimple_in_seq_2): Verify the label in a
	GIMPLE_TRANSACTION.
	(cleanup_dead_labels): Remove GIMPLE_GOTO hiccup from merge.
	* function.h (struct function): Move tm_restart field to struct
	gimple_df in tree-flow.h
	Move tm_restart_node to tree-flow.h
	* tree-flow.h (struct gimple_df): New location for tm_restart
	field.
	New location for tm_restart_node.
	(is_transactional_stmt): Remove.
	* trans-mem.c (is_transactional_stmt): Remove.
	(make_tm_edge): Field tm_restart is now in gimple_df.
	* cfgexpand.c (gimple_expand_cfg): Field tm_restart is now in
	cfun->gimple_df.
	Free tm_restart.
	* cfgexpand.c (expand_gimple_stmt): Field tm_restart is now in
	gimple_df.
	* ipa-inline.c (can_inline_edge_p): Do not check flag_tm.
	* trans-mem.c (is_tm_pure): Check flag_tm.
	(is_tm_safe): Same.

Index: ipa-inline.c
===================================================================
--- ipa-inline.c	(revision 181028)
+++ ipa-inline.c	(working copy)
@@ -286,8 +286,7 @@ can_inline_edge_p (struct cgraph_edge *e
     }
   /* TM pure functions should not get inlined if the outer function is
      a TM safe function.  */
-  else if (flag_tm
-	   && is_tm_pure (callee->decl)
+  else if (is_tm_pure (callee->decl)
 	   && is_tm_safe (e->caller->decl))
     {
       e->inline_failed = CIF_UNSPECIFIED;
Index: function.h
===================================================================
--- function.h	(revision 181028)
+++ function.h	(working copy)
@@ -467,14 +467,6 @@ extern GTY(()) struct rtl_data x_rtl;
    want to do differently.  */
 #define crtl (&x_rtl)
 
-/* This structure is used to map a gimple statement to a label,
-   or list of labels to represent transaction restart.  */
-
-struct GTY(()) tm_restart_node {
-  gimple stmt;
-  tree label_or_list;
-};
-
 struct GTY(()) stack_usage
 {
   /* # of bytes of static stack space allocated by the function.  */
@@ -526,10 +518,6 @@ struct GTY(()) function {
   /* Value histograms attached to particular statements.  */
   htab_t GTY((skip)) value_histograms;
 
-  /* Map gimple stmt to tree label (or list of labels) for transaction
-     restart and abort.  */
-  htab_t GTY ((param_is (struct tm_restart_node))) tm_restart;
-
   /* For function.c.  */
 
   /* Points to the FUNCTION_DECL of this function.  */
Index: trans-mem.c
===================================================================
--- trans-mem.c	(revision 181028)
+++ trans-mem.c	(working copy)
@@ -172,9 +172,13 @@ get_attrs_for (const_tree x)
 bool
 is_tm_pure (const_tree x)
 {
-  tree attrs = get_attrs_for (x);
-  if (attrs)
-    return lookup_attribute ("transaction_pure", attrs) != NULL;
+  if (flag_tm)
+    {
+      tree attrs = get_attrs_for (x);
+      if (attrs)
+	return lookup_attribute ("transaction_pure", attrs) != NULL;
+      return false;
+    }
   return false;
 }
 
@@ -205,16 +209,17 @@ is_tm_irrevocable (tree x)
 bool
 is_tm_safe (const_tree x)
 {
-  tree attrs = get_attrs_for (x);
-
-  if (attrs)
+  if (flag_tm)
     {
-      if (lookup_attribute ("transaction_safe", attrs))
-	return true;
-      if (lookup_attribute ("transaction_may_cancel_outer", attrs))
-	return true;
+      tree attrs = get_attrs_for (x);
+      if (attrs)
+	{
+	  if (lookup_attribute ("transaction_safe", attrs))
+	    return true;
+	  if (lookup_attribute ("transaction_may_cancel_outer", attrs))
+	    return true;
+	}
     }
-
   return false;
 }
 
@@ -270,22 +275,6 @@ is_tm_may_cancel_outer (tree x)
   return false;
 }
 
-/* Return true if STMT may alter control flow via a transactional edge.  */
-
-bool
-is_transactional_stmt (const_gimple stmt)
-{
-  switch (gimple_code (stmt))
-    {
-    case GIMPLE_CALL:
-      return (gimple_call_flags (stmt) & ECF_TM_OPS) != 0;
-    case GIMPLE_TRANSACTION:
-      return true;
-    default:
-      return false;
-    }
-}
-
 /* Return true for built in functions that "end" a transaction.   */
 
 bool
@@ -2470,13 +2459,13 @@ make_tm_edge (gimple stmt, basic_block b
   void **slot;
   struct tm_restart_node *n, dummy;
 
-  if (cfun->tm_restart == NULL)
-    cfun->tm_restart = htab_create_ggc (31, struct_ptr_hash,
-					struct_ptr_eq, ggc_free);
+  if (cfun->gimple_df->tm_restart == NULL)
+    cfun->gimple_df->tm_restart = htab_create_ggc (31, struct_ptr_hash,
+						   struct_ptr_eq, ggc_free);
 
   dummy.stmt = stmt;
   dummy.label_or_list = gimple_block_label (region->entry_block); 
-  slot = htab_find_slot (cfun->tm_restart, &dummy, INSERT);
+  slot = htab_find_slot (cfun->gimple_df->tm_restart, &dummy, INSERT);
   n = (struct tm_restart_node *) *slot;
   if (n == NULL)
     {
Index: cfgexpand.c
===================================================================
--- cfgexpand.c	(revision 181028)
+++ cfgexpand.c	(working copy)
@@ -2097,13 +2097,13 @@ expand_gimple_stmt (gimple stmt)
     }
 
   /* Mark all calls that can have a transaction restart.  */
-  if (cfun->tm_restart && is_gimple_call (stmt))
+  if (cfun->gimple_df->tm_restart && is_gimple_call (stmt))
     {
       struct tm_restart_node dummy;
       void **slot;
 
       dummy.stmt = stmt;
-      slot = htab_find_slot (cfun->tm_restart, &dummy, NO_INSERT);
+      slot = htab_find_slot (cfun->gimple_df->tm_restart, &dummy, NO_INSERT);
       if (slot)
 	{
 	  struct tm_restart_node *n = (struct tm_restart_node *) *slot;
@@ -4483,7 +4483,11 @@ gimple_expand_cfg (void)
   naked_return_label = NULL;
 
   /* After expanding, the tm_restart map is no longer needed.  */
-  cfun->tm_restart = NULL;
+  if (cfun->gimple_df->tm_restart)
+    {
+      htab_delete (cfun->gimple_df->tm_restart);
+      cfun->gimple_df->tm_restart = NULL;
+    }
 
   /* Tag the blocks with a depth number so that change_scope can find
      the common parent easily.  */
Index: tree-flow.h
===================================================================
--- tree-flow.h	(revision 181028)
+++ tree-flow.h	(working copy)
@@ -33,6 +33,14 @@ along with GCC; see the file COPYING3.  
 #include "tree-ssa-alias.h"
 
 
+/* This structure is used to map a gimple statement to a label,
+   or list of labels to represent transaction restart.  */
+
+struct GTY(()) tm_restart_node {
+  gimple stmt;
+  tree label_or_list;
+};
+
 /* Gimple dataflow datastructure. All publicly available fields shall have
    gimple_ accessor defined in tree-flow-inline.h, all publicly modifiable
    fields should have gimple_set accessor.  */
@@ -80,6 +88,10 @@ struct GTY(()) gimple_df {
   unsigned int ipa_pta : 1;
 
   struct ssa_operands ssa_operands;
+
+  /* Map gimple stmt to tree label (or list of labels) for transaction
+     restart and abort.  */
+  htab_t GTY ((param_is (struct tm_restart_node))) tm_restart;
 };
 
 /* Accessors for internal use only.  Generic code should use abstraction
@@ -778,9 +790,6 @@ extern bool maybe_duplicate_eh_stmt (gim
 extern bool verify_eh_edges (gimple);
 extern bool verify_eh_dispatch_edge (gimple);
 
-/* In gtm-low.c  */
-extern bool is_transactional_stmt (const_gimple);
-
 /* In tree-ssa-pre.c  */
 struct pre_expr_d;
 void add_to_value (unsigned int, struct pre_expr_d *);
Index: tree-cfg.c
===================================================================
--- tree-cfg.c	(revision 181028)
+++ tree-cfg.c	(working copy)
@@ -117,6 +117,7 @@ static int gimple_verify_flow_info (void
 static void gimple_make_forwarder_block (edge);
 static void gimple_cfg2vcg (FILE *);
 static gimple first_non_label_stmt (basic_block);
+static bool verify_gimple_transaction (gimple);
 
 /* Flowgraph optimization and cleanup.  */
 static void gimple_merge_blocks (basic_block, basic_block);
@@ -1262,27 +1263,13 @@ cleanup_dead_labels (void)
 	/* We have to handle gotos until they're removed, and we don't
 	   remove them until after we've created the CFG edges.  */
 	case GIMPLE_GOTO:
-	  if (!computed_goto_p (stmt))
+          if (!computed_goto_p (stmt))
 	    {
-	      label = gimple_goto_dest (stmt);
-	      new_label = main_block_label (label);
-	      if (new_label != label)
-		gimple_goto_set_dest (stmt, new_label);
+	      tree new_dest = main_block_label (gimple_goto_dest (stmt));
+	      gimple_goto_set_dest (stmt, new_dest);
 	    }
 	  break;
 
-	case GIMPLE_TRANSACTION:
-	  {
-	    tree label = gimple_transaction_label (stmt);
-	    if (label)
-	      {
-		tree new_label = main_block_label (label);
-		if (new_label != label)
-		  gimple_transaction_set_label (stmt, new_label);
-	      }
-	  }
-	  break;
-
 	default:
 	  break;
       }
@@ -4098,18 +4085,6 @@ verify_gimple_switch (gimple stmt)
   return false;
 }
 
-/* Verify the contents of a GIMPLE_TRANSACTION.  Returns true if there
-   is a problem, otherwise false.  */
-
-static bool
-verify_gimple_transaction (gimple stmt)
-{
-  tree lab = gimple_transaction_label (stmt);
-  if (lab != NULL && TREE_CODE (lab) != LABEL_DECL)
-    return true;
-  return false;
-}
-
 /* Verify a gimple debug statement STMT.
    Returns true if anything is wrong.  */
 
@@ -4339,7 +4314,7 @@ verify_gimple_in_seq_2 (gimple_seq stmts
 	  break;
 
 	case GIMPLE_TRANSACTION:
-	  err |= verify_gimple_in_seq_2 (gimple_transaction_body (stmt));
+	  err |= verify_gimple_transaction (stmt);
 	  break;
 
 	default:
@@ -4355,6 +4330,18 @@ verify_gimple_in_seq_2 (gimple_seq stmts
   return err;
 }
 
+/* Verify the contents of a GIMPLE_TRANSACTION.  Returns true if there
+   is a problem, otherwise false.  */
+
+static bool
+verify_gimple_transaction (gimple stmt)
+{
+  tree lab = gimple_transaction_label (stmt);
+  if (lab != NULL && TREE_CODE (lab) != LABEL_DECL)
+    return true;
+  return verify_gimple_in_seq_2 (gimple_transaction_body (stmt));
+}
+
 
 /* Verify the GIMPLE statements inside the statement list STMTS.  */
 
@@ -5122,7 +5109,6 @@ gimple_redirect_edge_and_branch (edge e,
     case GIMPLE_TRANSACTION:
       /* The ABORT edge has a stored label associated with it, otherwise
 	 the edges are simply redirectable.  */
-      /* ??? We don't really need this label after the cfg is created.  */
       if (e->flags == 0)
 	gimple_transaction_set_label (stmt, gimple_block_label (dest));
       break;

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [patch] 19/n: trans-mem: middle end/misc patches (LAST PATCH)
  2011-11-06 20:47     ` Richard Henderson
  2011-11-06 22:01       ` Aldy Hernandez
@ 2011-11-07  4:25       ` Aldy Hernandez
  2011-11-07  7:53       ` Aldy Hernandez
  2 siblings, 0 replies; 20+ messages in thread
From: Aldy Hernandez @ 2011-11-07  4:25 UTC (permalink / raw)
  To: Richard Henderson; +Cc: Richard Guenther, gcc-patches

[-- Attachment #1: Type: text/plain, Size: 1246 bytes --]

On 11/06/11 12:20, Richard Henderson wrote:
>>>> -          if (!computed_goto_p (stmt))
>>>> +         if (!computed_goto_p (stmt))
>>>>              {
>>>> -             tree new_dest = main_block_label (gimple_goto_dest (stmt));
>>>> -             gimple_goto_set_dest (stmt, new_dest);
>>>> +             label = gimple_goto_dest (stmt);
>>>> +             new_label = main_block_label (label);
>>>> +             if (new_label != label)
>>>> +               gimple_goto_set_dest (stmt, new_label);
>>>
>>> What's the reason for this changes?  Optimization?
>>
>> Yes.  Rth can elaborate if you deem necessary.
>
> Really?  I have no idea what this change achieves.
> I actually wonder if this is a merge error.

Removing this caused various TM tests failures, which I have yet to 
fully investigate.  I found the original patch by you [rth] (attached). 
  Perhaps you can elaborate as to its original use.

It may be that we need to remove all of the GIMPLE_GOTO, GIMPLE_COND, 
and GIMPLE_SWITCH hacks in cleanup_dead_labels, but I will wait for a 
double check by you before touching any more of this.

In the meantime, I will commit the patch sans this GIMPLE_GOTO removal 
which may still be used.  That is, after another round of tests.

[-- Attachment #2: original-goto-patch --]
[-- Type: text/plain, Size: 54583 bytes --]

Index: cgraphbuild.c
===================================================================
--- cgraphbuild.c	(revision 141199)
+++ cgraphbuild.c	(revision 141200)
@@ -125,7 +125,7 @@ compute_call_stmt_bb_frequency (basic_bl
 /* Eagerly clone functions so that TM expansion can create
    and redirect calls to a transactional clone.  */
 
-static void
+static void ATTRIBUTE_UNUSED
 prepare_tm_clone (struct cgraph_node *node)
 {
   struct cgraph_node *tm_node;
@@ -275,7 +275,7 @@ build_cgraph_edges (void)
 
   build_cgraph_edges_from_node (node);
   
-  prepare_tm_clone (node);
+  /* prepare_tm_clone (node); */
 
   return 0;
 }
Index: tree-pass.h
===================================================================
--- tree-pass.h	(revision 141199)
+++ tree-pass.h	(revision 141200)
@@ -388,7 +388,7 @@ extern struct gimple_opt_pass pass_reass
 extern struct gimple_opt_pass pass_rebuild_cgraph_edges;
 extern struct gimple_opt_pass pass_build_cgraph_edges;
 extern struct gimple_opt_pass pass_reset_cc_flags;
-extern struct gimple_opt_pass pass_expand_tm;
+extern struct gimple_opt_pass pass_lower_tm;
 extern struct gimple_opt_pass pass_checkpoint_tm;
 
 /* IPA Passes */
Index: gtm-low.c
===================================================================
--- gtm-low.c	(revision 141199)
+++ gtm-low.c	(revision 141200)
@@ -1,6 +1,6 @@
 /* Lowering pass for transactional memory directives.
    Converts markers of transactions into explicit calls to
-   the STM runtime library.
+   the TM runtime library.
 
    Copyright (C) 2008 Free Software Foundation, Inc.
 
@@ -18,34 +18,144 @@
 
    You should have received a copy of the GNU General Public License
    along with GCC; see the file COPYING3.  If not see
-   <http://www.gnu.org/licenses/>.
-
-*/
+   <http://www.gnu.org/licenses/>.  */
 
 #include "config.h"
 #include "system.h"
 #include "coretypes.h"
 #include "tm.h"
 #include "tree.h"
-#include "rtl.h"
 #include "gimple.h"
-#include "langhooks.h"
-#include "diagnostic.h"
 #include "tree-flow.h"
-#include "timevar.h"
-#include "flags.h"
-#include "function.h"
-#include "expr.h"
-#include "toplev.h"
 #include "tree-pass.h"
-#include "ggc.h"
 #include "except.h"
-#include "splay-tree.h"
-#include "optabs.h"
-#include "cfgloop.h"
-#include "tree-ssa-live.h"
-#include "tree-flow.h"
+#include "diagnostic.h"
+
+
+/* The representation of a transaction changes several times during the
+   lowering process.  In the beginning, in the front-end we have the
+   GENERIC tree TM_ATOMIC.  For example,
+
+	__tm_atomic {
+	  local++;
+	  if (++global == 10)
+	    __tm_abort;
+	}
+
+  is represented as
+
+	TM_ATOMIC {
+	  local++;
+	  if (++global == 10)
+	    __builtin___tm_abort ();
+	}
+
+  During initial gimplification (gimplify.c) the TM_ATOMIC node is
+  trivially replaced with a GIMPLE_TM_ATOMIC node, and we add bits
+  to handle EH cleanup of the transaction:
+
+	GIMPLE_TM_ATOMIC [label=NULL] {
+	  try {
+	    local = local + 1;
+	    t0 [tm_load]= global;
+	    t1 = t0 + 1;
+	    global [tm_store]= t1;
+	    if (t1 == 10)
+	      __builtin___tm_abort ();
+	  } finally {
+	    __builtin___tm_commit ();
+	  }
+	}
+
+  During pass_lower_eh, we create EH regions for the transactions,
+  intermixed with the regular EH stuff.  This gives us a nice persistent
+  mapping (all the way through rtl) from transactional memory operation
+  back to the transaction, which allows us to get the abnormal edges
+  correct to model transaction aborts and restarts.
+
+  During pass_lower_tm, we mark the gimple statements that perform
+  transactional memory operations with TM_LOAD/TM_STORE, and swap out
+  function calls with their (non-)transactional clones.  At this time
+  we flatten nested transactions (when possible), and flatten the
+  GIMPLE representation.
+
+	GIMPLE_TM_ATOMIC [label=over]
+	eh_label:
+	local = local + 1;
+	t0 [tm_load]= global;
+	t1 = t0 + 1;
+	global [tm_store]= t1;
+	if (t1 == 10)
+	  __builtin___tm_abort ();
+	__builtin___tm_commit ();
+	over:
+
+  During pass_checkpoint_tm, we complete the lowering of the
+  GIMPLE_TM_ATOMIC node.  Here we examine the SSA web and arange for
+  local variables to be saved and restored in the event of an abort.
+
+	save_local = local;
+	x = __builtin___tm_start (MAY_ABORT);
+	eh_label:
+	if (x & restore_locals) {
+	  local = save_local;
+	}
+	if (x & abort_transaction)
+	  goto over;
+	local = local + 1;
+	t0 [tm_load]= global;
+	t1 = t0 + 1;
+	global [tm_store]= t1;
+	if (t1 == 10)
+	  __builtin___tm_abort ();
+	__builtin___tm_commit ();
+	over:
+
+  During expansion to rtl, we expand the TM_LOAD/TM_STORE markings
+  with calls to the appropriate builtin functions.  Delaying this long
+  allows the tree optimizers the most visibility into the operations.  */
+
+DEF_VEC_O(gimple_stmt_iterator);
+DEF_VEC_ALLOC_O(gimple_stmt_iterator,heap);
+
+struct ltm_state
+{
+  /* Bits to be stored in the GIMPLE_TM_ATOMIC subcode.  */
+  unsigned subcode;
+
+  /* The EH region number for this transaction.  Non-negative numbers
+     represent an active transaction within this function; -1 represents
+     an active transaction from a calling function (i.e. we're compiling
+     a transaction clone).  For no active transaction, the state pointer
+     passed will be null.  */
+  int region_nr;
+
+  /* Record the iterator pointing to a __tm_commit function call that
+     binds to this transaction region.  There may be many such calls,
+     depending on how the EH expansion of the try-finally node went.
+     But there's usually exactly one such call, and essentially always
+     only a small number, so to avoid rescanning the entire sequence
+     when we need to remove these calls, record the iterator location.  */
+  VEC(gimple_stmt_iterator,heap) *commit_stmts;
+};
+
+
+static void lower_sequence_tm (struct ltm_state *, gimple_seq);
+static void lower_sequence_no_tm (gimple_seq);
+
 
+/* Record the transaction for this statement.  If the statement
+   already has a region number that's fine -- it means that the
+   statement can also throw.  If there's no region number, it 
+   means we're expanding a transactional clone and the region
+   is in a calling function.  */
+
+static void
+add_stmt_to_transaction (struct ltm_state *state, gimple stmt)
+{
+  if (state->region_nr >= 0 && lookup_stmt_eh_region (stmt) < 0)
+    add_stmt_to_eh_region (stmt, state->region_nr);
+}
 
 /* Determine whether X has to be instrumented using a read
    or write barrier.  */
@@ -78,31 +188,47 @@ requires_barrier (tree x)
     }
 }
 
-/* Subsituting a MODIFY_STMT with calls to the STM runtime.  */
+/* Mark the GIMPLE_ASSIGN statement as appropriate for being inside
+   a transaction region.  */
 
 static void
-maybe_transactify_assign (gimple stmt)
+lower_assign_tm (struct ltm_state *state, gimple_stmt_iterator *gsi)
 {
+  gimple stmt = gsi_stmt (*gsi);
   bool load_p = requires_barrier (gimple_assign_rhs1 (stmt));
   bool store_p = requires_barrier (gimple_assign_lhs (stmt));
 
-  if (load_p)
+  if (load_p && store_p)
+    {
+      /* ??? This is a copy between two aggregates in memory.  I
+	 believe the Intel compiler handles this with a special
+	 version of memcpy.  For now, just consider the transaction
+	 irrevokable at this point.  */
+      state->subcode |= GTMA_HAVE_CALL_IRREVOKABLE;
+      return;
+    }
+  else if (load_p)
     {
-      gcc_assert (!store_p);
       gimple_assign_set_rhs_code (stmt, TM_LOAD);
+      state->subcode |= GTMA_HAVE_LOAD;
     }
   else if (store_p)
-    gimple_assign_set_rhs_code (stmt, TM_STORE);
+    {
+      gimple_assign_set_rhs_code (stmt, TM_STORE);
+      state->subcode |= GTMA_HAVE_STORE;
+    }
+  else
+    return;
+
+  add_stmt_to_transaction (state, stmt);
 }
 
-/* Helper function that replaces call expressions inside
-   transactions and issues a warning if no transactional
-   clone is found. */
+/* Mark a GIMPLE_CALL as appropriate for being inside a transaction.  */
 
 static void
-maybe_transactify_call (gimple stmt)
+lower_call_tm (struct ltm_state *state, gimple_stmt_iterator *gsi)
 {
-  bool redirected = false;
+  gimple stmt = gsi_stmt (*gsi);
   tree fn_decl;
   struct cgraph_node *node, *orig_node;
   int flags;
@@ -114,9 +240,28 @@ maybe_transactify_call (gimple stmt)
   fn_decl = gimple_call_fndecl (stmt);
   if (!fn_decl)
     {
-      warning (0, "Indirect call potentially breaks isolation of transactions");
+      state->subcode |= GTMA_HAVE_CALL_INDIRECT;
       return;
     }
+
+  /* Check if this call is one of our transactional builtins.  */
+  if (DECL_BUILT_IN_CLASS (fn_decl) == BUILT_IN_NORMAL)
+    switch (DECL_FUNCTION_CODE (fn_decl))
+      {
+      case BUILT_IN_TM_COMMIT:
+	/* Remember the commit so that we can remove it if
+	   we decide to elide the transaction.  */
+	VEC_safe_push (gimple_stmt_iterator,heap, state->commit_stmts, gsi);
+	return;
+      case BUILT_IN_TM_ABORT:
+	state->subcode |= GTMA_HAVE_ABORT;
+	add_stmt_to_transaction (state, stmt);
+	return;
+
+      default:
+	break;
+      }
+
   if (DECL_IS_TM_PURE (fn_decl))
     return;
 
@@ -129,12 +274,11 @@ maybe_transactify_call (gimple stmt)
       if (DECL_IS_TM_CLONE (node->decl))
 	break;
     }
-
   if (DECL_IS_TM_CLONE (node->decl))
     {
       struct cgraph_edge *callers = orig_node->callers;
 
-      /* Find appropriate call stmt to redirect */
+      /* Find appropriate call stmt to redirect.  */
       while (callers)
 	{
 	  if (callers->call_stmt == stmt)
@@ -142,68 +286,192 @@ maybe_transactify_call (gimple stmt)
 	  callers = callers->next_caller;
 	}
 
-      /* Substitute call stmt. */
+      /* Substitute call stmt.  */
       if (callers)
 	{
 	  gimple_call_set_fndecl (stmt, node->decl);
 	  cgraph_redirect_edge_callee (callers, node);
 	  if (dump_file)
-	    fprintf (dump_file, "redirected edge to %s\n",
-		     get_name (node->decl));
-	  redirected = true;
+	    {
+	      fprintf (dump_file, "redirected edge to");
+	      print_generic_expr (dump_file, node->decl, 0);
+	      fputc ('\n', dump_file);
+	    }
+
+	  state->subcode |= GTMA_HAVE_CALL_TM;
+	  add_stmt_to_transaction (state, stmt);
+	  return;
 	}
     }
 
-  /* In case the function call was not redirected and the function
-     not marked as const or tm_pure, issue a warning. */
-  /* ??? Handling of calls to irrevocable functions can be expanded here. */
-  if (!redirected)
-    warning (0, "Call to %qD potentially breaks isolation of transactions.",
-	     fn_decl);
+  /* The function was not const, tm_pure, or redirected to a 
+     transactional clone.  The call is therefore considered to
+     be irrevokable.  */
+  state->subcode |= GTMA_HAVE_CALL_IRREVOKABLE;
+}
+
+/* Remove any calls to __tm_commit inside STATE which belong
+   to the transaction.  */
+
+static void
+remove_tm_commits (struct ltm_state *state)
+{
+  gimple_stmt_iterator *gsi;
+  unsigned i;
+
+  for (i = 0;
+       VEC_iterate(gimple_stmt_iterator, state->commit_stmts, i, gsi);
+       ++i)
+    gsi_remove (gsi, true);
 }
 
-/* This function expands the stmts within a transaction so that
-   the corresponding STM versions of the stmt is called. */
+/* Lower a GIMPLE_TM_ATOMIC statement.  The GSI is advanced.  */
 
-static void ATTRIBUTE_UNUSED
-transactify_stmt (gimple_stmt_iterator *gsi)
+static void
+lower_tm_atomic (struct ltm_state *outer_state, gimple_stmt_iterator *gsi)
 {
   gimple stmt = gsi_stmt (*gsi);
+  struct ltm_state this_state;
+  struct eh_region *eh_region;
+  tree label;
+
+  this_state.subcode = 0;
+  this_state.region_nr = lookup_stmt_eh_region (stmt);
+  this_state.commit_stmts = VEC_alloc(gimple_stmt_iterator, heap, 1);
+
+  gcc_assert (this_state.region_nr >= 0);
+  eh_region = get_eh_region_from_number (this_state.region_nr);
+
+  /* First, lower the body.  The scanning that we do inside gives
+     us some idea of what we're dealing with.  */
+  lower_sequence_tm (&this_state, gimple_seq_body (stmt));
+
+  /* If there was absolutely nothing transaction related inside the
+     transaction, we may elide it.  Likewise if this is a nested
+     transaction and does not contain an abort.  */
+  if (this_state.subcode == 0
+      || (!(this_state.subcode & GTMA_HAVE_ABORT)
+	  && outer_state != NULL))
+    {
+      if (outer_state)
+	outer_state->subcode |= this_state.subcode;
 
-  switch (gimple_code (stmt))
+      remove_tm_commits (&this_state);
+      gsi_insert_seq_before (gsi, gimple_seq_body (stmt), GSI_SAME_STMT);
+      gimple_seq_set_body (stmt, NULL);
+      gsi_remove (gsi, true);
+      remove_eh_handler (eh_region);
+      goto fini;
+    }
+
+  /* Insert an EH_LABEL immediately after the GIMPLE_TM_ATOMIC node.
+     This label won't really be used, but it mimicks the effect of 
+     the setjmp/longjmp that's going on behind the scenes.  */
+  label = create_artificial_label ();
+  set_eh_region_tree_label (eh_region, label);
+  gsi_insert_after (gsi, gimple_build_label (label), GSI_CONTINUE_LINKING);
+
+  /* Insert the entire transaction sequence.  */
+  gsi_insert_seq_after (gsi, gimple_seq_body (stmt), GSI_CONTINUE_LINKING);
+  gimple_seq_set_body (stmt, NULL);
+
+  /* Record a label at the end of the transaction that will be used in
+     case the transaction aborts.  */
+  if (this_state.subcode & GTMA_HAVE_ABORT)
     {
-    case GIMPLE_CALL:
-      maybe_transactify_call (stmt);
-      break;
+      label = create_artificial_label ();
+      gimple_tm_atomic_set_label (stmt, label);
+      gsi_insert_after (gsi, gimple_build_label (label), GSI_CONTINUE_LINKING);
+    }
 
-    case GIMPLE_ASSIGN:
-      /* Only memory reads/writes need to be instrumented.  */
-      if (gimple_assign_single_p (stmt))
-	maybe_transactify_assign (stmt);
-      break;
+  /* Record the set of operations found for use during final lowering
+     of the GIMPLE_TM_ATOMIC node.  */
+  gimple_tm_atomic_set_subcode (stmt, this_state.subcode);
 
-    default:
-      break;
+  /* Always update the iterator.  */
+  gsi_next (gsi);
+
+ fini:
+  VEC_free (gimple_stmt_iterator, heap, this_state.commit_stmts);
+}
+
+/* Iterate through the statements in the sequence, lowering them all
+   as appropriate for being in a transaction.  */
+
+static void
+lower_sequence_tm (struct ltm_state *state, gimple_seq seq)
+{
+  gimple_stmt_iterator gsi;
+
+  for (gsi = gsi_start (seq); !gsi_end_p (gsi); )
+    {
+      gimple stmt = gsi_stmt (gsi);
+      switch (gimple_code (stmt))
+	{
+	case GIMPLE_ASSIGN:
+	  /* Only memory reads/writes need to be instrumented.  */
+	  if (gimple_assign_single_p (stmt))
+	    lower_assign_tm (state, &gsi);
+	  break;
+
+	case GIMPLE_CALL:
+	  lower_call_tm (state, &gsi);
+	  break;
+
+	case GIMPLE_TM_ATOMIC:
+	  lower_tm_atomic (state, &gsi);
+	  goto no_update;
+
+	default:
+	  break;
+	}
+      gsi_next (&gsi);
+    no_update:
+      ;
     }
 }
 
-/* Main entry point for expanding TM-GIMPLE into runtime calls to the STM. */
+/* Iterate through the statements in the sequence, lowering them all
+   as appropriate for being outside of a transaction.  */
 
-static unsigned int
-execute_expand_tm (void)
+static void
+lower_sequence_no_tm (gimple_seq seq)
 {
-  bool in_transaction = false;
+  gimple_stmt_iterator gsi;
+
+  for (gsi = gsi_start (seq); !gsi_end_p (gsi); )
+    if (gimple_code (gsi_stmt (gsi)) == GIMPLE_TM_ATOMIC)
+      lower_tm_atomic (NULL, &gsi);
+    else
+      gsi_next (&gsi);
+}
+
+/* Main entry point for flattening GIMPLE_TM_ATOMIC constructs.  After
+   this, GIMPLE_TM_ATOMIC nodes still exist, but the nested body has
+   been moved out, and all the data required for constructing a proper
+   CFG has been recorded.  */
 
+static unsigned int
+execute_lower_tm (void)
+{
   /* Functions that are marked TM_PURE don't need annotation by definition.  */
+  /* ??? The Intel OOPSLA paper talks about performing the same scan of the
+     function as we would if the function was marked DECL_IS_TM_CLONE, and
+     warning if we find anything for which we would have made a change.  */
   if (DECL_IS_TM_PURE (current_function_decl))
     return 0;
 
   /* When instrumenting a transactional clone, we begin the function inside
      a transaction.  */
   if (DECL_IS_TM_CLONE (current_function_decl))
-    in_transaction = true;
-
-  /* Walk dominator tree expanding blocks.  Seed with in_transaction.  */
+    {
+      struct ltm_state state;
+      state.subcode = 0;
+      state.region_nr = -1;
+      lower_sequence_tm (&state, gimple_body (current_function_decl));
+    }
+  else
+    lower_sequence_no_tm (gimple_body (current_function_decl));
 
   return 0;
 }
@@ -216,227 +484,162 @@ gate_tm (void)
   return flag_tm;
 }
 
-struct gimple_opt_pass pass_expand_tm =
+struct gimple_opt_pass pass_lower_tm =
 {
  {
   GIMPLE_PASS,
-  "tmexp",				/* name */
+  "tmlower",				/* name */
   gate_tm,				/* gate */
-  execute_expand_tm,			/* execute */
+  execute_lower_tm,			/* execute */
   NULL,					/* sub */
   NULL,					/* next */
   0,					/* static_pass_number */
   0,					/* tv_id */
-  PROP_gimple_any,			/* properties_required */
+  PROP_gimple_leh,			/* properties_required */
   0,			                /* properties_provided */
   0,					/* properties_destroyed */
   0,					/* todo_flags_start */
-  TODO_dump_func
-  | TODO_cleanup_cfg
-  | TODO_ggc_collect,		        /* todo_flags_finish */
+  TODO_dump_func		        /* todo_flags_finish */
  }
 };
+\f
 
-
-#if 0
-/* Calculate live ranges on SSA. Then checkpoint the
-   live-in variables to the transaction. */
+/* ??? Find real values for these bits.  */
+#define TM_START_RESTORE_LIVE_IN	1
+#define TM_START_ABORT			2
+
+/* All local variables that have been modified since the beginning of the
+   transaction up until the last possible transaction restart need to be
+   reset when we restart the transaction.
+
+   Here we implement this by replacing the new SSA_NAME created by the
+   PHI at the join of the abnormal backedges by the old SSA_NAME that
+   was originally live at entry to the transaction.  This does result in
+   the old SSA_NAME continuing to stay live when new values are defined,
+   and this isn't necessarily the most efficient implementation, but it
+   is just about the easiest.  */
 
 static void
-checkpoint_live_in_variables (struct tm_region *region,
-			      gimple_stmt_iterator *gsi_recover,
-			      basic_block begin_bb)
-{
-  int index = begin_bb->index;
-  block_stmt_iterator bsi_save = bsi_for_stmt (region->setjmp_stmt);
-  basic_block save_bb = bb_for_stmt (region->setjmp_stmt);
-  basic_block recover_bb = bb_for_stmt (bsi_stmt (*bsi_recover));
-  tree ssa_var;
-  tree_live_info_p liveinfo;
-  var_map map;
-  int p;
-  tree rep;
-  unsigned int i;
-  unsigned int j;
-  bitmap_iterator bi;
-
-  map = init_var_map (num_ssa_names + 1);
-
-  /* Create liveness information for each SSA_NAME. */
-  for (j = 0; j < num_ssa_names; j++)
-    {
-      ssa_var = ssa_name (j);
-      if (!ssa_var)
-	continue;
-
-      if (TREE_CODE (ssa_var) == SSA_NAME)
-	{
-	  register_ssa_partition (map, ssa_var);
-	  p = partition_find (map->var_partition, SSA_NAME_VERSION (ssa_var));
-	  gcc_assert (p != NO_PARTITION);
-	  rep = partition_to_var (map, p);
-	}
-    }
-
-  liveinfo = calculate_live_ranges (map);
+checkpoint_live_in_variables (edge e)
+{
+  gimple_stmt_iterator gsi;
 
-  /* If variable is live-in at beginning of the
-     transaction checkpoint its value. */
-  if (liveinfo->livein)
+  for (gsi = gsi_start_phis (e->dest); !gsi_end_p (gsi); )
     {
-      if (dump_file)
-	fprintf (dump_file, "\nCheckpoint variables for transaction. BB %d : ", index);
-
-      EXECUTE_IF_SET_IN_BITMAP (liveinfo->livein[index], 0, i, bi)
+      gimple phi = gsi_stmt (gsi);
+      tree old_ssa, new_ssa;
+      unsigned i, n;
+
+      new_ssa = gimple_phi_result (phi);
+      old_ssa = gimple_phi_arg_def (phi, e->dest_idx);
+
+      /* We need to recompute SSA_NAME_OCCURS_IN_ABNORMAL_PHI on each
+	 of the other arguments to the PHI, discounting the one abnormal
+	 phi node that we're about to delete.  */
+      for (i = 0, n = gimple_phi_num_args (phi); i < n; ++i)
 	{
-	  tree var =  partition_to_var (map, i);
+	  tree arg = gimple_phi_arg_def (phi, i);
+	  imm_use_iterator imm_iter;
+	  use_operand_p use_p;
+	  bool in_abnormal_phi;
+
+	  if (TREE_CODE (arg) != SSA_NAME
+	      || !SSA_NAME_OCCURS_IN_ABNORMAL_PHI (arg))
+	    continue;
 
-	  /* TODO check restricts the use of temporaries by the compiler
-	     may impact other optimisations.
-	     Maybe reordering this part of the checkpointing before introducing
-	     temporary variables would avoid this check. */
-	  if ((!DECL_ARTIFICIAL (SSA_NAME_VAR (var)))
-	      && (!POINTER_TYPE_P (TREE_TYPE (var))))
+	  in_abnormal_phi = false;
+	  FOR_EACH_IMM_USE_FAST (use_p, imm_iter, arg)
 	    {
-	      if (dump_file)
+	      gimple phi2 = USE_STMT (use_p);
+	      if (gimple_code (phi2) == GIMPLE_PHI && phi2 != phi)
 		{
-		  print_generic_expr (dump_file, var, TDF_SLIM);
-		  fprintf (dump_file, "  ");
-		}
-	      /* Create name for temporary variable
-		 that checkpoints value of var. */
-	      const char* orig = get_name (SSA_NAME_VAR (var));
-	      int len = strlen (orig);
-	      char *name = xmalloc (sizeof (char) * (len + 10));
-	      strncpy (name, "txn_save_", 9);
-	      strncpy (name + 9, orig, len);
-	      *(name + len + 9) = '\0';
-
-	      /* Create temporary. */
-	      tree type = TREE_TYPE (var);
-	      tree save = create_tmp_var (type, name);
-	      add_referenced_var (save);
-	      tree stmt;
-
-	      /* Create gimple statement for saving value of var. */
-	      stmt = fold_build2 (GIMPLE_MODIFY_STMT, type, save, var);
-	      tree real_save = make_ssa_name (save, stmt);
-	      SSA_NAME_OCCURS_IN_ABNORMAL_PHI (real_save) = true;
-	      GIMPLE_STMT_OPERAND (stmt, 0) = real_save;
-
-	      bsi_insert_before (&bsi_save, stmt, BSI_SAME_STMT);
-
-	      /* Create gimple statement for restoring value of var. */
- 	      stmt = fold_build2 (GIMPLE_MODIFY_STMT, type, var, real_save);
-	      tree new_var = make_ssa_name (SSA_NAME_VAR (var), stmt);
-	      GIMPLE_STMT_OPERAND (stmt, 0) = new_var;
-	      bsi_insert_before (bsi_recover, stmt, BSI_SAME_STMT);
-
-	      /* Merge saved or recovered values before next basic block. */
-	      tree phi = create_phi_node (SSA_NAME_VAR (var), begin_bb);
-	      add_phi_arg (phi, new_var, FALLTHRU_EDGE (recover_bb));
-	      add_phi_arg (phi, var, FALLTHRU_EDGE (save_bb));
-	      tree new_var_phi = PHI_RESULT (phi);
-
-	      free_dominance_info (CDI_DOMINATORS);
-	      calculate_dominance_info (CDI_DOMINATORS);
-
-	      tree stmt2;
-	      imm_use_iterator iter;
-	      use_operand_p use_p;
-	      FOR_EACH_IMM_USE_STMT (stmt2, iter, var)
-		{
-		  if (stmt2 == phi)
-		    continue;
-
-		  basic_block tmp_bb = bb_for_stmt (stmt2);
-		  if (dominated_by_p (CDI_DOMINATORS, tmp_bb, begin_bb))
+		  unsigned ix = PHI_ARG_INDEX_FROM_USE (use_p);
+		  if (gimple_phi_arg_edge (phi2, ix)->flags & EDGE_ABNORMAL)
 		    {
-		      FOR_EACH_IMM_USE_ON_STMT (use_p, iter)
-			propagate_value (use_p, new_var_phi);
+		      in_abnormal_phi = true;
+		      break;
 		    }
 		}
 	    }
+	  SSA_NAME_OCCURS_IN_ABNORMAL_PHI (arg) = in_abnormal_phi;
 	}
-      if (dump_file)
-	fprintf (dump_file, "\n");
 
+      replace_uses_by (new_ssa, old_ssa);
+      remove_phi_node (&gsi, true);
     }
-  update_ssa(TODO_update_ssa);
-
-  return ;
 }
 
-/* Implements the checkpointing of transactions. */
 static void
-checkpoint_tm_txn (struct tm_region *region)
+expand_tm_atomic (basic_block bb, gimple_stmt_iterator *gsi)
 {
-  basic_block entry_bb = bb_for_stmt (region->setjmp_stmt);
-
-  edge branch = BRANCH_EDGE (entry_bb);
-  edge fall = FALLTHRU_EDGE (entry_bb);
+  tree status, tm_start;
+  basic_block body_bb, test_bb;
+  gimple_stmt_iterator gsi2;
+  tree t1, t2;
+  gimple g;
+  edge e;
+
+  tm_start = built_in_decls[BUILT_IN_TM_START];
+  status = make_rename_temp (TREE_TYPE (TREE_TYPE (tm_start)), "tm_state");
+
+  e = FALLTHRU_EDGE (bb);
+  body_bb = e->dest;
+  checkpoint_live_in_variables (e);
 
-  basic_block begin_bb = fall->dest;
-  basic_block recover_bb = branch->dest;
-  basic_block next_bb = single_succ (recover_bb);
-
-  gcc_assert(begin_bb == next_bb);
-  block_stmt_iterator bsi_recover = bsi_start (recover_bb);
-  gcc_assert (TREE_CODE (bsi_stmt (bsi_recover)) == LABEL_EXPR);
-
-  bsi_next (&bsi_recover);
-
-  checkpoint_live_in_variables (region, &bsi_recover, begin_bb);
-}
-
-/* Walk the region tree and start checkpointing. */
-static void
-checkpoint_tm (struct tm_region *region)
-{
-  while (region)
+  if (gimple_tm_atomic_label (gsi_stmt (*gsi)))
     {
-      /* First, introduce checkpoints for the inner regions.
-	 TODO: testing. Overlapping of inner and outer
-	 regions not handled correctly.
-	 Nesting of transactions not implemented correctly.*/
-      if (region->inner)
-	{
-	  checkpoint_tm_txn (region->inner);
-	}
-
-      checkpoint_tm_txn (region);
-
-      region = region->next;
+      e = split_block_after_labels (body_bb);
+      test_bb = e->src;
+      body_bb = e->dest;
+
+      gsi2 = gsi_last_bb (test_bb);
+
+      t1 = make_rename_temp (TREE_TYPE (status), NULL);
+      t2 = build_int_cst (TREE_TYPE (status), TM_START_ABORT);
+      g = gimple_build_assign_with_ops (BIT_AND_EXPR, t1, status, t2);
+      gsi_insert_after (&gsi2, g, GSI_CONTINUE_LINKING);
+
+      t2 = build_int_cst (TREE_TYPE (status), 0);
+      g = gimple_build_cond (NE_EXPR, t1, t2, NULL, NULL);
+      gsi_insert_after (&gsi2, g, GSI_CONTINUE_LINKING);
+
+      single_succ_edge (test_bb)->flags = EDGE_FALSE_VALUE;
+
+      e = BRANCH_EDGE (bb);
+      redirect_edge_pred (e, test_bb);
+      e->flags = EDGE_TRUE_VALUE;
     }
+
+  /* ??? Need to put the real input to __tm_start here.  */
+  t2 = build_int_cst (TREE_TYPE (status), 0);
+  g = gimple_build_call (tm_start, 1, t2);
+  gimple_call_set_lhs (g, status);
+  gsi_insert_before (gsi, g, GSI_SAME_STMT);
+  gsi_remove (gsi, true);
 }
 
 /* Entry point to the checkpointing. */
-void
+
+static unsigned int
 execute_checkpoint_tm (void)
 {
-  /* Regions are built during TM expansion pass. */
-  if (!root_tm_region)
-    return;
-
-  /* Checkpointing is done here. */
-  checkpoint_tm (root_tm_region);
+  basic_block bb;
 
-  if (dump_file)
+  FOR_EACH_BB (bb)
     {
-      fprintf (dump_file, "\nTM region tree after checkpointing\n\n");
-      dump_tm_region (dump_file, root_tm_region, 0);
-      fprintf (dump_file, "\n");
+      gimple_stmt_iterator gsi = gsi_last_bb (bb);
+      if (!gsi_end_p (gsi)
+	  && gimple_code (gsi_stmt (gsi)) == GIMPLE_TM_ATOMIC)
+	expand_tm_atomic (bb, &gsi);
     }
 
-  free_dominance_info (CDI_DOMINATORS);
-  cleanup_tree_cfg ();
-  free_tm_regions ();
-
-  return;
+  return 0;
 }
 
-struct tree_opt_pass pass_checkpoint_tm =
+struct gimple_opt_pass pass_checkpoint_tm =
 {
+ {
+  GIMPLE_PASS,
   "tmcheckpoint",			/* name */
   gate_tm,				/* gate */
   execute_checkpoint_tm,		/* execute */
@@ -451,6 +654,84 @@ struct tree_opt_pass pass_checkpoint_tm 
   TODO_update_ssa |
   TODO_verify_ssa |
   TODO_dump_func,			/* todo_flags_finish */
-  0					/* letter */
+ }
 };
-#endif
+\f
+/* Construct transaction restart edges for STMT.  */
+
+static void
+make_tm_edge_1 (struct eh_region *region, void *data)
+{
+  gimple stmt = (gimple) data;
+  basic_block src, dst;
+  unsigned flags;
+
+  src = gimple_bb (stmt);
+  dst = label_to_block (get_eh_region_tree_label (region));
+
+  /* Don't set EDGE_EH here, because that's supposed to be used when
+     we could in principal redirect the edge by modifying the exception
+     tables.  Transactions don't use tables though, only setjmp.  */
+  flags = EDGE_ABNORMAL;
+  if (gimple_code (stmt) == GIMPLE_CALL)
+    flags |= EDGE_ABNORMAL_CALL;
+  make_edge (src, dst, flags);
+}
+
+void
+make_tm_edge (gimple stmt)
+{
+  int region_nr;
+
+  /* Do nothing if the region is outside this function.  */
+  region_nr = lookup_stmt_eh_region (stmt);
+  if (region_nr < 0)
+    return;
+
+  /* The control structure inside tree-cfg.c isn't the best;
+     re-check whether this is actually a transactional stmt.  */
+  if (!is_transactional_stmt (stmt))
+    return;
+
+  foreach_reachable_transaction (region_nr, make_tm_edge_1, (void *) stmt);
+}
+
+/* Return true if STMT may alter control flow via a transactional edge.  */
+
+bool
+is_transactional_stmt (const_gimple stmt)
+{
+  switch (gimple_code (stmt))
+    {
+    case GIMPLE_ASSIGN:
+      {
+	/* We only want to process assignments that have been
+	   marked for transactional memory.  */
+	enum tree_code subcode = gimple_expr_code (stmt);
+	return (subcode == TM_LOAD || subcode == TM_STORE);
+      }
+
+    case GIMPLE_CALL:
+      {
+	tree fn_decl = gimple_call_fndecl (stmt);
+
+	/* We only want to process __tm_abort and cloned direct calls,
+	   since those are the only ones that can restart the transaction.  */
+	if (!fn_decl)
+	  return false;
+	if (DECL_BUILT_IN_CLASS (fn_decl) == BUILT_IN_NORMAL
+	    && DECL_FUNCTION_CODE (fn_decl) == BUILT_IN_TM_ABORT)
+	  return true;
+	if (DECL_IS_TM_CLONE (fn_decl))
+	  return true;
+	else
+	  return false;
+      }
+
+    case GIMPLE_TM_ATOMIC:
+      return true;
+
+    default:
+      return false;
+    }
+}
Index: gimple-low.c
===================================================================
--- gimple-low.c	(revision 141199)
+++ gimple-low.c	(revision 141200)
@@ -77,16 +77,12 @@ struct lower_data
 
   /* True if the function calls __builtin_setjmp.  */
   bool calls_builtin_setjmp;
-
-  /* True if we're lowering inside a transaction.  */
-  bool in_transaction;
 };
 
 static void lower_stmt (gimple_stmt_iterator *, struct lower_data *);
 static void lower_gimple_bind (gimple_stmt_iterator *, struct lower_data *);
 static void lower_gimple_return (gimple_stmt_iterator *, struct lower_data *);
 static void lower_builtin_setjmp (gimple_stmt_iterator *);
-static void record_vars_into_tm (tree, tree, bool);
 
 
 /* Lower the body of current_function_decl from High GIMPLE into Low
@@ -236,31 +232,6 @@ lower_sequence (gimple_seq seq, struct l
 }
 
 
-/* Lower the __tm_atomic statement pointed by TSI.  DATA is
-   passed through the recursion.  */
-
-static void
-lower_tm_atomic (gimple_stmt_iterator *gsi, struct lower_data *data)
-{
-  bool old_in_transaction = data->in_transaction;
-  gimple stmt = gsi_stmt (*gsi);
-  tree label = create_artificial_label ();
-
-  data->in_transaction = true;
-
-  lower_sequence (gimple_seq_body (stmt), data);
-
-  gsi_insert_before (gsi, stmt, GSI_SAME_STMT);
-  gsi_insert_seq_before (gsi, gimple_seq_body (stmt), GSI_SAME_STMT);
-  gsi_insert_before (gsi, gimple_build_label (label), GSI_SAME_STMT);
-
-  gimple_seq_set_body (stmt, NULL);
-  gimple_tm_atomic_set_label (stmt, label);
-  gsi_remove (gsi, false);
-
-  data->in_transaction = old_in_transaction;
-}
-
 /* Lower the OpenMP directive statement pointed by GSI.  DATA is
    passed through the recursion.  */
 
@@ -358,8 +329,8 @@ lower_stmt (gimple_stmt_iterator *gsi, s
       return;
 
     case GIMPLE_TM_ATOMIC:
-      lower_tm_atomic (gsi, data);
-      return;
+      lower_sequence (gimple_seq_body (stmt), data);
+      break;
 
     default:
       gcc_unreachable ();
@@ -405,8 +376,7 @@ lower_gimple_bind (gimple_stmt_iterator 
 	}
     }
 
-  record_vars_into_tm (gimple_bind_vars (stmt), current_function_decl,
-		       data->in_transaction);
+  record_vars (gimple_bind_vars (stmt));
   lower_sequence (gimple_bind_body (stmt), data);
 
   if (new_block)
@@ -820,11 +790,10 @@ lower_builtin_setjmp (gimple_stmt_iterat
 }
 \f
 
-/* Record the variables in VARS into function FN.  If IN_TRANSACTION is
-   true, mark them DECL_IS_TM_PURE_VAR.  */
+/* Record the variables in VARS into function FN.  */
 
-static void
-record_vars_into_tm (tree vars, tree fn, bool in_transaction)
+void
+record_vars_into (tree vars, tree fn)
 {
   if (fn != current_function_decl)
     push_cfun (DECL_STRUCT_FUNCTION (fn));
@@ -843,36 +812,21 @@ record_vars_into_tm (tree vars, tree fn,
 	continue;
 
       /* Record the variable.  */
-      cfun->local_decls = tree_cons (NULL_TREE, var,
-					     cfun->local_decls);
-
-      /* If we're inside a transaction, mark it for NOT checkpointing.  */
-      if (in_transaction)
-	DECL_IS_TM_PURE_VAR (var) = 1;
+      cfun->local_decls = tree_cons (NULL_TREE, var, cfun->local_decls);
     }
 
   if (fn != current_function_decl)
     pop_cfun ();
 }
 
-/* Record the variables in VARS into function FN.  */
-
-void
-record_vars_into (tree vars, tree fn)
-{
-  record_vars_into_tm (vars, fn, false);
-}
-
-
 /* Record the variables in VARS into current_function_decl.  */
 
 void
 record_vars (tree vars)
 {
-  record_vars_into_tm (vars, current_function_decl, false);
+  record_vars_into (vars, current_function_decl);
 }
 
-
 /* Mark BLOCK used if it has a used variable in it, then recurse over its
    subblocks.  */
 
Index: tree-eh.c
===================================================================
--- tree-eh.c	(revision 141199)
+++ tree-eh.c	(revision 141200)
@@ -310,6 +310,10 @@ collect_finally_tree (gimple stmt, gimpl
       collect_finally_tree_1 (gimple_eh_filter_failure (stmt), region);
       break;
 
+    case GIMPLE_TM_ATOMIC:
+      collect_finally_tree_1 (gimple_seq_body (stmt), region);
+      break;
+
     default:
       /* A type, a decl, or some kind of statement that we're not
 	 interested in.  Don't walk them.  */
@@ -1869,6 +1873,21 @@ lower_eh_constructs_2 (struct leh_state 
       /* Return since we don't want gsi_next () */
       return;
 
+    case GIMPLE_TM_ATOMIC:
+      {
+        /* Record the transaction region in the EH tree, then process
+	   the body of the transaction.  We don't lower the transaction
+	   node just yet.  */
+	struct eh_region *outer = state->cur_region;
+	state->cur_region = gen_eh_region_transaction (outer);
+
+	record_stmt_eh_region (state->cur_region, stmt);
+	lower_eh_constructs_1 (state, gimple_seq_body (stmt));
+
+	state->cur_region = outer;
+      }
+      break;
+
     default:
       /* A type, a decl, or some kind of statement that we're not
 	 interested in.  Don't walk them.  */
@@ -2043,6 +2062,9 @@ verify_eh_edges (gimple stmt)
 	}
       if (!stmt_could_throw_p (stmt))
 	{
+	  /* ??? Add bits to verify transactional edges.  */
+	  if (is_transactional_stmt (stmt))
+	    return false;
 	  error ("BB %i last statement has incorrectly set region", bb->index);
 	  return true;
 	}
Index: gimple-pretty-print.c
===================================================================
--- gimple-pretty-print.c	(revision 141199)
+++ gimple-pretty-print.c	(revision 141200)
@@ -161,6 +161,7 @@ debug_gimple_seq (gimple_seq seq)
      'd' - outputs an int as a decimal,
      's' - outputs a string,
      'n' - outputs a newline,
+     'x' - outputs an int as hexadecimal,
      '+' - increases indent by 2 then outputs a newline,
      '-' - decreases indent by 2 then outputs a newline.   */
 
@@ -215,6 +216,10 @@ dump_gimple_fmt (pretty_printer *buffer,
                 newline_and_indent (buffer, spc);
                 break;
 
+              case 'x':
+                pp_scalar (buffer, "%x", va_arg (args, int));
+                break;
+
               case '+':
                 spc += 2;
                 newline_and_indent (buffer, spc);
@@ -350,9 +355,19 @@ dump_binary_rhs (pretty_printer *buffer,
 static void
 dump_gimple_assign (pretty_printer *buffer, gimple gs, int spc, int flags)
 {
+  enum tree_code code;
+
+  /* Don't bypass the transactional markers like
+     gimple_assign_rhs_code would.  */
+  code = gimple_expr_code (gs);
+  if (code != TM_LOAD && code != TM_STORE
+      && get_gimple_rhs_class (code) == GIMPLE_SINGLE_RHS)
+    code = TREE_CODE (gimple_assign_rhs1 (gs));
+
   if (flags & TDF_RAW)
     {
       tree last;
+
       if (gimple_num_ops (gs) == 2)
         last = NULL_TREE;
       else if (gimple_num_ops (gs) == 3)
@@ -361,8 +376,8 @@ dump_gimple_assign (pretty_printer *buff
         gcc_unreachable ();
 
       dump_gimple_fmt (buffer, spc, flags, "%G <%s, %T, %T, %T>", gs,
-                       tree_code_name[gimple_assign_rhs_code (gs)],
-                       gimple_assign_lhs (gs), gimple_assign_rhs1 (gs), last);
+                       tree_code_name[code], gimple_assign_lhs (gs),
+		       gimple_assign_rhs1 (gs), last);
     }
   else
     {
@@ -372,6 +387,11 @@ dump_gimple_assign (pretty_printer *buff
 	  pp_space (buffer);
 	  pp_character (buffer, '=');
 
+	  if (code == TM_LOAD)
+	    pp_string (buffer, "{tm_load}");
+	  else if (code == TM_STORE)
+	    pp_string (buffer, "{tm_store}");
+
 	  if (gimple_assign_nontemporal_move_p (gs))
 	    pp_string (buffer, "{nt}");
 
@@ -1013,11 +1033,18 @@ static void
 dump_gimple_tm_atomic (pretty_printer *buffer, gimple gs, int spc, int flags)
 {
   if (flags & TDF_RAW)
-    dump_gimple_fmt (buffer, spc, flags, "%G <%+BODY <%S> >", gs,
-		     gimple_seq_body (gs));
+    dump_gimple_fmt (buffer, spc, flags,
+		     "%G [SUBCODE=%x,LABEL=%T] <%+BODY <%S> >",
+		     gs, gimple_tm_atomic_subcode (gs),
+		     gimple_tm_atomic_label (gs), gimple_seq_body (gs));
   else
     {
-      pp_string (buffer, "__tm_atomic");
+      pp_printf (buffer, "__tm_atomic [SUBCODE=%x,LABEL=",
+		 gimple_tm_atomic_subcode (gs));
+      dump_generic_node (buffer, gimple_tm_atomic_label (gs),
+			 spc, flags, false);
+      pp_string (buffer, "]");
+
       if (!gimple_seq_empty_p (gimple_seq_body (gs)))
 	{
 	  newline_and_indent (buffer, spc + 2);
Index: except.c
===================================================================
--- except.c	(revision 141199)
+++ except.c	(revision 141200)
@@ -140,7 +140,8 @@ struct eh_region GTY(())
     ERT_CATCH,
     ERT_ALLOWED_EXCEPTIONS,
     ERT_MUST_NOT_THROW,
-    ERT_THROW
+    ERT_THROW,
+    ERT_TRANSACTION
   } type;
 
   /* Holds the action to perform based on the preceding type.  */
@@ -178,6 +179,11 @@ struct eh_region GTY(())
     struct eh_region_u_cleanup {
       struct eh_region *prev_try;
     } GTY ((tag ("ERT_CLEANUP"))) cleanup;
+
+    /* ??? Nothing for now.  */
+    struct eh_region_u_transaction {
+      int dummy;
+    } GTY ((tag ("ERT_TRANSACTION"))) transaction;
   } GTY ((desc ("%0.type"))) u;
 
   /* Entry point for this region's handler before landing pads are built.  */
@@ -253,7 +259,6 @@ static hashval_t ehl_hash (const void *)
 static int ehl_eq (const void *, const void *);
 static void add_ehl_entry (rtx, struct eh_region *);
 static void remove_exception_handler_label (rtx);
-static void remove_eh_handler (struct eh_region *);
 static int for_each_eh_label_1 (void **, void *);
 
 /* The return value of reachable_next_level.  */
@@ -422,7 +427,7 @@ gen_eh_region (enum eh_region_type type,
   struct eh_region *new_eh;
 
 #ifdef ENABLE_CHECKING
-  gcc_assert (doing_eh (0));
+  gcc_assert (flag_tm || doing_eh (0));
 #endif
 
   /* Insert a new blank region as a leaf in the tree.  */
@@ -509,12 +514,24 @@ gen_eh_region_must_not_throw (struct eh_
   return gen_eh_region (ERT_MUST_NOT_THROW, outer);
 }
 
+struct eh_region *
+gen_eh_region_transaction (struct eh_region *outer)
+{
+  return gen_eh_region (ERT_TRANSACTION, outer);
+}
+
 int
 get_eh_region_number (struct eh_region *region)
 {
   return region->region_number;
 }
 
+struct eh_region *
+get_eh_region_from_number (int region_nr)
+{
+  return VEC_index (eh_region, cfun->eh->region_array, region_nr);
+}
+
 bool
 get_eh_region_may_contain_throw (struct eh_region *region)
 {
@@ -808,7 +825,8 @@ current_function_has_exception_handlers 
       region = VEC_index (eh_region, cfun->eh->region_array, i);
       if (region
 	  && region->region_number == i
-	  && region->type != ERT_THROW)
+	  && region->type != ERT_THROW
+	  && region->type != ERT_TRANSACTION)
 	return true;
     }
 
@@ -1473,6 +1491,7 @@ build_post_landing_pads (void)
 
 	case ERT_CATCH:
 	case ERT_THROW:
+	case ERT_TRANSACTION:
 	  /* Nothing to do.  */
 	  break;
 
@@ -2142,7 +2161,7 @@ remove_exception_handler_label (rtx labe
 
 /* Splice REGION from the region tree etc.  */
 
-static void
+void
 remove_eh_handler (struct eh_region *region)
 {
   struct eh_region **pp, **pp_start, *p, *outer, *inner;
@@ -2524,6 +2543,11 @@ reachable_next_level (struct eh_region *
       else
 	return RNL_BLOCKED;
 
+    case ERT_TRANSACTION:
+      /* Transaction regions don't catch exceptions, they catch
+	 transaction restarts.  */
+      return RNL_NOT_CAUGHT;
+
     case ERT_THROW:
     case ERT_UNKNOWN:
       /* Shouldn't see these here.  */
@@ -2538,8 +2562,7 @@ reachable_next_level (struct eh_region *
 
 void
 foreach_reachable_handler (int region_number, bool is_resx,
-			   void (*callback) (struct eh_region *, void *),
-			   void *callback_data)
+			   eh_callback callback, void *callback_data)
 {
   struct reachable_info info;
   struct eh_region *region;
@@ -2581,6 +2604,26 @@ foreach_reachable_handler (int region_nu
     }
 }
 
+/* Invoke CALLBACK for each TRANSACTION region inside REGION_NUMBER.  */
+
+void
+foreach_reachable_transaction (int region_number,
+			       eh_callback callback, void *callback_data)
+{
+  struct eh_region *region;
+
+  region = VEC_index (eh_region, cfun->eh->region_array, region_number);
+  while (region)
+    {
+      if (region->type == ERT_TRANSACTION)
+	{
+	  callback (region, callback_data);
+	  break;
+	}
+      region = region->outer;
+    }
+}
+
 /* Retrieve a list of labels of exception handlers which can be
    reached by a given insn.  */
 
@@ -3177,8 +3220,10 @@ collect_one_action_chain (htab_t ar_hash
 
     case ERT_CATCH:
     case ERT_THROW:
+    case ERT_TRANSACTION:
       /* CATCH regions are handled in TRY above.  THROW regions are
-	 for optimization information only and produce no output.  */
+	 for optimization information only and produce no output.
+	 TRANSACTION regions produce no output as well.  */
       return collect_one_action_chain (ar_hash, region->outer);
 
     default:
Index: except.h
===================================================================
--- except.h	(revision 141199)
+++ except.h	(revision 141200)
@@ -92,14 +92,16 @@ extern struct eh_region *gen_eh_region_t
 extern struct eh_region *gen_eh_region_catch (struct eh_region *, tree);
 extern struct eh_region *gen_eh_region_allowed (struct eh_region *, tree);
 extern struct eh_region *gen_eh_region_must_not_throw (struct eh_region *);
+extern struct eh_region *gen_eh_region_transaction (struct eh_region *);
 extern int get_eh_region_number (struct eh_region *);
+extern struct eh_region *get_eh_region_from_number (int);
 extern bool get_eh_region_may_contain_throw (struct eh_region *);
 extern tree get_eh_region_tree_label (struct eh_region *);
 extern void set_eh_region_tree_label (struct eh_region *, tree);
 
-extern void foreach_reachable_handler (int, bool,
-				       void (*) (struct eh_region *, void *),
-				       void *);
+typedef void (*eh_callback) (struct eh_region *, void *);
+extern void foreach_reachable_handler (int, bool, eh_callback, void *);
+extern void foreach_reachable_transaction (int, eh_callback, void *);
 
 extern void collect_eh_region_array (void);
 extern void expand_resx_expr (tree);
@@ -107,6 +109,7 @@ extern void verify_eh_tree (struct funct
 extern void dump_eh_tree (FILE *, struct function *);
 extern bool eh_region_outer_p (struct function *, int, int);
 extern int eh_region_outermost (struct function *, int, int);
+extern void remove_eh_handler (struct eh_region *);
 
 /* If non-NULL, this is a function that returns an expression to be
    executed if an unhandled exception is propagated out of a cleanup
Index: ChangeLog.tm
===================================================================
--- ChangeLog.tm	(revision 141199)
+++ ChangeLog.tm	(revision 141200)
@@ -1,3 +1,51 @@
+2008-10-17  Richard Henderson  <rth@redhat.com>
+
+	* except.c (struct eh_region): Add ERT_TRANSACTION.
+	(gen_eh_region): Allow if flag_tm.
+	(gen_eh_region_transaction, get_eh_region_from_number): New.
+	(remove_eh_handler): Export.
+	(current_function_has_exception_handlers): Handle ERT_TRANSACTION.
+	(build_post_landing_pads, reachable_next_level): Likewise.
+	(collect_one_action_chain): Likewise.
+	(foreach_reachable_transaction): New.
+	* except.h: Add new exported decls.
+	* gimple-low.c (struct lower_data): Remove in_transaction.
+	(lower_tm_atomic, record_vars_into_tm): Remove.
+	* gimple-pretty-print.c (dump_gimple_fmt): Add %x.
+	(dump_gimple_assign): Handle TM_LOAD/STORE.
+	(dump_gimple_tm_atomic): Dump the subcode.
+	* gimple.h (GTMA_HAVE_ABORT, GTMA_HAVE_LOAD, GTMA_HAVE_STORE,
+	GTMA_HAVE_CALL_TM, GTMA_HAVE_CALL_IRREVOKABLE, 
+	GTMA_MUST_CALL_IRREVOKABLE, GTMA_HAVE_CALL_INDIRECT): New.
+	(gimple_tm_atomic_subcode, gimple_tm_atomic_set_subcode): New.
+	* gtm-low.c (struct ltm_state, add_stmt_to_transaction,
+	lower_assign_tm, lower_call_tm, remove_tm_commits,
+	lower_tm_atomic, lower_sequence_tm, lower_sequence_no_tm): New.
+	(execute_lower_tm): Use them.
+	(TM_START_RESTORE_LIVE_IN, TM_START_ABORT): New.
+	(checkpoint_live_in_variables): Rewrite.
+	(checkpoint_tm_txn, checkpoint_tm): Remove.
+	(expand_tm_atomic): New.
+	(execute_checkpoint_tm): Use it.
+	(make_tm_edge_1, make_tm_edge, is_transactional_stmt): New.
+	(pass_lower_tm): Rename from pass_expand_tm.
+	* passes.c (init_optimization_passes): Run pass_lower_tm
+	immediately after pass_lower_eh.  Run pass_checkpoint_tm
+	after early optimizations.
+	* tree-cfg.c (make_edges): Call make_tm_edge.  Conditionally
+	create the __tm_atomic abort edge.
+	(cleanup_dead_labels): Handle GIMPLE_TM_ATOMIC.  Avoid unnecessary
+	writes into the statements to update labels.
+	(is_ctrl_altering_stmt): Include is_transactional_stmt.
+	(verify_stmt): Handle transactional edges.
+	* tree-eh.c (collect_finally_tree): Walk GIMPLE_TM_ATOMIC.
+	(lower_eh_constructs_2): Create EH regions for them.
+	(verify_eh_edges): Handle transactional edges.
+	* tree-flow.h (make_tm_edge, is_transactional_stmt): Declare.
+
+	* c-parser.c (c_parser_tm_abort): Call add_stmt.
+	* cgraphbuild.c (prepare_tm_clone): Disable for now.
+
 2008-10-15  Richard Henderson  <rth@redhat.com>
 
 	* builtin-attrs.def (ATTR_RETURNS_TWICE): New.
Index: tree-flow.h
===================================================================
--- tree-flow.h	(revision 141199)
+++ tree-flow.h	(revision 141200)
@@ -1079,6 +1079,9 @@ extern int lookup_expr_eh_region (tree);
 extern int lookup_stmt_eh_region (gimple);
 extern bool verify_eh_edges (gimple);
 
+/* In gtm-low.c  */
+extern void make_tm_edge (gimple);
+extern bool is_transactional_stmt (const_gimple);
 
 /* In tree-ssa-pre.c  */
 struct pre_expr_d;
Index: gimple.h
===================================================================
--- gimple.h	(revision 141199)
+++ gimple.h	(revision 141200)
@@ -735,6 +735,15 @@ struct gimple_statement_omp_atomic_store
 
 /* GIMPLE_TM_ATOMIC.  */
 
+/* Bits to be stored in the GIMPLE_TM_ATOMIC subcode.  */
+#define GTMA_HAVE_ABORT			(1u << 0)
+#define GTMA_HAVE_LOAD			(1u << 1)
+#define GTMA_HAVE_STORE			(1u << 2)
+#define GTMA_HAVE_CALL_TM		(1u << 3)
+#define GTMA_HAVE_CALL_IRREVOKABLE	(1u << 4)
+#define GTMA_MUST_CALL_IRREVOKABLE	(1u << 5)
+#define GTMA_HAVE_CALL_INDIRECT		(1u << 6)
+
 struct gimple_statement_tm_atomic GTY(())
 {
   /* [ WORD 1-5 ]  */
@@ -4134,6 +4143,15 @@ gimple_tm_atomic_label (const_gimple gs)
   return gs->gimple_tm_atomic.label;
 }
 
+/* Return the subcode associated with a GIMPLE_TM_ATOMIC.  */
+
+static inline unsigned int
+gimple_tm_atomic_subcode (const_gimple gs)
+{
+  GIMPLE_CHECK (gs, GIMPLE_TM_ATOMIC);
+  return gs->gsbase.subcode;
+}
+
 /* Set the label associated with a GIMPLE_TM_ATOMIC.  */
 
 static inline void
@@ -4143,6 +4161,16 @@ gimple_tm_atomic_set_label (gimple gs, t
   gs->gimple_tm_atomic.label = label;
 }
 
+/* Set the subcode associated with a GIMPLE_TM_ATOMIC.  */
+
+static inline void
+gimple_tm_atomic_set_subcode (gimple gs, unsigned int subcode)
+{
+  GIMPLE_CHECK (gs, GIMPLE_TM_ATOMIC);
+  gs->gsbase.subcode = subcode;
+}
+
+
 /* Return a pointer to the return value for GIMPLE_RETURN GS.  */
 
 static inline tree *
Index: tree-cfg.c
===================================================================
--- tree-cfg.c	(revision 141199)
+++ tree-cfg.c	(revision 141200)
@@ -514,6 +514,10 @@ make_edges (void)
 		 create abnormal edges to them.  */
 	      make_eh_edges (last);
 
+	      /* If this statement calls a transaction clone,
+		 add transactional restart edges.  */
+	      make_tm_edge (last);
+
 	      /* Some calls are known not to return.  */
 	      fallthru = !(gimple_call_flags (last) & ECF_NORETURN);
 	      break;
@@ -524,6 +528,7 @@ make_edges (void)
 	      if (is_ctrl_altering_stmt (last))
 		{
 		  make_eh_edges (last);
+		  make_tm_edge (last);
 		}
 	      fallthru = true;
 	      break;
@@ -613,11 +618,12 @@ make_edges (void)
 	      break;
 
 	    case GIMPLE_TM_ATOMIC:
-	      /* ??? The edge from __tm_atomic to the out-label is only
-		 used when __tm_abort is present.  Detect that's not
-		 present and omit it.  */
-	      make_edge (bb, label_to_block (gimple_tm_atomic_label (last)), 0);
-	      fallthru = true;
+	      {
+		tree abort_label = gimple_tm_atomic_label (last);
+		if (abort_label)
+		  make_edge (bb, label_to_block (abort_label), 0);
+		fallthru = true;
+	      }
 	      break;
 
 	    default:
@@ -988,22 +994,30 @@ cleanup_dead_labels (void)
   FOR_EACH_BB (bb)
     {
       gimple stmt = last_stmt (bb);
+      tree label, new_label;
+
       if (!stmt)
 	continue;
 
       switch (gimple_code (stmt))
 	{
 	case GIMPLE_COND:
-	  {
-	    tree true_label = gimple_cond_true_label (stmt);
-	    tree false_label = gimple_cond_false_label (stmt);
+	  label = gimple_cond_true_label (stmt);
+	  if (label)
+	    {
+	      new_label = main_block_label (label);
+	      if (new_label != label)
+		gimple_cond_set_true_label (stmt, new_label);
+	    }
 
-	    if (true_label)
-	      gimple_cond_set_true_label (stmt, main_block_label (true_label));
-	    if (false_label)
-	      gimple_cond_set_false_label (stmt, main_block_label (false_label));
-	    break;
-	  }
+	  label = gimple_cond_false_label (stmt);
+	  if (label)
+	    {
+	      new_label = main_block_label (label);
+	      if (new_label != label)
+		gimple_cond_set_false_label (stmt, new_label);
+	    }
+	  break;
 
 	case GIMPLE_SWITCH:
 	  {
@@ -1013,8 +1027,10 @@ cleanup_dead_labels (void)
 	    for (i = 0; i < n; ++i)
 	      {
 		tree case_label = gimple_switch_label (stmt, i);
-		tree label = main_block_label (CASE_LABEL (case_label));
-		CASE_LABEL (case_label) = label;
+		label = CASE_LABEL (case_label);
+		new_label = main_block_label (label);
+		if (new_label != label)
+		  CASE_LABEL (case_label) = label;
 	      }
 	    break;
 	  }
@@ -1024,10 +1040,24 @@ cleanup_dead_labels (void)
 	case GIMPLE_GOTO:
           if (!computed_goto_p (stmt))
 	    {
-	      tree new_dest = main_block_label (gimple_goto_dest (stmt));
-	      gimple_goto_set_dest (stmt, new_dest);
-	      break;
+	      label = gimple_goto_dest (stmt);
+	      new_label = main_block_label (label);
+	      if (new_label != label)
+		gimple_goto_set_dest (stmt, new_label);
 	    }
+	  break;
+
+	case GIMPLE_TM_ATOMIC:
+	  {
+	    tree label = gimple_tm_atomic_label (stmt);
+	    if (label)
+	      {
+		tree new_label = main_block_label (label);
+		if (new_label != label)
+		  gimple_tm_atomic_set_label (stmt, new_label);
+	      }
+	  }
+	  break;
 
 	default:
 	  break;
@@ -2578,12 +2608,14 @@ is_ctrl_altering_stmt (gimple t)
   if (is_gimple_omp (t))
     return true;
 
-  /* __tm_atomic can alter control flow.  */
-  if (gimple_code (t) == GIMPLE_TM_ATOMIC)
+  /* If a statement can throw, it alters control flow.  */
+  if (stmt_can_throw_internal (t))
     return true;
 
-  /* If a statement can throw, it alters control flow.  */
-  return stmt_can_throw_internal (t);
+  if (flag_tm && is_transactional_stmt (t))
+    return true;
+
+  return false;
 }
 
 
@@ -4085,12 +4117,15 @@ verify_stmt (gimple_stmt_iterator *gsi)
      to match.  */
   if (lookup_stmt_eh_region (stmt) >= 0)
     {
-      if (!stmt_could_throw_p (stmt))
+      bool is_tm = is_transactional_stmt (stmt);
+
+      if (!is_tm && !stmt_could_throw_p (stmt))
 	{
 	  error ("statement marked for throw, but doesn%'t");
 	  goto fail;
 	}
-      if (!last_in_block && stmt_can_throw_internal (stmt))
+      /* ??? Add a transactional function akin to stmt_can_throw_internal.  */
+      if (!last_in_block && !is_tm && stmt_can_throw_internal (stmt))
 	{
 	  error ("statement marked for throw in middle of block");
 	  goto fail;
Index: passes.c
===================================================================
--- passes.c	(revision 141199)
+++ passes.c	(revision 141200)
@@ -515,6 +515,7 @@ init_optimization_passes (void)
   NEXT_PASS (pass_lower_cf);
   NEXT_PASS (pass_refactor_eh);
   NEXT_PASS (pass_lower_eh);
+  NEXT_PASS (pass_lower_tm);
   NEXT_PASS (pass_build_cfg);
   NEXT_PASS (pass_lower_complex_O0);
   NEXT_PASS (pass_lower_vector);
@@ -540,13 +541,11 @@ init_optimization_passes (void)
       NEXT_PASS (pass_cleanup_cfg);
       NEXT_PASS (pass_init_datastructures);
       NEXT_PASS (pass_expand_omp);
-      NEXT_PASS (pass_expand_tm);
 
       NEXT_PASS (pass_referenced_vars);
       NEXT_PASS (pass_reset_cc_flags);
       NEXT_PASS (pass_build_ssa);
       NEXT_PASS (pass_early_warn_uninitialized);
-      /* NEXT_PASS (pass_checkpoint_tm); */
       NEXT_PASS (pass_all_early_optimizations);
 	{
 	  struct opt_pass **p = &pass_all_early_optimizations.pass.sub;
@@ -565,6 +564,7 @@ init_optimization_passes (void)
 	  NEXT_PASS (pass_convert_switch);
           NEXT_PASS (pass_profile);
 	}
+      NEXT_PASS (pass_checkpoint_tm);
       NEXT_PASS (pass_release_ssa_names);
       NEXT_PASS (pass_rebuild_cgraph_edges);
       NEXT_PASS (pass_inline_parameters);
Index: c-parser.c
===================================================================
--- c-parser.c	(revision 141199)
+++ c-parser.c	(revision 141200)
@@ -8255,7 +8255,7 @@ c_parser_tm_abort (c_parser *parser)
   /* ??? Verify that __tm_abort is contained within the
      lexical scope of a __tm_atomic.  */
 
-  return build_call_expr (built_in_decls[BUILT_IN_TM_ABORT], 0);
+  return add_stmt (build_call_expr (built_in_decls[BUILT_IN_TM_ABORT], 0));
 }
 \f
 /* Parse a single source file.  */

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [patch] 19/n: trans-mem: middle end/misc patches (LAST PATCH)
  2011-11-06 20:47     ` Richard Henderson
  2011-11-06 22:01       ` Aldy Hernandez
  2011-11-07  4:25       ` Aldy Hernandez
@ 2011-11-07  7:53       ` Aldy Hernandez
  2011-11-07  9:56         ` Richard Guenther
  2 siblings, 1 reply; 20+ messages in thread
From: Aldy Hernandez @ 2011-11-07  7:53 UTC (permalink / raw)
  To: Richard Henderson; +Cc: Richard Guenther, gcc-patches


> False.  You get the equivalent of bootstrap comparison mismatches.
> If we actually used tm during the bootstrap.
>
> The simplest thing to do is to change the hash this table uses.
> E.g. use the DECL_UID right from the start, rather than the pointer.

Woah!  Can it be that easy?  That's as easy as changing the hash, no 
conversion necessary.

OK for branch?

	* varasm.c (record_tm_clone_pair): Use DECL_UID as hash.
	(get_tm_clone_pair): Same.

Index: varasm.c
===================================================================
--- varasm.c	(revision 181067)
+++ varasm.c	(working copy)
@@ -5875,7 +5875,7 @@ record_tm_clone_pair (tree o, tree n)
      tm_clone_pairs = htab_create_ggc (32, tree_map_hash, tree_map_eq, 0);

    h = ggc_alloc_tree_map ();
-  h->hash = htab_hash_pointer (o);
+  h->hash = DECL_UID (o);
    h->base.from = o;
    h->to = n;

@@ -5892,7 +5892,7 @@ get_tm_clone_pair (tree o)
        struct tree_map *h, in;

        in.base.from = o;
-      in.hash = htab_hash_pointer (o);
+      in.hash = DECL_UID (o);
        h = (struct tree_map *) htab_find_with_hash (tm_clone_pairs,
  						   &in, in.hash);
        if (h)

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [patch] 19/n: trans-mem: middle end/misc patches (LAST PATCH)
  2011-11-06 19:07   ` Aldy Hernandez
  2011-11-06 20:47     ` Richard Henderson
  2011-11-06 21:50     ` Richard Henderson
@ 2011-11-07  9:48     ` Richard Guenther
  2011-11-07 16:08       ` Aldy Hernandez
                         ` (2 more replies)
  2 siblings, 3 replies; 20+ messages in thread
From: Richard Guenther @ 2011-11-07  9:48 UTC (permalink / raw)
  To: Aldy Hernandez; +Cc: gcc-patches, Richard Henderson

On Sun, Nov 6, 2011 at 7:53 PM, Aldy Hernandez <aldyh@redhat.com> wrote:
> [rth, more comments for you below]
>
> On 11/04/11 04:14, Richard Guenther wrote:
>
>>>    new_version = cgraph_create_node (new_decl);
>>>
>>> -   new_version->analyzed = true;
>>> +   new_version->analyzed = old_version->analyzed;
>>
>> Hm?  analyzed means "with body", sure you have a body if you clone.
>>
>>>    new_version->local = old_version->local;
>>>    new_version->local.externally_visible = false;
>>>    new_version->local.local = true;
>>> @@ -2294,6 +2294,7 @@ cgraph_copy_node_for_versioning (struct
>>>    new_version->rtl = old_version->rtl;
>>>    new_version->reachable = true;
>>>    new_version->count = old_version->count;
>>> +   new_version->lowered = true;
>>
>> OTOH this isn't necessary true.  cgraph exists before lowering.
>
> I don't understand what you want me to do on either of these two comments.
>  Could you elaborate?

Do not push new_version->lowered setting into the core routine but keep
it at the callers.

>>> +  /* TM pure functions should not get inlined if the outer function is
>>> +     a TM safe function.  */
>>> +  else if (flag_tm
>>
>> Please move flag checks into the respective prediates.  Any reason
>> why the is_tm_pure () predicate wouldn't already do the correct thing
>> with !flag_tm?
>
> Done.
>
>>> +  /* Map gimple stmt to tree label (or list of labels) for transaction
>>> +     restart and abort.  */
>>> +  htab_t GTY ((param_is (struct tm_restart_node))) tm_restart;
>>> +
>>
>> As this maps 'gimple' to tree shouldn't this go to fn->gimple_df instead?
>> That way you avoid growing generic struct function.  Or in to eh_status,
>> if that looks like a better fit.
>
> Done.
>
>>> +  /* Mark all calls that can have a transaction restart.  */
>>
>> Why isn't this done when we expand the call?  This walking of the
>> RTL sequence looks like a hack (an easy one, albeit).
>>
>>> +  if (cfun->tm_restart&&  is_gimple_call (stmt))
>>> +    {
>>> +      struct tm_restart_node dummy;
>>> +      void **slot;
>>> +
>>> +      dummy.stmt = stmt;
>>> +      slot = htab_find_slot (cfun->tm_restart,&dummy, NO_INSERT);
>>> +      if (slot)
>>> +       {
>>> +         struct tm_restart_node *n = (struct tm_restart_node *) *slot;
>>> +         tree list = n->label_or_list;
>>> +         rtx insn;
>>> +
>>> +         for (insn = next_real_insn (last); !CALL_P (insn);
>>> +              insn = next_real_insn (insn))
>>> +           continue;
>>> +
>>> +         if (TREE_CODE (list) == LABEL_DECL)
>>> +           add_reg_note (insn, REG_TM, label_rtx (list));
>>> +         else
>>> +           for (; list ; list = TREE_CHAIN (list))
>>> +             add_reg_note (insn, REG_TM, label_rtx (TREE_VALUE (list)));
>>> +       }
>>> +    }
>
> I can certainly move this to expand_call_stmt() if you prefer.  Do you have
> an objection to the RTL walk?  This isn't my code, but I'm open to
> suggestions on an alternative to implement.

It just catched my eye...  moving it to expand_call_stmt would be nice
indeed, but I was suggesting to add that note where we produce the
CALL rtx, not sure if that's reasonably straight-forward (I suppose there
was a reason to go with the hack above ...).

Richard.

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [patch] 19/n: trans-mem: middle end/misc patches (LAST PATCH)
  2011-11-07  7:53       ` Aldy Hernandez
@ 2011-11-07  9:56         ` Richard Guenther
  2011-11-07 15:54           ` Aldy Hernandez
  0 siblings, 1 reply; 20+ messages in thread
From: Richard Guenther @ 2011-11-07  9:56 UTC (permalink / raw)
  To: Aldy Hernandez; +Cc: Richard Henderson, gcc-patches

On Mon, Nov 7, 2011 at 6:55 AM, Aldy Hernandez <aldyh@redhat.com> wrote:
>
>> False.  You get the equivalent of bootstrap comparison mismatches.
>> If we actually used tm during the bootstrap.
>>
>> The simplest thing to do is to change the hash this table uses.
>> E.g. use the DECL_UID right from the start, rather than the pointer.
>
> Woah!  Can it be that easy?  That's as easy as changing the hash, no
> conversion necessary.
>
> OK for branch?

This won't work - DECL_UIDs are not stable -g vs. -g0 - only their
_order_ is stable - thus you won't get comparison fails with code generated
dependent on DECL_UID order, but you will if you depend on the DECL_UID
value (which you do by using it as a hash).

And we will still generate different object files based on garbage collector
settings this way - GC can shrink hashtables, causing re-hashing,
which changes the order of the elements.  It also causes re-ordering
with slight unrelated code changes (but if you say at runtime we always
sort the thing that might not be an issue).

Thus, the patch isn't a fix to get a stable order (you can't get
that for hashtable walks).  A quick fix is to collect the elements
into a VEC and qsort that after some stable key (like the DECL_UID).

Thanks,
Richard.

>        * varasm.c (record_tm_clone_pair): Use DECL_UID as hash.
>        (get_tm_clone_pair): Same.
>
> Index: varasm.c
> ===================================================================
> --- varasm.c    (revision 181067)
> +++ varasm.c    (working copy)
> @@ -5875,7 +5875,7 @@ record_tm_clone_pair (tree o, tree n)
>     tm_clone_pairs = htab_create_ggc (32, tree_map_hash, tree_map_eq, 0);
>
>   h = ggc_alloc_tree_map ();
> -  h->hash = htab_hash_pointer (o);
> +  h->hash = DECL_UID (o);
>   h->base.from = o;
>   h->to = n;
>
> @@ -5892,7 +5892,7 @@ get_tm_clone_pair (tree o)
>       struct tree_map *h, in;
>
>       in.base.from = o;
> -      in.hash = htab_hash_pointer (o);
> +      in.hash = DECL_UID (o);
>       h = (struct tree_map *) htab_find_with_hash (tm_clone_pairs,
>                                                   &in, in.hash);
>       if (h)
>

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [patch] 19/n: trans-mem: middle end/misc patches (LAST PATCH)
  2011-11-07  9:56         ` Richard Guenther
@ 2011-11-07 15:54           ` Aldy Hernandez
  2011-11-07 16:08             ` Richard Guenther
  0 siblings, 1 reply; 20+ messages in thread
From: Aldy Hernandez @ 2011-11-07 15:54 UTC (permalink / raw)
  To: Richard Guenther; +Cc: Richard Henderson, gcc-patches

[-- Attachment #1: Type: text/plain, Size: 1251 bytes --]


> This won't work - DECL_UIDs are not stable -g vs. -g0 - only their
> _order_ is stable - thus you won't get comparison fails with code generated
> dependent on DECL_UID order, but you will if you depend on the DECL_UID
> value (which you do by using it as a hash).
>
> And we will still generate different object files based on garbage collector
> settings this way - GC can shrink hashtables, causing re-hashing,
> which changes the order of the elements.  It also causes re-ordering
> with slight unrelated code changes (but if you say at runtime we always
> sort the thing that might not be an issue).

Ah, I get it.

> Thus, the patch isn't a fix to get a stable order (you can't get
> that for hashtable walks).  A quick fix is to collect the elements
> into a VEC and qsort that after some stable key (like the DECL_UID).

I've done so in the attached patch.

Notice I am hijacking the alias_pair object, because it has everything 
we need, though the DECL_UID goes in a field called emitted_diags.  I am 
avoiding having to create an exact same object with the exact same 
fields.  I can change duplicate this, if it's a big eye sore.

Preliminary tests show the TM tests all work, but a full test run is 
still underway.  OK pending tests?

[-- Attachment #2: curr --]
[-- Type: text/plain, Size: 6070 bytes --]

Index: ChangeLog.tm-merge
===================================================================
--- ChangeLog.tm-merge	(revision 181067)
+++ ChangeLog.tm-merge	(working copy)
@@ -120,7 +120,7 @@
 	(GTFILES): Add trans-mem.c.
 	* omp-low.c (WALK_SUBSTMTS): Add GIMPLE_TRANSACTION.
 	* output.h (record_tm_clone_pair, finish_tm_clone_pairs,
-	finish_tm_clone_pairs_1, get_tm_clone_pair): Declare.
+	get_tm_clone_pair): Declare.
 	* params.def (PARAM_TM_MAX_AGGREGATE_SIZE): New.
 	* passes.c (init_optimization_passes): Place transactional memory
 	passes.
@@ -190,5 +190,5 @@
 	is_tm_may_cancel_outer, is_tm_ending_fndecl, record_tm_replacement,
 	tm_malloc_replacement): Declare.
 	* varasm.c (tm_clone_pairs): New.
-	(record_tm_clone_pair, finish_tm_clone_pairs, finish_tm_clone_pairs_1,
-	get_tm_clone_pair): New.
+	(record_tm_clone_pair, finish_tm_clone_pairs, get_tm_clone_pair,
+	dump_tm_clone_to_vec, dump_tm_clone_pairs, alias_pair_cmp): New.
Index: varasm.c
===================================================================
--- varasm.c	(revision 181067)
+++ varasm.c	(working copy)
@@ -5901,58 +5901,104 @@ get_tm_clone_pair (tree o)
   return NULL_TREE;
 }
 
-/* Helper function for finish_tm_clone_pairs.  Dump the clone table.  */
+/* Helper function for finish_tm_clone_pairs.  Dump a hash table entry
+   into a VEC in INFO.  */
 
-int
-finish_tm_clone_pairs_1 (void **slot, void *info ATTRIBUTE_UNUSED)
+static int
+dump_tm_clone_to_vec (void **slot, void *info)
 {
   struct tree_map *map = (struct tree_map *) *slot;
-  bool *switched = (bool *) info;
-  tree src = map->base.from;
-  tree dst = map->to;
-  struct cgraph_node *src_n = cgraph_get_node (src);
-  struct cgraph_node *dst_n = cgraph_get_node (dst);
-
-  /* The function ipa_tm_create_version() marks the clone as needed if
-     the original function was needed.  But we also mark the clone as
-     needed if we ever called the clone indirectly through
-     TM_GETTMCLONE.  If neither of these are true, we didn't generate
-     a clone, and we didn't call it indirectly... no sense keeping it
-     in the clone table.  */
-  if (!dst_n || !dst_n->needed)
-    return 1;
+  VEC(alias_pair,gc) **alias_pairs = (VEC(alias_pair, gc) **) info;
+  alias_pair *p = VEC_safe_push (alias_pair, gc, *alias_pairs, NULL);
+  p->decl = map->base.from;
+  p->target = map->to;
+  p->emitted_diags = DECL_UID (p->decl);
+  return 1;
+}
 
-  /* This covers the case where we have optimized the original
-     function away, and only access the transactional clone.  */
-  if (!src_n || !src_n->needed)
-    return 1;
+/* Dump the actual pairs to the .tm_clone_table section.  */
 
-  if (!*switched)
+static void
+dump_tm_clone_pairs (VEC(alias_pair,gc) *alias_pairs)
+{
+  unsigned i;
+  alias_pair *p;
+  bool switched = false;
+
+  FOR_EACH_VEC_ELT (alias_pair, alias_pairs, i, p)
     {
-      switch_to_section (get_named_section (NULL, ".tm_clone_table", 3));
-      assemble_align (POINTER_SIZE);
-      *switched = true;
+      tree src = p->decl;
+      tree dst = p->target;
+      struct cgraph_node *src_n = cgraph_get_node (src);
+      struct cgraph_node *dst_n = cgraph_get_node (dst);
+
+      /* The function ipa_tm_create_version() marks the clone as needed if
+	 the original function was needed.  But we also mark the clone as
+	 needed if we ever called the clone indirectly through
+	 TM_GETTMCLONE.  If neither of these are true, we didn't generate
+	 a clone, and we didn't call it indirectly... no sense keeping it
+	 in the clone table.  */
+      if (!dst_n || !dst_n->needed)
+	continue;
+
+      /* This covers the case where we have optimized the original
+	 function away, and only access the transactional clone.  */
+      if (!src_n || !src_n->needed)
+	continue;
+
+      if (!switched)
+	{
+	  switch_to_section (get_named_section (NULL, ".tm_clone_table", 3));
+	  assemble_align (POINTER_SIZE);
+	  switched = true;
+	}
+
+      assemble_integer (XEXP (DECL_RTL (src), 0),
+			POINTER_SIZE / BITS_PER_UNIT, POINTER_SIZE, 1);
+      assemble_integer (XEXP (DECL_RTL (dst), 0),
+			POINTER_SIZE / BITS_PER_UNIT, POINTER_SIZE, 1);
     }
+}
 
-  assemble_integer (XEXP (DECL_RTL (src), 0),
-		    POINTER_SIZE / BITS_PER_UNIT, POINTER_SIZE, 1);
-  assemble_integer (XEXP (DECL_RTL (dst), 0),
-		    POINTER_SIZE / BITS_PER_UNIT, POINTER_SIZE, 1);
-  return 1;
+/* Helper comparison function for qsorting by the DECL_UID stored in
+   alias_pair->emitted_diags.  */
+
+static int
+alias_pair_cmp (const void *x, const void *y)
+{
+  const alias_pair *p1 = (const alias_pair *) x;
+  const alias_pair *p2 = (const alias_pair *) y;
+  if (p1->emitted_diags < p2->emitted_diags)
+    return -1;
+  if (p1->emitted_diags > p2->emitted_diags)
+    return 1;
+  return 0;
 }
 
 void
 finish_tm_clone_pairs (void)
 {
-  bool switched = false;
+  VEC(alias_pair,gc) *alias_pairs = NULL;
 
   if (tm_clone_pairs == NULL)
     return;
 
-  htab_traverse_noresize (tm_clone_pairs, finish_tm_clone_pairs_1,
-			  (void *) &switched);
+  /* We need a determenistic order for the .tm_clone_table, otherwise
+     we will get bootstrap comparison failures, so dump the hash table
+     to a vector, sort it, and dump the vector.  */
+
+  /* Dump the hashtable to a vector.  */
+  htab_traverse_noresize (tm_clone_pairs, dump_tm_clone_to_vec,
+			  (void *) &alias_pairs);
+  /* Sort it.  */
+  VEC_qsort (alias_pair, alias_pairs, alias_pair_cmp);
+
+  /* Dump it.  */
+  dump_tm_clone_pairs (alias_pairs);
+
   htab_delete (tm_clone_pairs);
   tm_clone_pairs = NULL;
+  VEC_free (alias_pair, gc, alias_pairs);
 }
 
 
Index: output.h
===================================================================
--- output.h	(revision 181067)
+++ output.h	(working copy)
@@ -608,7 +608,6 @@ extern void output_section_asm_op (const
 
 extern void record_tm_clone_pair (tree, tree);
 extern void finish_tm_clone_pairs (void);
-extern int finish_tm_clone_pairs_1 (void **, void *);
 extern tree get_tm_clone_pair (tree);
 
 extern void default_asm_output_source_filename (FILE *, const char *);

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [patch] 19/n: trans-mem: middle end/misc patches (LAST PATCH)
  2011-11-07 15:54           ` Aldy Hernandez
@ 2011-11-07 16:08             ` Richard Guenther
  2011-11-07 17:12               ` Aldy Hernandez
  0 siblings, 1 reply; 20+ messages in thread
From: Richard Guenther @ 2011-11-07 16:08 UTC (permalink / raw)
  To: Aldy Hernandez; +Cc: Richard Henderson, gcc-patches

On Mon, Nov 7, 2011 at 4:52 PM, Aldy Hernandez <aldyh@redhat.com> wrote
>
>> This won't work - DECL_UIDs are not stable -g vs. -g0 - only their
>> _order_ is stable - thus you won't get comparison fails with code
>> generated
>> dependent on DECL_UID order, but you will if you depend on the DECL_UID
>> value (which you do by using it as a hash).
>>
>> And we will still generate different object files based on garbage
>> collector
>> settings this way - GC can shrink hashtables, causing re-hashing,
>> which changes the order of the elements.  It also causes re-ordering
>> with slight unrelated code changes (but if you say at runtime we always
>> sort the thing that might not be an issue).
>
> Ah, I get it.
>
>> Thus, the patch isn't a fix to get a stable order (you can't get
>> that for hashtable walks).  A quick fix is to collect the elements
>> into a VEC and qsort that after some stable key (like the DECL_UID).
>
> I've done so in the attached patch.
>
> Notice I am hijacking the alias_pair object, because it has everything we
> need, though the DECL_UID goes in a field called emitted_diags.  I am
> avoiding having to create an exact same object with the exact same fields.
>  I can change duplicate this, if it's a big eye sore.

Yes, please duplicate this - alias_pairs are going away (hopefully for 4.7).
And in that light, simply use a heap allocated vector - it's
short-lived after all.

> Preliminary tests show the TM tests all work, but a full test run is still
> underway.  OK pending tests?

Otherwise ok.

Thanks,
Richard.

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [patch] 19/n: trans-mem: middle end/misc patches (LAST PATCH)
  2011-11-07  9:48     ` Richard Guenther
@ 2011-11-07 16:08       ` Aldy Hernandez
  2011-11-07 16:14         ` Richard Henderson
  2011-11-07 16:11       ` Richard Henderson
  2011-11-07 17:45       ` Aldy Hernandez
  2 siblings, 1 reply; 20+ messages in thread
From: Aldy Hernandez @ 2011-11-07 16:08 UTC (permalink / raw)
  To: Richard Guenther; +Cc: gcc-patches, Richard Henderson

[-- Attachment #1: Type: text/plain, Size: 490 bytes --]


>> I can certainly move this to expand_call_stmt() if you prefer.  Do you have
>> an objection to the RTL walk?  This isn't my code, but I'm open to
>> suggestions on an alternative to implement.
>
> It just catched my eye...  moving it to expand_call_stmt would be nice
> indeed, but I was suggesting to add that note where we produce the
> CALL rtx, not sure if that's reasonably straight-forward (I suppose there
> was a reason to go with the hack above ...).

Sure.

OK pending tests?

[-- Attachment #2: tm-restart --]
[-- Type: text/plain, Size: 2945 bytes --]

Index: ChangeLog.tm-merge
===================================================================
--- ChangeLog.tm-merge	(revision 181067)
+++ ChangeLog.tm-merge	(working copy)
@@ -58,8 +58,10 @@
 	* calls.c (is_tm_builtin): New.
 	(flags_from_decl_or_type): Add ECF_TM_OPS for TM clones.
 	* cfgbuild.c (make_edges): Add edges for REG_TM notes.
-	* cfgexpand.c (expand_gimple_stmt): Add REG_TM notes.
+	* cfgexpand.c (expand_call_stmt): Call
+	mark_transaction_restart_calls.
 	(gimple_expand_cfg): Free the tm_restart map.
+	(mark_transaction_restart_calls): New.
 	* cfgrtl.c (purge_dead_edges): Look for REG_TM notes.
 	* cgraph.c (dump_cgraph_node): Handle tm_clone.
 	* cgraph.h (struct cgraph_node): Add tm_clone field.
Index: cfgexpand.c
===================================================================
--- cfgexpand.c	(revision 181067)
+++ cfgexpand.c	(working copy)
@@ -1802,6 +1802,38 @@ expand_gimple_cond (basic_block bb, gimp
   return new_bb;
 }
 
+/* Mark all calls that can have a transaction restart.  */
+
+static void
+mark_transaction_restart_calls (gimple stmt)
+{
+  struct tm_restart_node dummy;
+  void **slot;
+
+  if (!cfun->gimple_df->tm_restart)
+    return;
+
+  dummy.stmt = stmt;
+  slot = htab_find_slot (cfun->gimple_df->tm_restart, &dummy, NO_INSERT);
+  if (slot)
+    {
+      struct tm_restart_node *n = (struct tm_restart_node *) *slot;
+      tree list = n->label_or_list;
+      rtx insn;
+
+      for (insn = next_real_insn (get_last_insn ());
+	   !CALL_P (insn);
+	   insn = next_real_insn (insn))
+	continue;
+
+      if (TREE_CODE (list) == LABEL_DECL)
+	add_reg_note (insn, REG_TM, label_rtx (list));
+      else
+	for (; list ; list = TREE_CHAIN (list))
+	  add_reg_note (insn, REG_TM, label_rtx (TREE_VALUE (list)));
+    }
+}
+
 /* A subroutine of expand_gimple_stmt_1, expanding one GIMPLE_CALL
    statement STMT.  */
 
@@ -1812,6 +1844,8 @@ expand_call_stmt (gimple stmt)
   bool builtin_p;
   size_t i;
 
+  mark_transaction_restart_calls (stmt);
+
   if (gimple_call_internal_p (stmt))
     {
       expand_internal_call (stmt);
@@ -2096,32 +2130,6 @@ expand_gimple_stmt (gimple stmt)
 	}
     }
 
-  /* Mark all calls that can have a transaction restart.  */
-  if (cfun->gimple_df->tm_restart && is_gimple_call (stmt))
-    {
-      struct tm_restart_node dummy;
-      void **slot;
-
-      dummy.stmt = stmt;
-      slot = htab_find_slot (cfun->gimple_df->tm_restart, &dummy, NO_INSERT);
-      if (slot)
-	{
-	  struct tm_restart_node *n = (struct tm_restart_node *) *slot;
-	  tree list = n->label_or_list;
-	  rtx insn;
-
-	  for (insn = next_real_insn (last); !CALL_P (insn);
-	       insn = next_real_insn (insn))
-	    continue;
-
-	  if (TREE_CODE (list) == LABEL_DECL)
-	    add_reg_note (insn, REG_TM, label_rtx (list));
-	  else
-	    for (; list ; list = TREE_CHAIN (list))
-	      add_reg_note (insn, REG_TM, label_rtx (TREE_VALUE (list)));
-	}
-    }
-
   return last;
 }
 

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [patch] 19/n: trans-mem: middle end/misc patches (LAST PATCH)
  2011-11-07  9:48     ` Richard Guenther
  2011-11-07 16:08       ` Aldy Hernandez
@ 2011-11-07 16:11       ` Richard Henderson
  2011-11-07 17:45       ` Aldy Hernandez
  2 siblings, 0 replies; 20+ messages in thread
From: Richard Henderson @ 2011-11-07 16:11 UTC (permalink / raw)
  To: Richard Guenther; +Cc: Aldy Hernandez, gcc-patches

On 11/07/2011 01:44 AM, Richard Guenther wrote:
> It just catched my eye...  moving it to expand_call_stmt would be nice
> indeed, but I was suggesting to add that note where we produce the
> CALL rtx, not sure if that's reasonably straight-forward (I suppose there
> was a reason to go with the hack above ...).

Because targets emit all sorts of stuff in gen_call that isn't a CALL_INSN.
We have to search anyway.  And the guts of expand_call are complex enough
already; no need to make it worse.


r~

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [patch] 19/n: trans-mem: middle end/misc patches (LAST PATCH)
  2011-11-07 16:08       ` Aldy Hernandez
@ 2011-11-07 16:14         ` Richard Henderson
  2011-11-07 16:23           ` Aldy Hernandez
  0 siblings, 1 reply; 20+ messages in thread
From: Richard Henderson @ 2011-11-07 16:14 UTC (permalink / raw)
  To: Aldy Hernandez; +Cc: Richard Guenther, gcc-patches

On 11/07/2011 08:04 AM, Aldy Hernandez wrote:
> @@ -1812,6 +1844,8 @@ expand_call_stmt (gimple stmt)
>    bool builtin_p;
>    size_t i;
>  
> +  mark_transaction_restart_calls (stmt);
> +
>    if (gimple_call_internal_p (stmt))
>      {
>        expand_internal_call (stmt);

You're calling it too early, Aldy.  The call you're searching
for hasn't been generated yet.


r~

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [patch] 19/n: trans-mem: middle end/misc patches (LAST PATCH)
  2011-11-07 16:14         ` Richard Henderson
@ 2011-11-07 16:23           ` Aldy Hernandez
  2011-11-07 16:32             ` Richard Henderson
  0 siblings, 1 reply; 20+ messages in thread
From: Aldy Hernandez @ 2011-11-07 16:23 UTC (permalink / raw)
  To: Richard Henderson; +Cc: Richard Guenther, gcc-patches

[-- Attachment #1: Type: text/plain, Size: 725 bytes --]

On 11/07/11 08:08, Richard Henderson wrote:
> On 11/07/2011 08:04 AM, Aldy Hernandez wrote:
>> @@ -1812,6 +1844,8 @@ expand_call_stmt (gimple stmt)
>>     bool builtin_p;
>>     size_t i;
>>
>> +  mark_transaction_restart_calls (stmt);
>> +
>>     if (gimple_call_internal_p (stmt))
>>       {
>>         expand_internal_call (stmt);
>
> You're calling it too early, Aldy.  The call you're searching
> for hasn't been generated yet.

*blush*

Whoops! And that's what happens when you post patches with "OK pending 
tests".  Sorry, my machines are chugging along and I'm queuing up 
patches for testing... Trying to take advantage of Richi's time zone...

TM tests all pass.  Further tests are still going.

OK pending tests?

[-- Attachment #2: tm-restart --]
[-- Type: text/plain, Size: 3030 bytes --]

Index: ChangeLog.tm-merge
===================================================================
--- ChangeLog.tm-merge	(revision 181067)
+++ ChangeLog.tm-merge	(working copy)
@@ -58,8 +58,10 @@
 	* calls.c (is_tm_builtin): New.
 	(flags_from_decl_or_type): Add ECF_TM_OPS for TM clones.
 	* cfgbuild.c (make_edges): Add edges for REG_TM notes.
-	* cfgexpand.c (expand_gimple_stmt): Add REG_TM notes.
+	* cfgexpand.c (expand_call_stmt): Call
+	mark_transaction_restart_calls.
 	(gimple_expand_cfg): Free the tm_restart map.
+	(mark_transaction_restart_calls): New.
 	* cfgrtl.c (purge_dead_edges): Look for REG_TM notes.
 	* cgraph.c (dump_cgraph_node): Handle tm_clone.
 	* cgraph.h (struct cgraph_node): Add tm_clone field.
Index: cfgexpand.c
===================================================================
--- cfgexpand.c	(revision 181067)
+++ cfgexpand.c	(working copy)
@@ -1802,6 +1802,38 @@ expand_gimple_cond (basic_block bb, gimp
   return new_bb;
 }
 
+/* Mark all calls that can have a transaction restart.  */
+
+static void
+mark_transaction_restart_calls (gimple stmt)
+{
+  struct tm_restart_node dummy;
+  void **slot;
+
+  if (!cfun->gimple_df->tm_restart)
+    return;
+
+  dummy.stmt = stmt;
+  slot = htab_find_slot (cfun->gimple_df->tm_restart, &dummy, NO_INSERT);
+  if (slot)
+    {
+      struct tm_restart_node *n = (struct tm_restart_node *) *slot;
+      tree list = n->label_or_list;
+      rtx insn;
+
+      for (insn = next_real_insn (get_last_insn ());
+	   !CALL_P (insn);
+	   insn = next_real_insn (insn))
+	continue;
+
+      if (TREE_CODE (list) == LABEL_DECL)
+	add_reg_note (insn, REG_TM, label_rtx (list));
+      else
+	for (; list ; list = TREE_CHAIN (list))
+	  add_reg_note (insn, REG_TM, label_rtx (TREE_VALUE (list)));
+    }
+}
+
 /* A subroutine of expand_gimple_stmt_1, expanding one GIMPLE_CALL
    statement STMT.  */
 
@@ -1888,6 +1920,8 @@ expand_call_stmt (gimple stmt)
     expand_assignment (lhs, exp, false);
   else
     expand_expr_real_1 (exp, const0_rtx, VOIDmode, EXPAND_NORMAL, NULL);
+
+  mark_transaction_restart_calls (stmt);
 }
 
 /* A subroutine of expand_gimple_stmt, expanding one gimple statement
@@ -2096,32 +2130,6 @@ expand_gimple_stmt (gimple stmt)
 	}
     }
 
-  /* Mark all calls that can have a transaction restart.  */
-  if (cfun->gimple_df->tm_restart && is_gimple_call (stmt))
-    {
-      struct tm_restart_node dummy;
-      void **slot;
-
-      dummy.stmt = stmt;
-      slot = htab_find_slot (cfun->gimple_df->tm_restart, &dummy, NO_INSERT);
-      if (slot)
-	{
-	  struct tm_restart_node *n = (struct tm_restart_node *) *slot;
-	  tree list = n->label_or_list;
-	  rtx insn;
-
-	  for (insn = next_real_insn (last); !CALL_P (insn);
-	       insn = next_real_insn (insn))
-	    continue;
-
-	  if (TREE_CODE (list) == LABEL_DECL)
-	    add_reg_note (insn, REG_TM, label_rtx (list));
-	  else
-	    for (; list ; list = TREE_CHAIN (list))
-	      add_reg_note (insn, REG_TM, label_rtx (TREE_VALUE (list)));
-	}
-    }
-
   return last;
 }
 

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [patch] 19/n: trans-mem: middle end/misc patches (LAST PATCH)
  2011-11-07 16:23           ` Aldy Hernandez
@ 2011-11-07 16:32             ` Richard Henderson
  0 siblings, 0 replies; 20+ messages in thread
From: Richard Henderson @ 2011-11-07 16:32 UTC (permalink / raw)
  To: Aldy Hernandez; +Cc: Richard Guenther, gcc-patches

On 11/07/2011 08:14 AM, Aldy Hernandez wrote:
> TM tests all pass.  Further tests are still going.
> 
> OK pending tests?

Much better.  Ok.


r~

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [patch] 19/n: trans-mem: middle end/misc patches (LAST PATCH)
  2011-11-07 16:08             ` Richard Guenther
@ 2011-11-07 17:12               ` Aldy Hernandez
  0 siblings, 0 replies; 20+ messages in thread
From: Aldy Hernandez @ 2011-11-07 17:12 UTC (permalink / raw)
  To: Richard Guenther; +Cc: Richard Henderson, gcc-patches

[-- Attachment #1: Type: text/plain, Size: 707 bytes --]


>> Notice I am hijacking the alias_pair object, because it has everything we
>> need, though the DECL_UID goes in a field called emitted_diags.  I am
>> avoiding having to create an exact same object with the exact same fields.
>>   I can change duplicate this, if it's a big eye sore.
>
> Yes, please duplicate this - alias_pairs are going away (hopefully for 4.7).
> And in that light, simply use a heap allocated vector - it's
> short-lived after all.
>
>
> Otherwise ok.

I have made the changes you suggested, and I also renamed tm_clone_pairs 
to tm_clone_hash to avoid confusion with the vector table.  We have too 
many tm_clone_* tables :).

Are you still OK with this patch? (Tests still going).

[-- Attachment #2: tm-clone-table --]
[-- Type: text/plain, Size: 7754 bytes --]

Index: ChangeLog.tm-merge
===================================================================
--- ChangeLog.tm-merge	(revision 181067)
+++ ChangeLog.tm-merge	(working copy)
@@ -120,7 +120,7 @@
 	(GTFILES): Add trans-mem.c.
 	* omp-low.c (WALK_SUBSTMTS): Add GIMPLE_TRANSACTION.
 	* output.h (record_tm_clone_pair, finish_tm_clone_pairs,
-	finish_tm_clone_pairs_1, get_tm_clone_pair): Declare.
+	get_tm_clone_pair): Declare.
 	* params.def (PARAM_TM_MAX_AGGREGATE_SIZE): New.
 	* passes.c (init_optimization_passes): Place transactional memory
 	passes.
@@ -189,6 +189,7 @@
 	(build_tm_abort_call, is_tm_safe, is_tm_pure,
 	is_tm_may_cancel_outer, is_tm_ending_fndecl, record_tm_replacement,
 	tm_malloc_replacement): Declare.
-	* varasm.c (tm_clone_pairs): New.
-	(record_tm_clone_pair, finish_tm_clone_pairs, finish_tm_clone_pairs_1,
-	get_tm_clone_pair): New.
+	* varasm.c (tm_clone_hash): New.
+	(record_tm_clone_pair, finish_tm_clone_pairs, get_tm_clone_pair,
+	dump_tm_clone_to_vec, dump_tm_clone_pairs, tm_alias_pair_cmp): New.
+	(struct tm_alias_pair): New.  Declare VEC types for object.
Index: varasm.c
===================================================================
--- varasm.c	(revision 181028)
+++ varasm.c	(working copy)
@@ -5864,15 +5864,15 @@ assemble_alias (tree decl, tree target)
    considered to be their own clone.  */
 
 static GTY((if_marked ("tree_map_marked_p"), param_is (struct tree_map)))
-     htab_t tm_clone_pairs;
+     htab_t tm_clone_hash;
 
 void
 record_tm_clone_pair (tree o, tree n)
 {
   struct tree_map **slot, *h;
 
-  if (tm_clone_pairs == NULL)
-    tm_clone_pairs = htab_create_ggc (32, tree_map_hash, tree_map_eq, 0);
+  if (tm_clone_hash == NULL)
+    tm_clone_hash = htab_create_ggc (32, tree_map_hash, tree_map_eq, 0);
 
   h = ggc_alloc_tree_map ();
   h->hash = htab_hash_pointer (o);
@@ -5880,20 +5880,20 @@ record_tm_clone_pair (tree o, tree n)
   h->to = n;
 
   slot = (struct tree_map **)
-    htab_find_slot_with_hash (tm_clone_pairs, h, h->hash, INSERT);
+    htab_find_slot_with_hash (tm_clone_hash, h, h->hash, INSERT);
   *slot = h;
 }
 
 tree
 get_tm_clone_pair (tree o)
 {
-  if (tm_clone_pairs)
+  if (tm_clone_hash)
     {
       struct tree_map *h, in;
 
       in.base.from = o;
       in.hash = htab_hash_pointer (o);
-      h = (struct tree_map *) htab_find_with_hash (tm_clone_pairs,
+      h = (struct tree_map *) htab_find_with_hash (tm_clone_hash,
 						   &in, in.hash);
       if (h)
 	return h->to;
@@ -5901,58 +5901,117 @@ get_tm_clone_pair (tree o)
   return NULL_TREE;
 }
 
-/* Helper function for finish_tm_clone_pairs.  Dump the clone table.  */
+typedef struct tm_alias_pair
+{
+  unsigned int uid;
+  tree from;
+  tree to;
+} tm_alias_pair;
 
-int
-finish_tm_clone_pairs_1 (void **slot, void *info ATTRIBUTE_UNUSED)
+DEF_VEC_O(tm_alias_pair);
+DEF_VEC_ALLOC_O(tm_alias_pair,heap);
+
+/* Helper function for finish_tm_clone_pairs.  Dump a hash table entry
+   into a VEC in INFO.  */
+
+static int
+dump_tm_clone_to_vec (void **slot, void *info)
 {
   struct tree_map *map = (struct tree_map *) *slot;
-  bool *switched = (bool *) info;
-  tree src = map->base.from;
-  tree dst = map->to;
-  struct cgraph_node *src_n = cgraph_get_node (src);
-  struct cgraph_node *dst_n = cgraph_get_node (dst);
-
-  /* The function ipa_tm_create_version() marks the clone as needed if
-     the original function was needed.  But we also mark the clone as
-     needed if we ever called the clone indirectly through
-     TM_GETTMCLONE.  If neither of these are true, we didn't generate
-     a clone, and we didn't call it indirectly... no sense keeping it
-     in the clone table.  */
-  if (!dst_n || !dst_n->needed)
-    return 1;
+  VEC(tm_alias_pair,heap) **tm_alias_pairs
+    = (VEC(tm_alias_pair, heap) **) info;
+  tm_alias_pair *p;
+
+  p = VEC_safe_push (tm_alias_pair, heap, *tm_alias_pairs, NULL);
+  p->from = map->base.from;
+  p->to = map->to;
+  p->uid = DECL_UID (p->from);
+  return 1;
+}
 
-  /* This covers the case where we have optimized the original
-     function away, and only access the transactional clone.  */
-  if (!src_n || !src_n->needed)
-    return 1;
+/* Dump the actual pairs to the .tm_clone_table section.  */
+
+static void
+dump_tm_clone_pairs (VEC(tm_alias_pair,heap) *tm_alias_pairs)
+{
+  unsigned i;
+  tm_alias_pair *p;
+  bool switched = false;
 
-  if (!*switched)
+  FOR_EACH_VEC_ELT (tm_alias_pair, tm_alias_pairs, i, p)
     {
-      switch_to_section (get_named_section (NULL, ".tm_clone_table", 3));
-      assemble_align (POINTER_SIZE);
-      *switched = true;
+      tree src = p->from;
+      tree dst = p->to;
+      struct cgraph_node *src_n = cgraph_get_node (src);
+      struct cgraph_node *dst_n = cgraph_get_node (dst);
+
+      /* The function ipa_tm_create_version() marks the clone as needed if
+	 the original function was needed.  But we also mark the clone as
+	 needed if we ever called the clone indirectly through
+	 TM_GETTMCLONE.  If neither of these are true, we didn't generate
+	 a clone, and we didn't call it indirectly... no sense keeping it
+	 in the clone table.  */
+      if (!dst_n || !dst_n->needed)
+	continue;
+
+      /* This covers the case where we have optimized the original
+	 function away, and only access the transactional clone.  */
+      if (!src_n || !src_n->needed)
+	continue;
+
+      if (!switched)
+	{
+	  switch_to_section (get_named_section (NULL, ".tm_clone_table", 3));
+	  assemble_align (POINTER_SIZE);
+	  switched = true;
+	}
+
+      assemble_integer (XEXP (DECL_RTL (src), 0),
+			POINTER_SIZE / BITS_PER_UNIT, POINTER_SIZE, 1);
+      assemble_integer (XEXP (DECL_RTL (dst), 0),
+			POINTER_SIZE / BITS_PER_UNIT, POINTER_SIZE, 1);
     }
+}
 
-  assemble_integer (XEXP (DECL_RTL (src), 0),
-		    POINTER_SIZE / BITS_PER_UNIT, POINTER_SIZE, 1);
-  assemble_integer (XEXP (DECL_RTL (dst), 0),
-		    POINTER_SIZE / BITS_PER_UNIT, POINTER_SIZE, 1);
-  return 1;
+/* Helper comparison function for qsorting by the DECL_UID stored in
+   alias_pair->emitted_diags.  */
+
+static int
+tm_alias_pair_cmp (const void *x, const void *y)
+{
+  const tm_alias_pair *p1 = (const tm_alias_pair *) x;
+  const tm_alias_pair *p2 = (const tm_alias_pair *) y;
+  if (p1->uid < p2->uid)
+    return -1;
+  if (p1->uid > p2->uid)
+    return 1;
+  return 0;
 }
 
 void
 finish_tm_clone_pairs (void)
 {
-  bool switched = false;
+  VEC(tm_alias_pair,heap) *tm_alias_pairs = NULL;
 
-  if (tm_clone_pairs == NULL)
+  if (tm_clone_hash == NULL)
     return;
 
-  htab_traverse_noresize (tm_clone_pairs, finish_tm_clone_pairs_1,
-			  (void *) &switched);
-  htab_delete (tm_clone_pairs);
-  tm_clone_pairs = NULL;
+  /* We need a determenistic order for the .tm_clone_table, otherwise
+     we will get bootstrap comparison failures, so dump the hash table
+     to a vector, sort it, and dump the vector.  */
+
+  /* Dump the hashtable to a vector.  */
+  htab_traverse_noresize (tm_clone_hash, dump_tm_clone_to_vec,
+			  (void *) &tm_alias_pairs);
+  /* Sort it.  */
+  VEC_qsort (tm_alias_pair, tm_alias_pairs, tm_alias_pair_cmp);
+
+  /* Dump it.  */
+  dump_tm_clone_pairs (tm_alias_pairs);
+
+  htab_delete (tm_clone_hash);
+  tm_clone_hash = NULL;
+  VEC_free (tm_alias_pair, heap, tm_alias_pairs);
 }
 
 
Index: output.h
===================================================================
--- output.h	(revision 181028)
+++ output.h	(working copy)
@@ -608,7 +608,6 @@ extern void output_section_asm_op (const
 
 extern void record_tm_clone_pair (tree, tree);
 extern void finish_tm_clone_pairs (void);
-extern int finish_tm_clone_pairs_1 (void **, void *);
 extern tree get_tm_clone_pair (tree);
 
 extern void default_asm_output_source_filename (FILE *, const char *);

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [patch] 19/n: trans-mem: middle end/misc patches (LAST PATCH)
  2011-11-07  9:48     ` Richard Guenther
  2011-11-07 16:08       ` Aldy Hernandez
  2011-11-07 16:11       ` Richard Henderson
@ 2011-11-07 17:45       ` Aldy Hernandez
  2011-11-07 20:57         ` Aldy Hernandez
  2 siblings, 1 reply; 20+ messages in thread
From: Aldy Hernandez @ 2011-11-07 17:45 UTC (permalink / raw)
  To: Richard Guenther; +Cc: gcc-patches, Richard Henderson

[-- Attachment #1: Type: text/plain, Size: 193 bytes --]


> Do not push new_version->lowered setting into the core routine but keep
> it at the callers.

Well, why didn't you say so?  :).  That can't be hard :).

I'm testing this.

How does it look?

[-- Attachment #2: tm-lowered --]
[-- Type: text/plain, Size: 1914 bytes --]

Index: ChangeLog.tm-merge
===================================================================
--- ChangeLog.tm-merge	(revision 181098)
+++ ChangeLog.tm-merge	(working copy)
@@ -67,8 +67,7 @@
 	(struct cgraph_local_info): Add tm_may_enter_irr.
 	(cgraph_copy_node_for_versioning): Declare.
 	* cgraphunit.c (cgraph_copy_node_for_versioning): Export;
-	copy analyzed from old version. Move setting lowered to true from ...
-	(cgraph_function_versioning): ... here.
+	copy analyzed from old version.
 	* combine.c (distribute_notes): Handle REG_TM notes.
 	* common.opt: Add -fgnu-tm.
 	* crtstuff.c (__TMC_LIST__, __TMC_END__): New.
Index: cgraphunit.c
===================================================================
--- cgraphunit.c	(revision 181098)
+++ cgraphunit.c	(working copy)
@@ -2294,7 +2294,6 @@ cgraph_copy_node_for_versioning (struct 
    new_version->rtl = old_version->rtl;
    new_version->reachable = true;
    new_version->count = old_version->count;
-   new_version->lowered = true;
 
    for (e = old_version->callees; e; e=e->next_callee)
      if (!bbs_to_copy
@@ -2390,6 +2389,7 @@ cgraph_function_versioning (struct cgrap
   DECL_VIRTUAL_P (new_version_node->decl) = 0;
   new_version_node->local.externally_visible = 0;
   new_version_node->local.local = 1;
+  new_version_node->lowered = true;
 
   /* Update the call_expr on the edges to call the new version node. */
   update_call_expr (new_version_node);
Index: trans-mem.c
===================================================================
--- trans-mem.c	(revision 181098)
+++ trans-mem.c	(working copy)
@@ -4219,6 +4219,7 @@ ipa_tm_create_version (struct cgraph_nod
     DECL_COMDAT_GROUP (new_decl) = tm_mangle (DECL_COMDAT_GROUP (old_decl));
 
   new_node = cgraph_copy_node_for_versioning (old_node, new_decl, NULL, NULL);
+  new_node->lowered = true;
   new_node->tm_clone = 1;
   get_cg_data (old_node)->clone = new_node;
 

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [patch] 19/n: trans-mem: middle end/misc patches (LAST PATCH)
  2011-11-07 17:45       ` Aldy Hernandez
@ 2011-11-07 20:57         ` Aldy Hernandez
  0 siblings, 0 replies; 20+ messages in thread
From: Aldy Hernandez @ 2011-11-07 20:57 UTC (permalink / raw)
  To: Richard Guenther; +Cc: gcc-patches, Richard Henderson

On 11/07/11 09:43, Aldy Hernandez wrote:
>
>> Do not push new_version->lowered setting into the core routine but keep
>> it at the callers.
>
> Well, why didn't you say so? :). That can't be hard :).
>
> I'm testing this.
>
> How does it look?

rth approved this off-list.  I have just committed.

Richi, if you have any issues (with this patch, or anything else, please 
let me know).

Thanks.

^ permalink raw reply	[flat|nested] 20+ messages in thread

end of thread, other threads:[~2011-11-07 20:38 UTC | newest]

Thread overview: 20+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-11-03 19:58 [patch] 19/n: trans-mem: middle end/misc patches (LAST PATCH) Aldy Hernandez
2011-11-04 11:22 ` Richard Guenther
2011-11-06 19:07   ` Aldy Hernandez
2011-11-06 20:47     ` Richard Henderson
2011-11-06 22:01       ` Aldy Hernandez
2011-11-07  4:25       ` Aldy Hernandez
2011-11-07  7:53       ` Aldy Hernandez
2011-11-07  9:56         ` Richard Guenther
2011-11-07 15:54           ` Aldy Hernandez
2011-11-07 16:08             ` Richard Guenther
2011-11-07 17:12               ` Aldy Hernandez
2011-11-06 21:50     ` Richard Henderson
2011-11-07  9:48     ` Richard Guenther
2011-11-07 16:08       ` Aldy Hernandez
2011-11-07 16:14         ` Richard Henderson
2011-11-07 16:23           ` Aldy Hernandez
2011-11-07 16:32             ` Richard Henderson
2011-11-07 16:11       ` Richard Henderson
2011-11-07 17:45       ` Aldy Hernandez
2011-11-07 20:57         ` Aldy Hernandez

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).