From patchwork Fri Feb 12 16:34:21 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jakub Jelinek X-Patchwork-Id: 582227 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 0C9281401C7 for ; Sat, 13 Feb 2016 03:34:40 +1100 (AEDT) Authentication-Results: ozlabs.org; dkim=pass (1024-bit key; unprotected) header.d=gcc.gnu.org header.i=@gcc.gnu.org header.b=idsBaloC; 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:date :from:to:cc:subject:message-id:reply-to:references:mime-version :content-type:in-reply-to; q=dns; s=default; b=b9yutnso+VABm5WYU Uv5MxVTwOXdgODrQ3GlXvH/WBdP0GjfGaDym0SRpDk5SP+k2mwsa0h2tTFeawlp7 h0a8hdEbNIgVCbnvaoETHk4GU8EJ5loAlGRU2jkyXyN6M9JjkgfN1/aqsdg8ppkJ b5AMY8MOpwjwp44QCtCCIuyEhw= 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:date :from:to:cc:subject:message-id:reply-to:references:mime-version :content-type:in-reply-to; s=default; bh=99Z5RtKTVHrLJ8YbEt8OMdB Xq28=; b=idsBaloCTQvjx6+o7tjjeRfaUqYo9t629FVWK5+ntgpw1I7EB8uZKGX TpDKikGlXnhmgZEuA8qeKwnTanjdUc3MlWzZAB71tNZUDAA2VuUxnUsULZLRLaoS l/Zo+1zGk0/iRxevZACccZB06CZl+v0lCIV8uC0cUQT4Q7Q9p1uE= Received: (qmail 2875 invoked by alias); 12 Feb 2016 16:34:30 -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 2860 invoked by uid 89); 12 Feb 2016 16:34:29 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.0 required=5.0 tests=BAYES_00, RP_MATCHES_RCVD, SPF_HELO_PASS autolearn=ham version=3.3.2 spammy=138 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, 12 Feb 2016 16:34:28 +0000 Received: from int-mx13.intmail.prod.int.phx2.redhat.com (int-mx13.intmail.prod.int.phx2.redhat.com [10.5.11.26]) by mx1.redhat.com (Postfix) with ESMTPS id 6E87219D4CD; Fri, 12 Feb 2016 16:34:27 +0000 (UTC) Received: from tucnak.zalov.cz ([10.3.113.11]) by int-mx13.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id u1CGYOFM017868 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=NO); Fri, 12 Feb 2016 11:34:26 -0500 Received: from tucnak.zalov.cz (localhost [127.0.0.1]) by tucnak.zalov.cz (8.15.2/8.15.2) with ESMTP id u1CGYMP9024731; Fri, 12 Feb 2016 17:34:23 +0100 Received: (from jakub@localhost) by tucnak.zalov.cz (8.15.2/8.15.2/Submit) id u1CGYL9v024730; Fri, 12 Feb 2016 17:34:21 +0100 Date: Fri, 12 Feb 2016 17:34:21 +0100 From: Jakub Jelinek To: Bernd Schmidt Cc: Richard Biener , gcc-patches@gcc.gnu.org Subject: Re: [PATCH] Fix PR69771, bogus CONST_INT during shift expansion Message-ID: <20160212163421.GT3017@tucnak.redhat.com> Reply-To: Jakub Jelinek References: <20160212103203.GM3017@tucnak.redhat.com> <56BDCCCF.2030601@redhat.com> <20160212122438.GP3017@tucnak.redhat.com> <20160212132813.GR3017@tucnak.redhat.com> <56BDEA17.7090808@redhat.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <56BDEA17.7090808@redhat.com> User-Agent: Mutt/1.5.24 (2015-08-30) X-IsSubscribed: yes On Fri, Feb 12, 2016 at 03:20:07PM +0100, Bernd Schmidt wrote: > >>- mode1 = GET_MODE (xop1) != VOIDmode ? GET_MODE (xop1) : mode; > >>+ mode1 = GET_MODE (xop1) != VOIDmode ? GET_MODE (xop1) : mode1; > >> if (xmode1 != VOIDmode && xmode1 != mode1) > >> { > > Here, things aren't so clear, and the fact that the mode1 calculation now > differs from the mode0 one may be overlooked by someone in the future. > > Rather than use codes like "mode variable is VOIDmode", I'd prefer to use > booleans with descriptive names, like "op1_may_need_conversion". So do you prefer e.g. following? Bootstrapped/regtested on x86_64-linux and i686-linux. 2016-02-12 Jakub Jelinek PR rtl-optimization/69764 PR rtl-optimization/69771 * optabs.c (expand_binop_directly): For shift_optab_p, force convert_modes with VOIDmode if xop1 has VOIDmode. * c-c++-common/pr69764.c: New test. * gcc.dg/torture/pr69771.c: New testcase. Jakub --- gcc/optabs.c.jj 2016-02-12 00:50:30.410240366 +0100 +++ gcc/optabs.c 2016-02-12 10:33:32.743492532 +0100 @@ -993,6 +993,7 @@ expand_binop_directly (machine_mode mode bool commutative_p; rtx_insn *pat; rtx xop0 = op0, xop1 = op1; + bool canonicalize_op1 = false; /* If it is a commutative operator and the modes would match if we would swap the operands, we can save the conversions. */ @@ -1006,6 +1007,11 @@ expand_binop_directly (machine_mode mode xop0 = avoid_expensive_constant (xmode0, binoptab, 0, xop0, unsignedp); if (!shift_optab_p (binoptab)) xop1 = avoid_expensive_constant (xmode1, binoptab, 1, xop1, unsignedp); + else + /* Shifts and rotates often use a different mode for op1 from op0; + for VOIDmode constants we don't know the mode, so force it + to be canonicalized using convert_modes. */ + canonicalize_op1 = true; /* In case the insn wants input operands in modes different from those of the actual operands, convert the operands. It would @@ -1020,7 +1026,8 @@ expand_binop_directly (machine_mode mode mode0 = xmode0; } - mode1 = GET_MODE (xop1) != VOIDmode ? GET_MODE (xop1) : mode; + mode1 = (GET_MODE (xop1) != VOIDmode || canonicalize_op1) + ? GET_MODE (xop1) : mode; if (xmode1 != VOIDmode && xmode1 != mode1) { xop1 = convert_modes (xmode1, mode1, xop1, unsignedp); --- gcc/testsuite/c-c++-common/pr69764.c.jj 2016-02-12 10:27:34.522587995 +0100 +++ gcc/testsuite/c-c++-common/pr69764.c 2016-02-12 10:27:34.522587995 +0100 @@ -0,0 +1,38 @@ +/* PR rtl-optimization/69764 */ +/* { dg-do compile { target int32plus } } */ + +unsigned char +fn1 (unsigned char a) +{ + return a >> ~6; /* { dg-warning "right shift count is negative" } */ +} + +unsigned short +fn2 (unsigned short a) +{ + return a >> ~6; /* { dg-warning "right shift count is negative" } */ +} + +unsigned int +fn3 (unsigned int a) +{ + return a >> ~6; /* { dg-warning "right shift count is negative" } */ +} + +unsigned char +fn4 (unsigned char a) +{ + return a >> 0xff03; /* { dg-warning "right shift count >= width of type" } */ +} + +unsigned short +fn5 (unsigned short a) +{ + return a >> 0xff03; /* { dg-warning "right shift count >= width of type" } */ +} + +unsigned int +fn6 (unsigned int a) +{ + return a >> 0xff03; /* { dg-warning "right shift count >= width of type" } */ +} --- gcc/testsuite/gcc.dg/torture/pr69771.c.jj 2016-02-12 10:27:34.522587995 +0100 +++ gcc/testsuite/gcc.dg/torture/pr69771.c 2016-02-12 10:27:34.522587995 +0100 @@ -0,0 +1,12 @@ +/* PR rtl-optimization/69771 */ +/* { dg-do compile } */ + +unsigned char a = 5, c; +unsigned short b = 0; +unsigned d = 0x76543210; + +void +foo (void) +{ + c = d >> ~(a || ~b); /* { dg-warning "shift count is negative" } */ +}