From patchwork Fri Sep 11 15:38:29 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Kyrylo Tkachov X-Patchwork-Id: 516844 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 928EE14012C for ; Sat, 12 Sep 2015 01:38:44 +1000 (AEST) Authentication-Results: ozlabs.org; dkim=pass (1024-bit key; unprotected) header.d=gcc.gnu.org header.i=@gcc.gnu.org header.b=x6ZvPUNE; 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 :message-id:date:from:mime-version:to:cc:subject:references :in-reply-to:content-type; q=dns; s=default; b=GFEEC1wCgEGUex5dg va7Pf9d5dNJ5oZgDl9r9fzfyZjRLGnLMSn82Atat9BWZyaBlxj2S3rM+14OHdN5J xLTC2SIC700p7XtCIZDpHfz7pDGKDr3AtNyDwV9c5D/oDQYjlorRXwtv2SlrZE6+ ixswOOaXR93OGUPH4peHm+HhRM= 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:date:from:mime-version:to:cc:subject:references :in-reply-to:content-type; s=default; bh=iK5XcIPB2gwHJrSyqbQ9yLd 7QxI=; b=x6ZvPUNErOBx0KqGzkmEnXb/LTms+GG86c/xyliKWsarfa+eQfWIDVm P/X1o9lWgVK+eo0ahl1X1EzDgdxu6LHOOBYTYbZeOjwteJM9HmWh2luKabhKQNqA aLxZNC3YiFcq9ow3CD0Y89wC0GcV56DbqyuzDxrFzNJhNxOPwMWQ= Received: (qmail 40885 invoked by alias); 11 Sep 2015 15:38:36 -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 40874 invoked by uid 89); 11 Sep 2015 15:38:36 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.7 required=5.0 tests=AWL, BAYES_00, SPF_PASS autolearn=ham version=3.3.2 X-HELO: eu-smtp-delivery-143.mimecast.com Received: from eu-smtp-delivery-143.mimecast.com (HELO eu-smtp-delivery-143.mimecast.com) (207.82.80.143) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Fri, 11 Sep 2015 15:38:34 +0000 Received: from cam-owa1.Emea.Arm.com (fw-tnat.cambridge.arm.com [217.140.96.140]) by eu-smtp-1.mimecast.com with ESMTP id uk-mta-24-XbCegpijSa-MUn4p632d7w-1; Fri, 11 Sep 2015 16:38:29 +0100 Received: from [10.2.207.50] ([10.1.2.79]) by cam-owa1.Emea.Arm.com with Microsoft SMTPSVC(6.0.3790.3959); Fri, 11 Sep 2015 16:38:29 +0100 Message-ID: <55F2F575.6080609@arm.com> Date: Fri, 11 Sep 2015 16:38:29 +0100 From: Kyrill Tkachov User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.2.0 MIME-Version: 1.0 To: Rainer Orth CC: GCC Patches , Jeff Law Subject: Re: [PATCH][RTL-ifcvt] PR rtl-optimization/67465: Handle pairs of complex+simple blocks and empty blocks more gracefully References: <55F13D66.9010207@arm.com> <55F197EB.3010404@arm.com> In-Reply-To: X-MC-Unique: XbCegpijSa-MUn4p632d7w-1 X-IsSubscribed: yes On 11/09/15 09:51, Rainer Orth wrote: > Kyrill Tkachov writes: > >> On 10/09/15 12:43, Rainer Orth wrote: >>> Hi Kyrill, >>> >>>> Rainer, could you please check that this patch still fixes the SPARC >>>> regressions? >>> unfortunately, it breaks sparc-sun-solaris2.10 bootstrap: compiling >>> stage2 libiberty/regex.c FAILs: >>> >>> >> Thanks for providing the preprocessed file. >> I've reproduced and fixed the ICE in this version of the patch. >> The problem was that I was taking the mode of x before the check >> of whether a and b are MEMs, after which we would change x to an >> address_mode reg, >> thus confusing emit_move_insn. >> >> The fix is to take the mode of x and perform the can_conditionally_move_p check >> after that transformation. >> >> Bootstrapped and tested on aarch64 and x86_64. >> The preprocessed regex.i that Rainer provided now compiles successfully for me >> on a sparc-sun-solaris2.10 stage-1 cross-compiler. >> >> Rainer, thanks for your help so far, could you please try out this patch? > While bootstrap succeeds again, the testsuite regression in > gcc.c-torture/execute/20071216-1.c reoccured. Right, so I dug into the RTL dumps and I think this is a separate issue that's being exacerbated by my patch. The code tries to if-convert a block which contains a compare instruction i.e. sets the CC register. Now, bb_valid_for_noce_process_p should have caught this, and in particular insn_valid_noce_process_p which should check that the instruction doesn't set the CC register. However, on SPARC the cc_in_cond returns NULL! This is due to the canonicalize_comparison implementation that seems to remove the CC register from the condition expression and returns something like: (leu (reg/v:SI 109 [ b ]) (const_int -4096 [0xfffffffffffff000]) Therefore the set_of (cc_in_cond (cond), insn) check doesn't get triggered because cc_in_cond returns NULL. Regardless of how the branch condition got canonicalized, I think we still want to reject any insn in the block that sets a condition code register, so this patch checks the destination of every set in the block for a MODE_CC expression and cancels if-conversion if that's the case. Oleg pointed me to the older PR 58517 affecting SH which seems similar and I think my previous ifcvt patch would expose this problem more. Anyway, with this patch the failing SPARC testcase gcc.c-torture/execute/20071216-1.c generates the same assembly as before r227368 and bootstrap and test on aarch64 and x86_64 passes ok for me. Rainer, could you try this patch on top of the previous patch? (https://gcc.gnu.org/ml/gcc-patches/2015-09/msg00689.html) The two together should fix all of PR 67456, 67464, 67465 and 67481. Thanks, Kyrill 2015-09-11 Kyrylo Tkachov PR rtl-optimization/67481 * ifcvt.c (contains_ccmode_rtx_p): New function. (insn_valid_noce_process_p): Use it. diff --git a/gcc/ifcvt.c b/gcc/ifcvt.c index 9af3249..090a584 100644 --- a/gcc/ifcvt.c +++ b/gcc/ifcvt.c @@ -1838,6 +1838,19 @@ noce_try_cmove (struct noce_if_info *if_info) return FALSE; } +/* Return true if X contains a conditional code mode rtx. */ + +static bool +contains_ccmode_rtx_p (rtx x) +{ + subrtx_iterator::array_type array; + FOR_EACH_SUBRTX (iter, array, x, ALL) + if (GET_MODE_CLASS (GET_MODE (*iter)) == MODE_CC) + return true; + + return false; +} + /* Helper for bb_valid_for_noce_process_p. Validate that the rtx insn INSN is a single set that does not set the conditional register CC and is in general valid for @@ -1856,6 +1869,7 @@ insn_valid_noce_process_p (rtx_insn *insn, rtx cc) /* Currently support only simple single sets in test_bb. */ if (!sset || !noce_operand_ok (SET_DEST (sset)) + || contains_ccmode_rtx_p (SET_DEST (sset)) || !noce_operand_ok (SET_SRC (sset))) return false;