From patchwork Tue Jun 28 10:47:30 2011 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Andrew Stubbs X-Patchwork-Id: 2353 Return-Path: X-Original-To: patchwork@peony.canonical.com Delivered-To: patchwork@peony.canonical.com Received: from fiordland.canonical.com (fiordland.canonical.com [91.189.94.145]) by peony.canonical.com (Postfix) with ESMTP id E162C23F08 for ; Tue, 28 Jun 2011 10:47:38 +0000 (UTC) Received: from mail-qw0-f52.google.com (mail-qw0-f52.google.com [209.85.216.52]) by fiordland.canonical.com (Postfix) with ESMTP id 9041DA187C1 for ; Tue, 28 Jun 2011 10:47:38 +0000 (UTC) Received: by qwb8 with SMTP id 8so45834qwb.11 for ; Tue, 28 Jun 2011 03:47:38 -0700 (PDT) Received: by 10.229.1.140 with SMTP id 12mr5526428qcf.118.1309258057727; Tue, 28 Jun 2011 03:47:37 -0700 (PDT) X-Forwarded-To: linaro-patchwork@canonical.com X-Forwarded-For: patch@linaro.org linaro-patchwork@canonical.com Delivered-To: patches@linaro.org Received: by 10.229.48.135 with SMTP id r7cs4862qcf; Tue, 28 Jun 2011 03:47:36 -0700 (PDT) Received: by 10.216.37.198 with SMTP id y48mr355029wea.100.1309258055429; Tue, 28 Jun 2011 03:47:35 -0700 (PDT) Received: from mail-wy0-f178.google.com (mail-wy0-f178.google.com [74.125.82.178]) by mx.google.com with ESMTPS id g82si149156wes.74.2011.06.28.03.47.35 (version=TLSv1/SSLv3 cipher=OTHER); Tue, 28 Jun 2011 03:47:35 -0700 (PDT) Received-SPF: pass (google.com: domain of andrew.stubbs@gmail.com designates 74.125.82.178 as permitted sender) client-ip=74.125.82.178; Authentication-Results: mx.google.com; spf=pass (google.com: domain of andrew.stubbs@gmail.com designates 74.125.82.178 as permitted sender) smtp.mail=andrew.stubbs@gmail.com; dkim=pass (test mode) header.i=@gmail.com Received: by wyf19 with SMTP id 19so58975wyf.37 for ; Tue, 28 Jun 2011 03:47:35 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=gamma; h=message-id:date:from:user-agent:mime-version:to:cc:subject :references:in-reply-to:content-type; bh=P5EfjOyqv12HD+1UIZltCVBQLQxMItiT5SmkUF0Yz1A=; b=XNY72pJfAWQR9kYol4k+Oax8X4ELUg0yI9sFnSRPnBwcElVFVEe+DXBLjZCefdQgZS KnDz4hnY2lDRFObFx65L05/5BkXmuRQtVAXIvTuxfXzaCVPkWPnUCQ/LYrP65ZD5OOqj I4GgwwynPsaPfpmTETbnoC3ZZILoLXUdfAlyQ= Received: by 10.227.167.129 with SMTP id q1mr6374520wby.101.1309258054902; Tue, 28 Jun 2011 03:47:34 -0700 (PDT) Received: from [192.168.0.100] (cpc2-hawk4-0-0-cust828.aztw.cable.virginmedia.com [82.32.123.61]) by mx.google.com with ESMTPS id ej7sm52548wbb.53.2011.06.28.03.47.32 (version=SSLv3 cipher=OTHER); Tue, 28 Jun 2011 03:47:33 -0700 (PDT) Message-ID: <4E09B142.4020402@codesourcery.com> Date: Tue, 28 Jun 2011 11:47:30 +0100 From: Andrew Stubbs User-Agent: Mozilla/5.0 (X11; U; Linux x86_64; en-US; rv:1.9.2.17) Gecko/20110516 Lightning/1.0b2 Thunderbird/3.1.10 MIME-Version: 1.0 To: Richard Guenther CC: gcc-patches@gcc.gnu.org, patches@linaro.org Subject: Re: [PATCH (3/7)] Widening multiply-and-accumulate pattern matching References: <4E034EF2.3070503@codesourcery.com> <4E03504B.9060305@codesourcery.com> <4E044559.5000105@linaro.org> <1A77B5B39081C241A68E6CF16983025F020906F6@EU1-MAIL.mgc.mentorg.com> In-Reply-To: On 24/06/11 16:47, Richard Guenther wrote: >> I can certainly add checks to make sure that the skipped operations >> > actually don't make any important changes to the value, but do I need to? > Yes. OK, how about this patch? I've added checks to make sure the value is not truncated at any point. I've also changed the test cases to address Janis' comments. Andrew 2011-06-28 Andrew Stubbs gcc/ * gimple.h (tree_ssa_harmless_type_conversion): New prototype. (tree_ssa_strip_harmless_type_conversions): New prototype. (harmless_type_conversion_p): New prototype. * tree-ssa-math-opts.c (convert_plusminus_to_widen): Look for multiply statement beyond no-op conversion statements. * tree-ssa.c (harmless_type_conversion_p): New function. (tree_ssa_harmless_type_conversion): New function. (tree_ssa_strip_harmless_type_conversions): New function. gcc/testsuite/ * gcc.target/arm/wmul-5.c: New file. * gcc.target/arm/no-wmla-1.c: New file. --- a/gcc/gimple.h +++ b/gcc/gimple.h @@ -1090,8 +1090,11 @@ extern bool validate_gimple_arglist (const_gimple, ...); /* In tree-ssa.c */ extern bool tree_ssa_useless_type_conversion (tree); +extern bool tree_ssa_harmless_type_conversion (tree); extern tree tree_ssa_strip_useless_type_conversions (tree); +extern tree tree_ssa_strip_harmless_type_conversions (tree); extern bool useless_type_conversion_p (tree, tree); +extern bool harmless_type_conversion_p (tree, tree); extern bool types_compatible_p (tree, tree); /* Return the code for GIMPLE statement G. */ --- /dev/null +++ b/gcc/testsuite/gcc.target/arm/no-wmla-1.c @@ -0,0 +1,11 @@ +/* { dg-do compile } */ +/* { dg-options "-O2 -march=armv7-a" } */ + +int +foo (int a, short b, short c) +{ + int bc = b * c; + return a + (short)bc; +} + +/* { dg-final { scan-assembler "mul" } } */ --- /dev/null +++ b/gcc/testsuite/gcc.target/arm/wmul-5.c @@ -0,0 +1,10 @@ +/* { dg-do compile } */ +/* { dg-options "-O2 -march=armv7-a" } */ + +long long +foo (long long a, char *b, char *c) +{ + return a + *b * *c; +} + +/* { dg-final { scan-assembler "umlal" } } */ --- a/gcc/tree-ssa-math-opts.c +++ b/gcc/tree-ssa-math-opts.c @@ -2117,23 +2117,19 @@ convert_plusminus_to_widen (gimple_stmt_iterator *gsi, gimple stmt, rhs1 = gimple_assign_rhs1 (stmt); rhs2 = gimple_assign_rhs2 (stmt); - if (TREE_CODE (rhs1) == SSA_NAME) - { - rhs1_stmt = SSA_NAME_DEF_STMT (rhs1); - if (is_gimple_assign (rhs1_stmt)) - rhs1_code = gimple_assign_rhs_code (rhs1_stmt); - } - else + if (TREE_CODE (rhs1) != SSA_NAME + || TREE_CODE (rhs2) != SSA_NAME) return false; - if (TREE_CODE (rhs2) == SSA_NAME) - { - rhs2_stmt = SSA_NAME_DEF_STMT (rhs2); - if (is_gimple_assign (rhs2_stmt)) - rhs2_code = gimple_assign_rhs_code (rhs2_stmt); - } - else - return false; + rhs1 = tree_ssa_strip_harmless_type_conversions (rhs1); + rhs1_stmt = SSA_NAME_DEF_STMT (rhs1); + if (is_gimple_assign (rhs1_stmt)) + rhs1_code = gimple_assign_rhs_code (rhs1_stmt); + + rhs2 = tree_ssa_strip_harmless_type_conversions(rhs2); + rhs2_stmt = SSA_NAME_DEF_STMT (rhs2); + if (is_gimple_assign (rhs2_stmt)) + rhs2_code = gimple_assign_rhs_code (rhs2_stmt); if (code == PLUS_EXPR && rhs1_code == MULT_EXPR) { --- a/gcc/tree-ssa.c +++ b/gcc/tree-ssa.c @@ -1484,6 +1484,33 @@ useless_type_conversion_p (tree outer_type, tree inner_type) return false; } +/* Return true if the conversion from INNER_TYPE to OUTER_TYPE will + not alter the arithmetic meaning of a type, otherwise return false. + + For example, widening an integer type leaves the value unchanged, + but narrowing an integer type can cause truncation. + + Note that switching between signed and unsigned modes doesn't change + the underlying representation, and so is harmless. + + This function is not yet a complete definition of what is harmless + but should reject everything that is not. */ + +bool +harmless_type_conversion_p (tree outer_type, tree inner_type) +{ + /* If it's useless, it's also harmless. */ + if (useless_type_conversion_p (outer_type, inner_type)) + return true; + + if (INTEGRAL_TYPE_P (inner_type) + && INTEGRAL_TYPE_P (outer_type) + && TYPE_PRECISION (inner_type) <= TYPE_PRECISION (outer_type)) + return true; + + return false; +} + /* Return true if a conversion from either type of TYPE1 and TYPE2 to the other is not required. Otherwise return false. */ @@ -1515,6 +1542,29 @@ tree_ssa_useless_type_conversion (tree expr) return false; } +/* Return true if EXPR is a harmless type conversion, otherwise return + false. */ + +bool +tree_ssa_harmless_type_conversion (tree expr) +{ + gimple stmt; + + if (TREE_CODE (expr) != SSA_NAME) + return false; + + stmt = SSA_NAME_DEF_STMT (expr); + + if (!is_gimple_assign (stmt)) + return false; + + if (!CONVERT_EXPR_CODE_P (gimple_assign_rhs_code (stmt))) + return false; + + return harmless_type_conversion_p (TREE_TYPE (gimple_assign_lhs (stmt)), + TREE_TYPE (gimple_assign_rhs1 (stmt))); +} + /* Strip conversions from EXP according to tree_ssa_useless_type_conversion and return the resulting expression. */ @@ -1527,6 +1577,18 @@ tree_ssa_strip_useless_type_conversions (tree exp) return exp; } +/* Strip conversions from EXP according to + tree_ssa_harmless_type_conversion and return the resulting + expression. */ + +tree +tree_ssa_strip_harmless_type_conversions (tree exp) +{ + while (tree_ssa_harmless_type_conversion (exp)) + exp = gimple_assign_rhs1 (SSA_NAME_DEF_STMT (exp)); + return exp; +} + /* Internal helper for walk_use_def_chains. VAR, FN and DATA are as described in walk_use_def_chains.