From patchwork Mon Nov 21 15:42:39 2011 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Michael Matz X-Patchwork-Id: 126820 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from sourceware.org (server1.sourceware.org [209.132.180.131]) by ozlabs.org (Postfix) with SMTP id 89F85B720F for ; Tue, 22 Nov 2011 02:43:54 +1100 (EST) Received: (qmail 7766 invoked by alias); 21 Nov 2011 15:43:50 -0000 Received: (qmail 7749 invoked by uid 22791); 21 Nov 2011 15:43:48 -0000 X-SWARE-Spam-Status: No, hits=-3.6 required=5.0 tests=AWL, BAYES_00, RP_MATCHES_RCVD X-Spam-Check-By: sourceware.org Received: from cantor2.suse.de (HELO mx2.suse.de) (195.135.220.15) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Mon, 21 Nov 2011 15:42:41 +0000 Received: from relay1.suse.de (charybdis-ext.suse.de [195.135.221.2]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (No client certificate requested) by mx2.suse.de (Postfix) with ESMTP id 0EE15A2110; Mon, 21 Nov 2011 16:42:40 +0100 (CET) Date: Mon, 21 Nov 2011 16:42:39 +0100 (CET) From: Michael Matz To: gcc-patches@gcc.gnu.org Cc: Aldy Hernandez , Torvald Riegel Subject: Fix PR51125 (was: death@scope broke TM tests) In-Reply-To: <4EC6CB2F.5020004@redhat.com> Message-ID: References: <4EC6CB2F.5020004@redhat.com> MIME-Version: 1.0 X-IsSubscribed: yes Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Delivered-To: mailing list gcc-patches@gcc.gnu.org Hi, On Fri, 18 Nov 2011, Aldy Hernandez wrote: > I just CC'ed you on a bug that I believe was caused by your patch. > > I forgot to CC you on the bug before I wrote the comment on it. > > http://gcc.gnu.org/bugzilla/show_bug.cgi?id=51125#c4 > > Perhaps you can take a look? The attached patch fixes the problem. The TM machinery tried to deal with the death marker as if it was a real assignment leading to the trouble. The patch avoids this (leaving the clobber for expand to see). As I'm not totally sure about semantics of TM I don't know if the occurrence of this problem doesn't also point to a missed optimization for the TM handling. The situation after gimplification looks basically like so: __transaction_atomic { { struct shared_count sc; try // ctor(&sc), dtor(&sc) finally { sc = {CLOBBER}; } } } later lowered to: : __transaction_atomic // SUBCODE=[ GTMA_HAVE_STORE ] : // ctor(&sc),dtor(&sc) [tm-clone] goto ; : sc ={v} {CLOBBER}; __builtin__ITM_commitTransaction (); : return 0; : landing-pad The object 'sc' is constructed on the stack. Is it necessary to make it transact? If not, then you can possibly improve code quality somewhat by ignoring the clobbers also in requires_barrier (which then would need changes to receive the whole statement, not just the lhs/rhs to determine if this is an ignorable statement). I haven't tried this, though. The patch below would be necessary anyway. In any case, patch fixes the testcases again, regstrapping on x86_64-linux in progress. Okay for trunk? Ciao, Michael. PR other/51125 * trans-mem.c (expand_block_tm): Ignore clobbers. Index: trans-mem.c =================================================================== --- trans-mem.c (revision 181582) +++ trans-mem.c (working copy) @@ -2319,7 +2319,8 @@ expand_block_tm (struct tm_region *regio { case GIMPLE_ASSIGN: /* Only memory reads/writes need to be instrumented. */ - if (gimple_assign_single_p (stmt)) + if (gimple_assign_single_p (stmt) + && !gimple_clobber_p (stmt)) { expand_assign_tm (region, &gsi); continue;