From patchwork Fri Oct 9 05:26:07 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Alexandre Oliva X-Patchwork-Id: 528096 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 AC468140B04 for ; Fri, 9 Oct 2015 16:26:38 +1100 (AEDT) Authentication-Results: ozlabs.org; dkim=pass (1024-bit key; unprotected) header.d=gcc.gnu.org header.i=@gcc.gnu.org header.b=oyfKzNPf; dkim-atps=neutral DomainKey-Signature: a=rsa-sha1; c=nofws; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender:from :to:cc:subject:references:date:in-reply-to:message-id :mime-version:content-type; q=dns; s=default; b=m4FsoayInpPNVF/L d9OwIhXUdePsvxhxm4m82DqOc+8dZ+OM7dPRzVIDIGC4wBRG4Eusdp4VahwCrrP8 DWIFJ9u5pangOWrGoT0jopYo+ByF0JUyEnlLS/FV1yrpxJLtoqkpEfiumNOFWY03 qZAYX9eMUZZD0edY65FOCAO6qZo= 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:from :to:cc:subject:references:date:in-reply-to:message-id :mime-version:content-type; s=default; bh=CCetJkeeqpNMk/4tQKOZKw F/IJc=; b=oyfKzNPfgZcqOi41uTTA0Ac2r6eq0FJDbMIur4AgV603MX/tR5aRtH RYeLeO1fFVj6vHx7q1KhC7SH98PnOPART6Vvg21XWdE9Y8G4ULrNpqsOgNv7tB/j cIREXKfVOeKz+hObUhvpAb2JwTKlPnrzfs13BFVyXtJXDSOYQ6Beo= Received: (qmail 71668 invoked by alias); 9 Oct 2015 05:26:31 -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 71654 invoked by uid 89); 9 Oct 2015 05:26:30 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.7 required=5.0 tests=AWL, BAYES_00, SPF_HELO_PASS, T_RP_MATCHES_RCVD autolearn=ham version=3.3.2 X-HELO: mx1.redhat.com Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES256-GCM-SHA384 encrypted) ESMTPS; Fri, 09 Oct 2015 05:26:28 +0000 Received: from int-mx10.intmail.prod.int.phx2.redhat.com (int-mx10.intmail.prod.int.phx2.redhat.com [10.5.11.23]) by mx1.redhat.com (Postfix) with ESMTPS id B7E67A442E; Fri, 9 Oct 2015 05:26:26 +0000 (UTC) Received: from freie.home (ovpn01.gateway.prod.ext.phx2.redhat.com [10.5.9.1]) by int-mx10.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id t995QMB7022208 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=NO); Fri, 9 Oct 2015 01:26:24 -0400 Received: from livre.home (livre.home [172.31.160.2]) by freie.home (8.14.9/8.14.9) with ESMTP id t995Q74V000460; Fri, 9 Oct 2015 02:26:09 -0300 From: Alexandre Oliva To: Zhendong Su , Richard Biener Cc: Alan Lawrence , Jeff Law , James Greenhalgh , "H.J. Lu" , Segher Boessenkool , GCC Patches , Christophe Lyon , David Edelsohn , Eric Botcazou Subject: [PR67828] don't unswitch loops on undefined SSA values (was: Re: [PR64164] drop copyrename, integrate into expand) References: <20150723203112.GB27818@gate.crashing.org> <20150810082355.GA31149@arm.com> <55C8BFC3.3030603@redhat.com> <55E72D4C.40705@arm.com> <55FC3171.7040509@arm.com> Date: Fri, 09 Oct 2015 02:26:07 -0300 In-Reply-To: (Richard Biener's message of "Fri, 25 Sep 2015 13:21:34 +0200") Message-ID: User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/24.5 (gnu/linux) MIME-Version: 1.0 This patch fixes a latent bug in loop unswitching exposed by the PR64164 changes. We would move a test out of a loop that might never have been executed, and that accessed an uninitialized variable. The uninitialized SSA name, due to uncprop, now gets coalescesd with other SSA names, expanding the ill effects of the undefined behavior we introduce: in spite of the zero initialization introduced in later rtl stages for the uninitialized pseudo, by then we've already expanded a PHI node that referenced the unitialized variable in the path coming from a path in which it would necessarily be zero, to a copy from the coalesced pseudo, that gets modified between the zero-initialization and the copy, so the copied zero is no longer zero. Oops. We might want to be stricter in coalesce conflict detection to avoid this sort of problem, and perhaps to avoid undefined values in uncprop, but this would all be attempting to limit the effects of undefined behavior, which is probably a waste of effort. As long as we avoid introducing undefined behavior ourselves, we shouldn't have to do any of that. So, this patch fixes loop unswitching so as to not introduce undefined behavior. Regstrapped on x86_64-linux-gnu and i686-linux-gnu. Ok to install? [PR67828] don't unswitch on default defs of non-parms From: Alexandre Oliva for gcc/ChangeLog PR rtl-optimizatoin/67828 * tree-ssa-loop-unswitch.c: Include tree-ssa.h. (tree_may_unswitch_on): Don't unswitch on expressions involving undefined values. for gcc/testsuite/ChangeLog PR rtl-optimization/67828 * gcc.dg/torture/pr67828.c: New. --- gcc/testsuite/gcc.dg/torture/pr67828.c | 43 ++++++++++++++++++++++++++++++++ gcc/tree-ssa-loop-unswitch.c | 5 ++++ 2 files changed, 48 insertions(+) create mode 100644 gcc/testsuite/gcc.dg/torture/pr67828.c diff --git a/gcc/testsuite/gcc.dg/torture/pr67828.c b/gcc/testsuite/gcc.dg/torture/pr67828.c new file mode 100644 index 0000000..c7b6965 --- /dev/null +++ b/gcc/testsuite/gcc.dg/torture/pr67828.c @@ -0,0 +1,43 @@ +/* Check that we don't misoptimize the final value of d. We used to + apply loop unswitching on if(j), introducing undefined behavior + that the original code wouldn't exercise, and this undefined + behavior would get later passes to misoptimize the loop. */ + +/* { dg-do run } */ + +#include +#include + +int x; + +int __attribute__ ((noinline, noclone)) +xprintf (int d) { + if (d) + { + if (x) + printf ("%d", d); + abort (); + } +} + +int a, b; +short c; + +int +main () +{ + int j, d = 1; + for (; c >= 0; c++) + { + a = d; + d = 0; + if (b) + { + xprintf (0); + if (j) + xprintf (0); + } + } + xprintf (d); + exit (0); +} diff --git a/gcc/tree-ssa-loop-unswitch.c b/gcc/tree-ssa-loop-unswitch.c index 4328d6a..d6faa37 100644 --- a/gcc/tree-ssa-loop-unswitch.c +++ b/gcc/tree-ssa-loop-unswitch.c @@ -32,6 +32,7 @@ along with GCC; see the file COPYING3. If not see #include "internal-fn.h" #include "gimplify.h" #include "tree-cfg.h" +#include "tree-ssa.h" #include "tree-ssa-loop-niter.h" #include "tree-ssa-loop.h" #include "tree-into-ssa.h" @@ -139,6 +140,10 @@ tree_may_unswitch_on (basic_block bb, struct loop *loop) /* Condition must be invariant. */ FOR_EACH_SSA_TREE_OPERAND (use, stmt, iter, SSA_OP_USE) { + /* Unswitching on undefined values would introduce undefined + behavior that the original program might never exercise. */ + if (ssa_undefined_value_p (use, true)) + return NULL_TREE; def = SSA_NAME_DEF_STMT (use); def_bb = gimple_bb (def); if (def_bb