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.
        * 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;

Reply via email to