From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 99908 invoked by alias); 2 Jun 2015 08:53:26 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Received: (qmail 98809 invoked by uid 89); 2 Jun 2015 08:53:25 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.2 required=5.0 tests=AWL,BAYES_00,FREEMAIL_FROM,RCVD_IN_DNSWL_LOW,SPF_PASS autolearn=ham version=3.3.2 X-HELO: mail-wi0-f172.google.com Received: from mail-wi0-f172.google.com (HELO mail-wi0-f172.google.com) (209.85.212.172) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES128-GCM-SHA256 encrypted) ESMTPS; Tue, 02 Jun 2015 08:53:19 +0000 Received: by wibut5 with SMTP id ut5so62024192wib.1 for ; Tue, 02 Jun 2015 01:53:16 -0700 (PDT) X-Received: by 10.180.77.34 with SMTP id p2mr28534944wiw.22.1433235196069; Tue, 02 Jun 2015 01:53:16 -0700 (PDT) Received: from localhost ([95.144.14.193]) by mx.google.com with ESMTPSA id fm8sm20490724wib.9.2015.06.02.01.53.14 (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Tue, 02 Jun 2015 01:53:15 -0700 (PDT) From: Richard Sandiford To: Richard Biener Mail-Followup-To: Richard Biener ,Marek Polacek , GCC Patches , Jason Merrill , Joseph Myers , rdsandiford@googlemail.com Cc: Marek Polacek , GCC Patches , Jason Merrill , Joseph Myers Subject: Re: [C/C++ PATCH] Implement -Wshift-overflow (PR c++/55095) References: <20150525194816.GX27320@redhat.com> <877frnchxt.fsf@googlemail.com> Date: Tue, 02 Jun 2015 09:01:00 -0000 In-Reply-To: (Richard Biener's message of "Tue, 2 Jun 2015 10:13:28 +0200") Message-ID: <87wpzmbif9.fsf@googlemail.com> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/24.5 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain X-SW-Source: 2015-06/txt/msg00163.txt.bz2 Richard Biener writes: > On Mon, Jun 1, 2015 at 10:06 PM, Richard Sandiford > wrote: >> Marek Polacek writes: >>> + /* Left-hand operand must be signed. */ >>> + if (TYPE_UNSIGNED (type0)) >>> + return false; >>> + >>> + /* Compute the result in infinite precision math (sort of). */ >>> + widest_int w = wi::lshift (wi::to_widest (op0), wi::to_widest (op1)); >>> + unsigned int min_prec = wi::min_precision (w, SIGNED); >>> + /* Watch out for shifting a negative value. */ >>> + tree r = wide_int_to_tree (tree_int_cst_sgn (op0) >= 0 >>> + ? unsigned_type_for (type0) >>> + : type0, w); >>> + bool overflowed = wi::cmps (w, wi::to_widest (r)); >>> + if (overflowed && c_inhibit_evaluation_warnings == 0) >>> + warning_at (loc, OPT_Wshift_overflow, >>> + "result of %qE requires %u bits to represent, " >>> + "but %qT only has %u bits", >>> + build2_loc (loc, LSHIFT_EXPR, type0, op0, op1), >>> + min_prec, type0, TYPE_PRECISION (type0)); >>> + >>> + return overflowed; >> >> Yeah, this "sort of" is a bit worrying :-) Especially as the number >> of bits in a widest_int depends on the number of bits in the target's >> widest integer mode. E.g. for i386 it's forced to 128, but for ARM >> it's 512 (IIRC). >> >> Could you do the check based on the wi::min_precision of the unshifted >> value? I.e. see whether adding the shift amount to that gives a value >> greater than type's precision. > > You could always use a FIXED_WIDE_INT like VRP does for its overflow > detection stuff. That would work too, but why impose an arbitrary limit? Unless I'm missing something, the code above should be equivalent to: unsigned int min_prec = (wi::min_precision (op0, SIGNED) + TREE_INT_CST_LOW (op1)); bool overflowed = min_prec > TYPE_PRECISION (type0); if (overflowed && c_inhibit_evaluation_warnings == 0) warning_at (loc, OPT_Wshift_overflow, "result of %qE requires %u bits to represent, " "but %qT only has %u bits", build2_loc (loc, LSHIFT_EXPR, type0, op0, op1), min_prec, type0, TYPE_PRECISION (type0)); which seems simpler than anything involving wider precision. Thanks, Richard