From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 12418 invoked by alias); 21 Feb 2013 22:14:25 -0000 Received: (qmail 12405 invoked by uid 22791); 21 Feb 2013 22:14:24 -0000 X-SWARE-Spam-Status: No, hits=-6.6 required=5.0 tests=AWL,BAYES_00,KHOP_RCVD_UNTRUST,KHOP_SPAMHAUS_DROP,RCVD_IN_DNSWL_HI,RCVD_IN_HOSTKARMA_W,RP_MATCHES_RCVD,SPF_HELO_PASS,TW_TM X-Spam-Check-By: sourceware.org Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Thu, 21 Feb 2013 22:14:17 +0000 Received: from int-mx11.intmail.prod.int.phx2.redhat.com (int-mx11.intmail.prod.int.phx2.redhat.com [10.5.11.24]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id r1LMEGZA026752 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK) for ; Thu, 21 Feb 2013 17:14:16 -0500 Received: from houston.quesejoda.com (vpn-53-251.rdu2.redhat.com [10.10.53.251]) by int-mx11.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id r1LMEEKm020045; Thu, 21 Feb 2013 17:14:15 -0500 Message-ID: <51269C36.3070203@redhat.com> Date: Thu, 21 Feb 2013 22:14:00 -0000 From: Aldy Hernandez User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:17.0) Gecko/17.0 Thunderbird/17.0 MIME-Version: 1.0 To: Richard Henderson CC: gcc-patches Subject: do not pass PR_INSTRUMENTEDCODE if there is no instrumentation Content-Type: multipart/mixed; boundary="------------020508000301000204070005" Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org X-SW-Source: 2013-02/txt/msg01036.txt.bz2 This is a multi-part message in MIME format. --------------020508000301000204070005 Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Content-length: 806 It has come to my attention, by..ahem...you, that in handling the testcase from my previous patch: __transaction_relaxed { __asm__(""); } ...if we avoid instrumentation altogether, we shouldn't lie to the run-time and pass PR_INSTRUMENTEDCODE. I suppose we could cheat and avoid passing PR_INSTRUMENTEDCODE if we ever enter expand_block_tm(), but perhaps we could do a little better as with the attached patch. With this patch, we generate: tm_state.2_6 = __builtin__ITM_beginTransaction (16458); [ uninstrumentedCode hasNoAbort doesGoIrrevocable readOnly ] Notice there is no "instrumentedCode". By the way, can you double check my logic in expand_assign_tm() and expand_call_tm(), particularly if we should bother setting GTMA_INSTRUMENTED_CODE for thread private variables? Thanks. --------------020508000301000204070005 Content-Type: text/plain; charset=us-ascii; name="curr" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename="curr" Content-length: 3881 commit a441d916db334b71e76d9f4350ffdf992c058b4f Author: Aldy Hernandez Date: Thu Feb 21 16:02:11 2013 -0600 * trans-mem.c (expand_transaction): Only set PR_INSTRUMENTEDCODE if there was instrumented code. (expand_call_tm): Set GTMA_INSTRUMENTED_CODE. (expand_assign_tm): Same. * gimple.h (GTMA_INSTRUMENTED_CODE): New. diff --git a/gcc/gimple.h b/gcc/gimple.h index 4bd6b3d..baa5c24 100644 --- a/gcc/gimple.h +++ b/gcc/gimple.h @@ -661,6 +661,8 @@ struct GTY(()) gimple_statement_omp_atomic_store { tell the runtime that it should begin the transaction in serial-irrevocable mode. */ #define GTMA_DOES_GO_IRREVOCABLE (1u << 6) +/* The transaction has at least one instrumented instruction within. */ +#define GTMA_INSTRUMENTED_CODE (1u << 7) struct GTY(()) gimple_statement_transaction { diff --git a/gcc/testsuite/gcc.dg/tm/instrumented-mask.c b/gcc/testsuite/gcc.dg/tm/instrumented-mask.c new file mode 100644 index 0000000..6cfd3e4 --- /dev/null +++ b/gcc/testsuite/gcc.dg/tm/instrumented-mask.c @@ -0,0 +1,16 @@ +/* { dg-do compile } */ +/* { dg-options "-fgnu-tm -fdump-tree-tmmark" } */ + +/* If we're sure to go irrevocable, as in the case below, do not pass + PR_INSTRUMENTEDCODE to the run-time if there is nothing + instrumented within the transaction. */ + +int +main() +{ + __transaction_relaxed { __asm__(""); } + return 0; +} + +/* { dg-final { scan-tree-dump-times " instrumentedCode" 0 "tmmark" } } */ +/* { dg-final { cleanup-tree-dump "tmmark" } } */ diff --git a/gcc/trans-mem.c b/gcc/trans-mem.c index 71eaa44..315f00c 100644 --- a/gcc/trans-mem.c +++ b/gcc/trans-mem.c @@ -2146,6 +2146,9 @@ expand_assign_tm (struct tm_region *region, gimple_stmt_iterator *gsi) { /* Add thread private addresses to log if applicable. */ requires_barrier (region->entry_block, lhs, stmt); + // ?? Should we set GTMA_INSTRUMENTED_CODE here if there is no + // instrumentation, but requires_barrier added a thread private + // address to the log? gsi_next (gsi); return; } @@ -2153,6 +2156,8 @@ expand_assign_tm (struct tm_region *region, gimple_stmt_iterator *gsi) // Remove original load/store statement. gsi_remove (gsi, true); + transaction_subcode_ior (region, GTMA_INSTRUMENTED_CODE); + if (load_p && !store_p) { transaction_subcode_ior (region, GTMA_HAVE_LOAD); @@ -2230,9 +2235,12 @@ expand_call_tm (struct tm_region *region, if (fn_decl == builtin_decl_explicit (BUILT_IN_TM_MEMCPY) || fn_decl == builtin_decl_explicit (BUILT_IN_TM_MEMMOVE)) - transaction_subcode_ior (region, GTMA_HAVE_STORE | GTMA_HAVE_LOAD); + transaction_subcode_ior (region, GTMA_HAVE_STORE + | GTMA_HAVE_LOAD + | GTMA_INSTRUMENTED_CODE); if (fn_decl == builtin_decl_explicit (BUILT_IN_TM_MEMSET)) - transaction_subcode_ior (region, GTMA_HAVE_STORE); + transaction_subcode_ior (region, GTMA_HAVE_STORE + | GTMA_INSTRUMENTED_CODE); if (is_tm_pure_call (stmt)) return false; @@ -2243,7 +2251,8 @@ expand_call_tm (struct tm_region *region, { /* Assume all non-const/pure calls write to memory, except transaction ending builtins. */ - transaction_subcode_ior (region, GTMA_HAVE_STORE); + transaction_subcode_ior (region, GTMA_HAVE_STORE + | GTMA_INSTRUMENTED_CODE); } /* For indirect calls, we already generated a call into the runtime. */ @@ -2602,7 +2611,7 @@ expand_transaction (struct tm_region *region, void *data ATTRIBUTE_UNUSED) flags |= PR_HASNOABORT; if ((subcode & GTMA_HAVE_STORE) == 0) flags |= PR_READONLY; - if (inst_edge) + if (inst_edge && (subcode & GTMA_INSTRUMENTED_CODE)) flags |= PR_INSTRUMENTEDCODE; if (uninst_edge) flags |= PR_UNINSTRUMENTEDCODE; --------------020508000301000204070005--