As discussed in the PR, we already remove nested transactions in the
tmlower stage, but inlining may add more nested transactions later.
The problem with removing these nested transactions after proper IPA, is
that we'd either have to add another IPA pass later or remove the
problematic transactions much later in the .tmmark stage. Either of
these places will be after we have added the uninstrumented code path,
so things look a little messy CFG-wise (not impossible to do, but messy).
What I've done in this patch is to fix the inliner cost model so that
the early inliner sees and inlines the transaction early on. This way,
.tmipa can see the inlined nested transaction before it adds the
uninstrumented code path, making everything simpler. Then it's just a
matter of removing the GIMPLE_TRANSACTION and associated commit.
I'm still unsure whether the IPA inliner (not the early inliner) will
add other nested transactions, so we may have to do everything in
.tmmark and handle the dual code paths :(. Either way, this is a start.
Also, is there anything I should check here, or is checking for
GTMA_HAVE_ABORT enough?
+ /* An inner transactions which has no explicit aborts can be
+ folded into its outer transaction. */
+ unsigned int sub = gimple_transaction_subcode
(region->transaction_stmt);
+ if (outer && !(sub & GTMA_HAVE_ABORT))
Tested on x86-64 Linux.
How does this look?
commit 460af022f8dcc5fb67410a8cd6a33d2be13819b9
Author: Aldy Hernandez <al...@redhat.com>
Date: Wed Dec 18 09:53:54 2013 -0800
PR tree-optimization/56572
* tree-inline.c (init_inline_once): Adjust .tm_cost size.
* trans-mem.c (remove_transaction): New.
(flatten_nested_transactions_1): New.
(flatten_nested_transactions): New.
(ipa_tm_execute): Call flatten_nested_transactions.
diff --git a/gcc/testsuite/gcc.dg/tm/pr56572.c
b/gcc/testsuite/gcc.dg/tm/pr56572.c
new file mode 100644
index 0000000..0b43569
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/tm/pr56572.c
@@ -0,0 +1,21 @@
+/* { dg-do compile } */
+/* { dg-options "-fgnu-tm -fdump-ipa-tmipa -O2" } */
+
+/* Test that nested unnecessary transactions are properly folded. */
+
+int a;
+
+static inline void f() {
+ __transaction_atomic {
+ ++a;
+ }
+}
+
+void g() {
+ __transaction_atomic {
+ f();
+ }
+}
+
+/* { dg-final { scan-ipa-dump-times "__transaction_atomic" 1 "tmipa" } } */
+/* { dg-final { cleanup-ipa-dump "tmipa" } } */
diff --git a/gcc/trans-mem.c b/gcc/trans-mem.c
index ba34488..5e9e2ab 100644
--- a/gcc/trans-mem.c
+++ b/gcc/trans-mem.c
@@ -2956,6 +2956,120 @@ propagate_tm_flags_out (struct tm_region *region)
propagate_tm_flags_out (region->next);
}
+/* Callback for expand_regions.
+
+ Given a transaction in REGION, remove the GIMPLE_TRANSACTION and
+ its corresponding BUILT_IN_TM_COMMIT*. The tm_region itself is
+ removed by the caller. */
+
+static void *
+remove_transaction (struct tm_region *region, void *data ATTRIBUTE_UNUSED)
+{
+ // Remove the transaction.
+ if (dump_file)
+ fprintf (dump_file, "Folding unnecessary inner transaction in BB%d\n",
+ gimple_bb (region->transaction_stmt)->index);
+ gimple_stmt_iterator gsi = gsi_for_stmt (region->transaction_stmt);
+ gsi_remove (&gsi, true);
+
+ // Find the corresponding BUILT_IN_TM_COMMIT* and remove it as well.
+ if (region->exit_blocks)
+ {
+ bitmap_iterator bi;
+ unsigned i;
+
+ EXECUTE_IF_SET_IN_BITMAP (region->exit_blocks, 0, i, bi)
+ {
+ gimple_stmt_iterator gsi
+ = gsi_last_bb (BASIC_BLOCK_FOR_FN (cfun, i));
+ gimple stmt = gsi_stmt (gsi);
+ if (gimple_code (stmt) != GIMPLE_CALL)
+ continue;
+ tree fndecl = gimple_call_fndecl (stmt);
+ if (fndecl
+ && DECL_BUILT_IN_CLASS (fndecl) == BUILT_IN_NORMAL
+ && (DECL_FUNCTION_CODE (fndecl) == BUILT_IN_TM_COMMIT
+ || DECL_FUNCTION_CODE (fndecl) == BUILT_IN_TM_COMMIT_EH))
+ {
+ tree vdef = gimple_vdef (stmt);
+ tree vuse = gimple_vuse (stmt);
+ if (vdef && vuse)
+ replace_uses_by (vdef, vuse);
+ gsi_remove (&gsi, true);
+ }
+ }
+ }
+ return NULL;
+}
+
+/* A helper function for flatten_nested_transactions.
+
+ Go through all the transactional regions, starting at TOP, and
+ remove any nested transactions that are redundant, removing both
+ the start/end of a transaction as well as removing the region
+ from the tm_region list.
+
+ OUTER is TOP's outer transaction and is set to null if TOP is the
+ outermost transaction.
+
+ Returns FALSE if the region passed in TOP should be removed from
+ the list of regions by the caller. */
+static bool
+flatten_nested_transactions_1 (struct tm_region *top, struct tm_region *outer)
+{
+ /* We could fold this traversal into ipa_tm_scan_calls_transaction,
+ and only traverse all_tm_regions once, but the nested
+ transactions we handle below would complicate things. Besides,
+ there aren't liable to be many TM regions in a function to make
+ this extra iteration so unprofitable.
+
+ ?? We could even only call flatten_nested_transactions from
+ within ipa_tm_scan_calls_transaction only if we have any inner
+ transactions. Hmmm, probably not worth it. */
+ struct tm_region *prev = NULL;
+ for (struct tm_region *region = top; region; region = region->next)
+ {
+ if (region->inner
+ && !flatten_nested_transactions_1 (region->inner, region))
+ region->inner = NULL;
+
+ /* An inner transactions which has no explicit aborts can be
+ folded into its outer transaction. */
+ unsigned int sub = gimple_transaction_subcode (region->transaction_stmt);
+ if (outer && !(sub & GTMA_HAVE_ABORT))
+ {
+ expand_regions (region, remove_transaction, NULL, false);
+
+ /* Remove region from the list if its anything but the first
+ item, otherwise make the caller remove the first
+ item. */
+ if (prev == NULL)
+ return false;
+ else
+ prev->next = region->next;
+ }
+ prev = region;
+ }
+ return true;
+}
+
+/* Go through all the transactional regions and remove any nested
+ transactions that are redundant. This is done by removing the
+ GIMPLE_TRANSACTION + BUILT_IN_TM_COMMIT* pairs and by removing the
+ transaction region from the all_tm_regions list.
+
+ Note: We already did this in the tmlower pass (lower_transaction),
+ but inlining may have added more nested transactions that are now
+ candidates for removal. */
+
+static void
+flatten_nested_transactions ()
+{
+ if (!flatten_nested_transactions_1 (all_tm_regions, NULL))
+ all_tm_regions = NULL;
+}
+
+
/* Entry point to the MARK phase of TM expansion. Here we replace
transactional memory statements with calls to builtins, and function
calls with their transactional clones (if available). But we don't
@@ -5358,6 +5472,8 @@ ipa_tm_execute (void)
{
d = get_cg_data (&node, true);
+ flatten_nested_transactions ();
+
/* Scan for calls that are in each transaction, and
generate the uninstrumented code path. */
ipa_tm_scan_calls_transaction (d, &tm_callees);
diff --git a/gcc/tree-inline.c b/gcc/tree-inline.c
index 96a4805..6c6ef41 100644
--- a/gcc/tree-inline.c
+++ b/gcc/tree-inline.c
@@ -3972,7 +3972,7 @@ init_inline_once (void)
eni_size_weights.target_builtin_call_cost = 1;
eni_size_weights.div_mod_cost = 1;
eni_size_weights.omp_cost = 40;
- eni_size_weights.tm_cost = 10;
+ eni_size_weights.tm_cost = 2;
eni_size_weights.time_based = false;
eni_size_weights.return_cost = 1;
