From patchwork Tue Jul 5 13:50:06 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Kyrill Tkachov X-Patchwork-Id: 644782 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 3rkQJ65YLjz9sf9 for ; Tue, 5 Jul 2016 23:50:28 +1000 (AEST) Authentication-Results: ozlabs.org; dkim=pass (1024-bit key; unprotected) header.d=gcc.gnu.org header.i=@gcc.gnu.org header.b=yyObrDlT; 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:subject:references :in-reply-to:content-type; q=dns; s=default; b=qxsPNi6cgwLpAUuFm jn5316LQBkzKgC2q1Fz9Hf9eZh7Qst0KO7rnMd+vYJ8wxvhkKlxKEpalYG3PKW3Q fs+LEj97HgGJXJuyADGYairrQlOy/Zvb6hks6cvHfBMkjlEO0VA/QYr4XpqWrhfO /LCUuYugLLBOPQ2ycGBXjrkh20= 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:subject:references :in-reply-to:content-type; s=default; bh=2943zRtFxudl4nBu4Ua6Hjm erGQ=; b=yyObrDlTnV8TcWhfnoFh70JZG3OORRLWQDtYfeyIvq2X3BUWM99QCqc B1dCpIl937dFUtHEGlWkn0hkMeAy5diQCW3+rKnBSzzUI3QOoOuebadKUBe/9niK nyZWwD+m5BuSVdRV+uaU6BqJr+72JZ6kCxczSsoYDqv6CeGtsglg= Received: (qmail 118376 invoked by alias); 5 Jul 2016 13:50:20 -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 118355 invoked by uid 89); 5 Jul 2016 13:50:20 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.2 required=5.0 tests=BAYES_00, KAM_LAZY_DOMAIN_SECURITY, RP_MATCHES_RCVD autolearn=ham version=3.3.2 spammy= X-HELO: foss.arm.com Received: from foss.arm.com (HELO foss.arm.com) (217.140.101.70) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Tue, 05 Jul 2016 13:50:09 +0000 Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.72.51.249]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id 4CA1828; Tue, 5 Jul 2016 06:51:06 -0700 (PDT) Received: from [10.2.206.43] (e100706-lin.cambridge.arm.com [10.2.206.43]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id B7E793F215; Tue, 5 Jul 2016 06:50:07 -0700 (PDT) Message-ID: <577BBB0E.4010509@foss.arm.com> Date: Tue, 05 Jul 2016 14:50:06 +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: Bernd Schmidt , gcc-patches@gcc.gnu.org Subject: Re: [PATCH][RTL ifcvt] PR rtl-optimization/71594: ICE in noce_emit_cmove due to mismatched source modes References: <576CF030.1010600@foss.arm.com> <577A3A5E.1020304@foss.arm.com> <953831b2-c361-5285-354b-26405a1282fc@redhat.com> <577A4601.1070506@foss.arm.com> <59f70e60-928d-8203-9fc5-55801d6d4fdc@redhat.com> In-Reply-To: <59f70e60-928d-8203-9fc5-55801d6d4fdc@redhat.com> On 04/07/16 12:19, Bernd Schmidt wrote: > On 07/04/2016 01:18 PM, Kyrill Tkachov wrote: >> That does seem like it could cause trouble but I couldn't think of how >> that sequence could appear or what its >> semantics would be. Would assigning to the SImode reg 0 in your example >> not touch the upper bits of the DImode value? > > No, multi-word subreg accesses are per-word. > >> In any case, bb_ok_for_noce_convert_multiple_sets doesn't keep track of >> dependencies between the instructions >> so I think the best place to handle this case would be in >> noce_convert_multiple_sets where instead of the assert >> in this patch we'd just end_sequence () and return FALSE. >> Would that be preferable? > > That should at least work, and I'd be ok with that. > Ok, here's the updated patch with the assert replaced by failing the conversion. Bootstrapped and tested on x86_64. Also tested on aarch64. Is this ok? Thanks, Kyrill 2016-07-05 Kyrylo Tkachov PR rtl-optimization/71594 * ifcvt.c (noce_convert_multiple_sets): Wrap new_val or old_val into subregs of appropriate mode before trying to emit a conditional move. 2016-07-05 Kyrylo Tkachov PR rtl-optimization/71594 * gcc.dg/torture/pr71594.c: New test. diff --git a/gcc/ifcvt.c b/gcc/ifcvt.c index fd2951673fb6bd6d9e5d52cdb88765434a603fb6..f7f120e04b11dc4f25be969e0c183a36e067a61c 100644 --- a/gcc/ifcvt.c +++ b/gcc/ifcvt.c @@ -3228,6 +3228,41 @@ noce_convert_multiple_sets (struct noce_if_info *if_info) if (if_info->then_else_reversed) std::swap (old_val, new_val); + + /* We allow simple lowpart register subreg SET sources in + bb_ok_for_noce_convert_multiple_sets. Be careful when processing + sequences like: + (set (reg:SI r1) (reg:SI r2)) + (set (reg:HI r3) (subreg:HI (r1))) + For the second insn new_val or old_val (r1 in this example) will be + taken from the temporaries and have the wider mode which will not + match with the mode of the other source of the conditional move, so + we'll end up trying to emit r4:HI = cond ? (r1:SI) : (r3:HI). + Wrap the two cmove operands into subregs if appropriate to prevent + that. */ + if (GET_MODE (new_val) != GET_MODE (temp)) + { + machine_mode src_mode = GET_MODE (new_val); + machine_mode dst_mode = GET_MODE (temp); + if (GET_MODE_SIZE (src_mode) <= GET_MODE_SIZE (dst_mode)) + { + end_sequence (); + return FALSE; + } + new_val = lowpart_subreg (dst_mode, new_val, src_mode); + } + if (GET_MODE (old_val) != GET_MODE (temp)) + { + machine_mode src_mode = GET_MODE (old_val); + machine_mode dst_mode = GET_MODE (temp); + if (GET_MODE_SIZE (src_mode) <= GET_MODE_SIZE (dst_mode)) + { + end_sequence (); + return FALSE; + } + old_val = lowpart_subreg (dst_mode, old_val, src_mode); + } + /* Actually emit the conditional move. */ rtx temp_dest = noce_emit_cmove (if_info, temp, cond_code, x, y, new_val, old_val); diff --git a/gcc/testsuite/gcc.dg/torture/pr71594.c b/gcc/testsuite/gcc.dg/torture/pr71594.c new file mode 100644 index 0000000000000000000000000000000000000000..468a9f6891c92ff76520af0eee242f08b01ae0cf --- /dev/null +++ b/gcc/testsuite/gcc.dg/torture/pr71594.c @@ -0,0 +1,15 @@ +/* { dg-do compile } */ +/* { dg-options "--param max-rtl-if-conversion-insns=2" } */ + +unsigned short a; +int b, c; +int *d; +void fn1() { + *d = 24; + for (; *d <= 65;) { + unsigned short *e = &a; + b = (a &= 0 <= 0) < (c ?: (*e %= *d)); + for (; *d <= 83;) + ; + } +}