From patchwork Sun Jan 11 12:39:46 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Bernd Edlinger X-Patchwork-Id: 427461 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]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id D7499140151 for ; Sun, 11 Jan 2015 23:40:00 +1100 (AEDT) DomainKey-Signature: a=rsa-sha1; c=nofws; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender :message-id:content-type:from:to:cc:subject:date:in-reply-to :references:mime-version; q=dns; s=default; b=dUlURlAzPZTU0THzKZ Ny19cXExJtB2WQKO1Wy5C088ek7hHMehiEQabFY8p0PTJZ9mYKS/TaFOK/Ar/GTD xhN/p7mQMi+r7wz6U78IwmHOF1qLd6F3KnUhWERKvbsPzbyACEvkpbmNwsYJd540 hUeWxhQj7z5gIRvZrk70yu1tc= DKIM-Signature: v=1; a=rsa-sha1; c=relaxed; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender :message-id:content-type:from:to:cc:subject:date:in-reply-to :references:mime-version; s=default; bh=v4fde1c0KuLRcQ2TGAWTJ5bL +y4=; b=UQCxpLoVMwiEqJqJTGTv7FWPjVQhCqpyZz7gCo7Qujy7QWPHEG2LMWuu QoSysU7jK2xiMc3vy1m6z8ZWJgr16e+NLj7EQPaWoEyvGiom2vbOlpM/v6FsA8/x tvvHNPV/Zwbp34QB/UlQeiRmvrkGu5gRCGjd6U5lOI+mC9iZ1Ts= Received: (qmail 26254 invoked by alias); 11 Jan 2015 12:39:52 -0000 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 Received: (qmail 26239 invoked by uid 89); 11 Jan 2015 12:39:51 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.0 required=5.0 tests=AWL, BAYES_00, FREEMAIL_FROM, RCVD_IN_DNSWL_NONE, SPF_PASS autolearn=ham version=3.3.2 X-HELO: DUB004-OMC4S26.hotmail.com Received: from dub004-omc4s26.hotmail.com (HELO DUB004-OMC4S26.hotmail.com) (157.55.2.101) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES256-SHA256 encrypted) ESMTPS; Sun, 11 Jan 2015 12:39:49 +0000 Received: from DUB118-W11 ([157.55.2.71]) by DUB004-OMC4S26.hotmail.com over TLS secured channel with Microsoft SMTPSVC(7.5.7601.22751); Sun, 11 Jan 2015 04:39:46 -0800 X-TMN: [3scfh24z4Zbsjrhzy7eVBzJALsnVZ+Ce] Message-ID: From: Bernd Edlinger To: Richard Biener CC: Jakub Jelinek , Jeff Law , "gcc-patches@gcc.gnu.org" , Eric Botcazou Subject: RE: [PATCH] Enable experimental TSAN support for Ada Date: Sun, 11 Jan 2015 13:39:46 +0100 In-Reply-To: <80580167-6720-4147-9059-1BAF9EBDBD05@gmail.com> References: , <54AAED52.5040506@redhat.com>, , , , , <20150109123045.GY1405@tucnak.redhat.com>, , , <80580167-6720-4147-9059-1BAF9EBDBD05@gmail.com> MIME-Version: 1.0 Hi Richard, On Fri, 9 Jan 2015 17:19:57, Richard Biener wrote: > > > Yes. As said, you generally need to run folding results through force_gimple_operand. > > Richard. > I have now used force_gimple_operand instead of special casing the VIEW_CONVERT_EXPRs. And I see that all Ada test cases still work with -fsanitize=thread. So this feels like an improvement. I have checked with a large C++ application, to see if the generated code changes or not. And although this looked like it should not change the resulting code, I found one small difference at -O3 -fsanitize=thread while compiling the function xmlSchemaCompareValuesInt in xmlschematypes.c of libxml2.  The generated code size did not change, only two blocks of code changed place. That was the only difference in about 16 MB of code. The reason for this seems to be the following changes in the xmlschemastypes.c.104t.tsan1    :    p1_179 = xmlSchemaDateNormalize (x_7(D), 0.0);    # DEBUG p1 => p1_179    _180 = _xmlSchemaDateCastYMToDays (p1_179); -  _660 = &p1_179->value.date; -  _659 = &MEM[(struct xmlSchemaValDate *)_660 + 8B]; -  __builtin___tsan_read2 (_659); +  _660 = &MEM[(struct xmlSchemaValDate *)p1_179 + 24B]; +  __builtin___tsan_read2 (_660);    _181 = p1_179->value.date.day;    _182 = (long int) _181;    p1d_183 = _180 + _182; this pattern is repeated everywhere. (- = before the patch. + = with the patch) So it looks as if the generated code quality slightly improves with this change. I have also tried to fold &base + offset + bitpos,  like this: For simplicity first only in the simple case without DECL_BIT_FIELD_REPRESENTATIVE. I tried this change at the same large C++ application, and see the code still works, but the binary size increases at -O3 by about 1%. So my conclusion would be that it is better to use force_gimple_operand directly on build_fold_addr_expr (unshare_expr (expr)), without using offset. Well, I think this still resolves your objections. Furthermore I used may_be_nonaddressable_p instead of is_gimple_addressable and just return if it is found to be not true. (That did not happen in my tests.) And I reworked the block with the pt_solution_includes. I found that It can be rewritten, because pt_solution_includes can be expanded to (is_global_var (decl) || pt_solution_includes_1 (&cfun->gimple_df->escaped, decl) || pt_solution_includes_1 (&ipa_escaped_pt, decl)) So, by De Morgan's law, you can rewite that block to   if (DECL_P (base))     {       if (!is_global_var (base)           && !pt_solution_includes_1 (&cfun->gimple_df->escaped, base)           && !pt_solution_includes_1 (&ipa_escaped_pt, base))         return false;       if (!is_global_var (base) && !may_be_aliased (base))         return false;     } Therefore I can move the common term !is_global_var (base) out of the block.  That's what I did. As far as I can tell, none of the other terms here seem to be redundant. Attached patch was boot-strapped and regression-tested on x86_64-linux-gnu. OK for trunk? Thanks Bernd. gcc/ChangeLog: 2015-01-11 Bernd Edlinger * tsan.c (instrument_expr): Use force_gimple_operand. Use may_be_nonaddressable_p instead of is_gimple_addressable. --- tsan.c.orig    2015-01-10 00:39:06.465210937 +0100 +++ tsan.c    2015-01-11 09:28:38.109423856 +0100 @@ -213,7 +213,18 @@ instrument_expr (gimple_stmt_iterator gs        align = get_object_alignment (expr);        if (align < BITS_PER_UNIT)      return false; -      expr_ptr = build_fold_addr_expr (unshare_expr (expr)); +      expr_ptr = build_fold_addr_expr (unshare_expr (base)); +      if (bitpos != 0) +    { +      if (offset != NULL) +        offset = size_binop (PLUS_EXPR, offset, +                 build_int_cst (sizetype, +                        bitpos / BITS_PER_UNIT)); +      else +        offset = build_int_cst (sizetype, bitpos / BITS_PER_UNIT); +    } +      if (offset != NULL) +    expr_ptr = fold_build_pointer_plus (expr_ptr, offset);      }    expr_ptr = force_gimple_operand (expr_ptr, &seq, true, NULL_TREE);    if ((size & (size - 1)) != 0 || size> 16