From patchwork Thu Sep 6 07:50:44 2012 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Andrew Pinski X-Patchwork-Id: 182065 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 4C9032C0099 for ; Thu, 6 Sep 2012 17:51:09 +1000 (EST) Comment: DKIM? See http://www.dkim.org DKIM-Signature: v=1; a=rsa-sha1; c=relaxed/relaxed; d=gcc.gnu.org; s=default; x=1347522669; h=Comment: DomainKey-Signature:Received:Received:Received:Received: MIME-Version:Received:Received:In-Reply-To:References:Date: Message-ID:Subject:From:To:Cc:Content-Type:Mailing-List: Precedence:List-Id:List-Unsubscribe:List-Archive:List-Post: List-Help:Sender:Delivered-To; bh=Qh4FIUhy+nCXsmAZ/pA7alpL3Mk=; b=Cr4jQ6o0VG7cd5BYyIUa8wso5ZyDn0k1BC9B48SKn1XffWrYYzF5JkFCDC5zTj p9SfgFsF3Je1Z4llLvMPxL7DnpGcCrIWuqW8hiSYQh5Tr9BdMj3bO88ToxETQMcG AXHXeCHGOGQIZKVEdqqFpI8126qZvsZOqtVg/N1cQNXJ8= Comment: DomainKeys? See http://antispam.yahoo.com/domainkeys DomainKey-Signature: a=rsa-sha1; q=dns; c=nofws; s=default; d=gcc.gnu.org; h=Received:Received:X-SWARE-Spam-Status:X-Spam-Check-By:Received:Received:MIME-Version:Received:Received:In-Reply-To:References:Date:Message-ID:Subject:From:To:Cc:Content-Type:X-IsSubscribed:Mailing-List:Precedence:List-Id:List-Unsubscribe:List-Archive:List-Post:List-Help:Sender:Delivered-To; b=rANlgmOhjM+xq0VPKyi86uF8x+4bq/M/U7ouRApn9ieGkTsK+GYvzNgIDk6Oka LlxtfU96lGsL5dvZpHhZqRsbXPod7Lg5EC1xkgeyQ7fzAEcDNJZ5foSqv5UlrjYb jsDQ2CUdy6RdV7QM8oo6Qdo/CXbMqBjpQPfDKbXfwi5dI=; Received: (qmail 31301 invoked by alias); 6 Sep 2012 07:51:04 -0000 Received: (qmail 31290 invoked by uid 22791); 6 Sep 2012 07:51:03 -0000 X-SWARE-Spam-Status: No, hits=-5.0 required=5.0 tests=AWL, BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, FREEMAIL_FROM, KHOP_RCVD_TRUST, KHOP_THREADED, RCVD_IN_DNSWL_LOW, RCVD_IN_HOSTKARMA_YE X-Spam-Check-By: sourceware.org Received: from mail-wg0-f51.google.com (HELO mail-wg0-f51.google.com) (74.125.82.51) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Thu, 06 Sep 2012 07:50:46 +0000 Received: by wgbed3 with SMTP id ed3so949821wgb.8 for ; Thu, 06 Sep 2012 00:50:45 -0700 (PDT) MIME-Version: 1.0 Received: by 10.216.131.196 with SMTP id m46mr816784wei.35.1346917844901; Thu, 06 Sep 2012 00:50:44 -0700 (PDT) Received: by 10.216.2.4 with HTTP; Thu, 6 Sep 2012 00:50:44 -0700 (PDT) In-Reply-To: <20120906070757.GQ1999@tucnak.redhat.com> References: <20120906070757.GQ1999@tucnak.redhat.com> Date: Thu, 6 Sep 2012 00:50:44 -0700 Message-ID: Subject: Re: [PATCH] Fix PR 54494, removal of volatile store in strlen optimization because of the inlininer From: Andrew Pinski To: Jakub Jelinek Cc: GCC Patches , Jan Hubicka 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 On Thu, Sep 6, 2012 at 12:07 AM, Jakub Jelinek wrote: > On Wed, Sep 05, 2012 at 09:10:53PM -0700, Andrew Pinski wrote: >> The inlininer likes to recreate some MEM_REF, it copies most of the >> bits (TREE_THIS_NOTRAP, TREE_THIS_VOLATILE, etc.) but forgets about >> TREE_SIDE_EFFECTS. This causes the strlen optimization to think the >> memory store does not have a side effects. >> >> OK? Bootstrapped and tested on x86_64-linux-gnu with no regressions. >> >> Thanks, >> Andrew Pinski >> >> ChangeLog: >> >> * tree-inline.c (remap_gimple_op_r): Copy TREE_SIDE_EFFECTS also. >> >> testsuite/ChangeLog: >> * gcc.dg/tree-ssa/strlen-1.c: New testcase. > > Patch preapproved, but you've attached a different patch. Sorry about that. Here is the correct one. Also is this ok for the 4.7 branch? Thanks, Andrew Pinski > > I'd say copy_tree_body_r's MEM_REF handling should also copy > TREE_SIDE_EFFECTS/TREE_THIS_NOTRAP (what about TREE_READONLY?), > maybe copy_decl_to_var/copy_result_decl_to_var should also > copy TREE_SIDE_EFFECTS, perhaps omp-low.c copy_var_decl, install_var_field, > tree-nested.c lookup_field_for_decl, tree-sra.c sra_ipa_reset_debug_stmts > (grep TREE_THIS_VOLATILE.*TREE_THIS_VOLATILE, looking for missing > TREE_SIDE_EFFECTS copy nearby). That said, perhaps the tree-ssa-strlen.c > change is desirable too, unless we add some checking that TREE_THIS_VOLATILE > references/decls have TREE_SIDE_EFFECTS bit set in the IL. > >> --- testsuite/gcc.c-torture/compile/pr49474.c (revision 0) >> +++ testsuite/gcc.c-torture/compile/pr49474.c (revision 0) >> --- cprop.c (revision 176187) >> +++ cprop.c (working copy) > > Jakub Index: testsuite/gcc.dg/tree-ssa/strlen-1.c =================================================================== --- testsuite/gcc.dg/tree-ssa/strlen-1.c (revision 0) +++ testsuite/gcc.dg/tree-ssa/strlen-1.c (revision 0) @@ -0,0 +1,17 @@ +/* { dg-do compile } */ +/* { dg-options "-O2 -fdump-tree-optimized" } */ +extern const unsigned long base; +static inline void wreg(unsigned char val, unsigned long addr) __attribute__((always_inline)); +static inline void wreg(unsigned char val, unsigned long addr) +{ + *((volatile unsigned char *) (__SIZE_TYPE__) (base + addr)) = val; +} +void wreg_twice(void) +{ + wreg(0, 42); + wreg(0, 42); +} + +/* We should not remove the second null character store to (base+42) address. */ +/* { dg-final { scan-tree-dump-times " ={v} 0;" 2 "optimized" } } */ +/* { dg-final { cleanup-tree-dump "optimized" } } */ Index: tree-inline.c =================================================================== --- tree-inline.c (revision 191004) +++ tree-inline.c (working copy) @@ -848,6 +848,7 @@ remap_gimple_op_r (tree *tp, int *walk_s ptr, TREE_OPERAND (*tp, 1)); TREE_THIS_NOTRAP (*tp) = TREE_THIS_NOTRAP (old); TREE_THIS_VOLATILE (*tp) = TREE_THIS_VOLATILE (old); + TREE_SIDE_EFFECTS (*tp) = TREE_SIDE_EFFECTS (old); TREE_NO_WARNING (*tp) = TREE_NO_WARNING (old); *walk_subtrees = 0; return NULL;