From patchwork Fri Aug 17 14:04:04 2012 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Richard Earnshaw X-Patchwork-Id: 178211 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]) by ozlabs.org (Postfix) with SMTP id B70252C00A0 for ; Sat, 18 Aug 2012 00:04:37 +1000 (EST) Comment: DKIM? See http://www.dkim.org DKIM-Signature: v=1; a=rsa-sha1; c=relaxed/relaxed; d=gcc.gnu.org; s=default; x=1345817078; h=Comment: DomainKey-Signature:Received:Received:Received:Received:Received: Message-ID:Date:From:User-Agent:MIME-Version:To:Subject: Content-Type:Mailing-List:Precedence:List-Id:List-Unsubscribe: List-Archive:List-Post:List-Help:Sender:Delivered-To; bh=gvXVrn2 pdhgq31f5CjCriDoV9pY=; b=xUJYrirZJJWJ/wtxUe23TA1RDeOcqaDI2vGbiXz vUncOF4fcQ6sffKtiMVoIJ5f6Z5DFvMJHzcfM6iZp16ExouhZXg95VJnMAe19eLx BUvIFVr+UTf5sb1UcFh32bx3ihALtiLHAQLSsrmbEpUKKt29X28hiLSYyW0JbmF0 snzs= Comment: DomainKeys? See http://antispam.yahoo.com/domainkeys DomainKey-Signature: a=rsa-sha1; q=dns; c=nofws; s=default; d=gcc.gnu.org; h=Received:Received:X-SWARE-Spam-Status:X-Spam-Check-By:Received:Received:Received:Message-ID:Date:From:User-Agent:MIME-Version:To:Subject:X-MC-Unique:Content-Type:X-IsSubscribed:Mailing-List:Precedence:List-Id:List-Unsubscribe:List-Archive:List-Post:List-Help:Sender:Delivered-To; b=TKCNRzotYe41DWznfoFCFHZ1GdrynfCQxRRD1JgYc74pTAqnnBLI3m4pVsGYUG aZyEIRLuadENgx3VE9f7x6rgwio7QHvHvNHSAYH8gS1oK4LmRQpZ5uOr6ZM4Wpzj 7aS4mxC7orUxz/BCProclm4WLIS3iy6n2o4OCTfk5x8Yw=; Received: (qmail 8633 invoked by alias); 17 Aug 2012 14:04:31 -0000 Received: (qmail 8600 invoked by uid 22791); 17 Aug 2012 14:04:27 -0000 X-SWARE-Spam-Status: No, hits=-0.8 required=5.0 tests=AWL, BAYES_00, KHOP_RCVD_UNTRUST, RCVD_IN_DNSWL_LOW, TW_TM X-Spam-Check-By: sourceware.org Received: from service87.mimecast.com (HELO service87.mimecast.com) (91.220.42.44) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Fri, 17 Aug 2012 14:04:07 +0000 Received: from cam-owa2.Emea.Arm.com (fw-tnat.cambridge.arm.com [217.140.96.21]) by service87.mimecast.com; Fri, 17 Aug 2012 15:04:05 +0100 Received: from [10.1.69.67] ([10.1.255.212]) by cam-owa2.Emea.Arm.com with Microsoft SMTPSVC(6.0.3790.3959); Fri, 17 Aug 2012 15:05:50 +0100 Message-ID: <502E4F54.9040309@arm.com> Date: Fri, 17 Aug 2012 15:04:04 +0100 From: Richard Earnshaw User-Agent: Mozilla/5.0 (X11; Linux i686 on x86_64; rv:13.0) Gecko/20120614 Thunderbird/13.0.1 MIME-Version: 1.0 To: gcc-patches@gcc.gnu.org, Richard Sandiford , Andrew Stubbs Subject: [patch, tree-ssa] PR54295 Incorrect value extension in widening multiply-accumulate X-MC-Unique: 112081715040515901 X-IsSubscribed: yes 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 PR54295 shows that widening multiply-accumulate operations can end up with incorrect code. The path to the failure is as follows 1) Compiler detects a widening multiply operation and generates the correct widening-multiply operation. This involves stripping off the widening cast to leave the underlying operands. That is X = (c1) a * (c2) b => X = a w* b the type of w is picked based on the types of a and b; if both are signed a signed multiply-extend operation is used; if both are unsigned an unsigned multiply-extend is used. If they differ a signed/unsigned multiply is used, if it exists. If either a or b is a positive constant it is coerced, if possible, into the type of the other operand. 2) The compiler then notices that the result, X is used in an addition operation Y = X + d As X is a widening multiply, the compiler then looks inside it to try and generate a widening-multiply-accumulate operation. However, it passes the rewritten expression (X = a w* b) to is_widening_mult_p and that routine does not correctly check what type of multiply is being done: the code blindly tries to strip of an conversion operations. The failure happens when a is also the result of a cast operation, for example, being cast from an unsigned to an int. The compiler then re-formulates a signed multiply-extend into an unsigned multiply-extend. That is, if a = (int) e // typeof(e) == unsigned Then instead of Y = WIDEN_MULT_PLUS (a, b, d) we end up with Y = WIDEN_MULT_PLUS (e, b, d) The fix is to make is_widening_mult_p note that it has been called with a WIDEN_MULT_EXPR and rather than decompose the operands again, to simply extract the existing operands, which have already been formulated correctly for a widening multiply operation. PR tree-ssa/54295 * tree-ssa-math-opts.c (is_widening_mult_p): Short-circuit when the stmt is already a widening multiply. Testsuite PR tree-ssa/54295 * gcc.c-torture/execute/20120817-1.c: New test. OK to commit once testing has completed? --- tree-ssa-math-opts.c (revision 190462) +++ tree-ssa-math-opts.c (local) @@ -2039,6 +2039,25 @@ is_widening_mult_p (gimple stmt, && TREE_CODE (type) != FIXED_POINT_TYPE) return false; + /* If we already have a widening multiply expression, simply extract the + operands. */ + if (gimple_assign_rhs_code (stmt) == WIDEN_MULT_EXPR) + { + *rhs1_out = gimple_assign_rhs1 (stmt); + *rhs2_out = gimple_assign_rhs2 (stmt); + if (TREE_CODE (*rhs1_out) == INTEGER_CST) + *type1_out = TREE_TYPE (*rhs2_out); + else + *type1_out = TREE_TYPE (*rhs1_out); + + if (TREE_CODE (*rhs2_out) == INTEGER_CST) + *type2_out = TREE_TYPE (*rhs1_out); + else + *type2_out = TREE_TYPE (*rhs2_out); + + return true; + } + if (!is_widening_mult_rhs_p (type, gimple_assign_rhs1 (stmt), type1_out, rhs1_out)) return false; Index: 20120817-1.c =================================================================== --- 20120817-1.c (revision 0) +++ 20120817-1.c (revision 0) @@ -0,0 +1,14 @@ +typedef unsigned long long u64; +unsigned long foo = 0; +u64 f() __attribute__((noinline)); + +u64 f() { + return ((u64)40) + ((u64) 24) * (int)(foo - 1); +} + +int main () +{ + if (f () != 16) + abort (); + exit (0); +}