From patchwork Sun Jun 25 21:28:15 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Andrew Pinski X-Patchwork-Id: 780535 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 3wwlfj26VGz9s4q for ; Mon, 26 Jun 2017 07:28: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="d5PjipRT"; 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 :mime-version:in-reply-to:references:from:date:message-id :subject:to:content-type; q=dns; s=default; b=T72IZD/BrAvxeHyWXW FXEW0FNtuGFNmO1kcymy6eJ4BD/R3ibyieCqVOK2lMm7cuKVrS8syWvFe06QtpTV lVaA7ovf0n9yRoQ4hFec1wT4oDYs3vQcwFT+TT0w45TbC2ltfyF+jJF5WYa6IZKv Ozy2a0okRuGNPFu6ZBZHkW30g= 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 :mime-version:in-reply-to:references:from:date:message-id :subject:to:content-type; s=default; bh=ujjmBJj7tM2f/kp6tAJ2Obim 3q4=; b=d5PjipRTNaS6hIwNPa7mv5+a7cxX5anpjf7UuP0t2Sfl/L01yVExx2Gu tcNUU4JZRzq7GOUzpgYqemrPD8RvCwqx82QRwFc5g1IS92591fZpOl+Z01r0yqZ6 JtgJOn4EnuoD1ysQtBzp14GvhyiEALy07k5vK+5bSCDk5SHVlyk= Received: (qmail 12054 invoked by alias); 25 Jun 2017 21:28:21 -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 12037 invoked by uid 89); 25 Jun 2017 21:28:20 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-10.2 required=5.0 tests=AWL, BAYES_00, FREEMAIL_FROM, GIT_PATCH_2, GIT_PATCH_3, KAM_ASCII_DIVIDERS, RCVD_IN_DNSWL_NONE, SPF_PASS autolearn=ham version=3.3.2 spammy=Noticed, H*f:sk:Sn1m-pM X-HELO: mail-yw0-f169.google.com Received: from mail-yw0-f169.google.com (HELO mail-yw0-f169.google.com) (209.85.161.169) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Sun, 25 Jun 2017 21:28:18 +0000 Received: by mail-yw0-f169.google.com with SMTP id s127so3150651ywg.1 for ; Sun, 25 Jun 2017 14:28:17 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:in-reply-to:references:from:date :message-id:subject:to; bh=qKrY/VZWSVyad7y1E+n7yxVjwBM4gqbN5bei6FumhvM=; b=P8DU0LUQX3MalS+3GZLL8hsxqhsHatV6+qraR+HzLqRi7gO9aSfc8Sk4daYRKXybaT qgNte7nUQ/SVvhNdyXzoY0AP/cvJMnpoOPsfgQMMvWYIJhQgD39R13TtmRd6wsXUdzmE Yey09VZw4dG1qktjbCN6dVxfbZ59mqpp5joN7Qs53skh1XI/tOIwO8HUlM8w7re1dAdC +IC4MLUU70KVbJ8Agu7bG6JVDEqWfmsVhbA5P/uoqOhfRl16Pz9KWZTxhXUf72oP3JTw mDKA48i+bo+d82Ur1p5VPishFOWYDS5AmljPmF8oJ15JByQLnccPs0KoK9yB47FAPNji PB3Q== X-Gm-Message-State: AKS2vOwykDtD9aWTdby8S9As5K9PFhqgjH7aqXEx71fBCiX5yZE6KP25 IiONzTKN0wC3mOGwahhdjEStwtu3DSvj X-Received: by 10.13.230.212 with SMTP id p203mr13075849ywe.237.1498426095926; Sun, 25 Jun 2017 14:28:15 -0700 (PDT) MIME-Version: 1.0 Received: by 10.129.47.200 with HTTP; Sun, 25 Jun 2017 14:28:15 -0700 (PDT) In-Reply-To: References: From: Andrew Pinski Date: Sun, 25 Jun 2017 14:28:15 -0700 Message-ID: Subject: Re: [PATCH] Fold (a > 0 ? 1.0 : -1.0) into copysign (1.0, a) and a * copysign (1.0, a) into abs(a) To: GCC Patches X-IsSubscribed: yes On Sun, Jun 25, 2017 at 11:18 AM, Andrew Pinski wrote: > On Sun, Jun 25, 2017 at 1:28 AM, Marc Glisse wrote: >> +(for cmp (gt ge lt le) >> + outp (convert convert negate negate) >> + outn (negate negate convert convert) >> + /* Transform (X > 0.0 ? 1.0 : -1.0) into copysign(1, X). */ >> + /* Transform (X >= 0.0 ? 1.0 : -1.0) into copysign(1, X). */ >> + /* Transform (X < 0.0 ? 1.0 : -1.0) into copysign(1,-X). */ >> + /* Transform (X <= 0.0 ? 1.0 : -1.0) into copysign(1,-X). */ >> + (simplify >> + (cond (cmp @0 real_zerop) real_onep real_minus_onep) >> + (if (!HONOR_NANS (type) && !HONOR_SIGNED_ZEROS (type) >> + && types_match (type, TREE_TYPE (@0))) >> + (switch >> + (if (types_match (type, float_type_node)) >> + (BUILT_IN_COPYSIGNF { build_one_cst (type); } (outp @0))) >> + (if (types_match (type, double_type_node)) >> + (BUILT_IN_COPYSIGN { build_one_cst (type); } (outp @0))) >> + (if (types_match (type, long_double_type_node)) >> + (BUILT_IN_COPYSIGNL { build_one_cst (type); } (outp @0)))))) >> >> There is already a 1.0 of the right type in the input, it would be easier to >> reuse it in the output than build a new one. > > Right. Fixed. > >> >> Non-generic builtins like copysign are such a pain... We also end up missing >> the 128-bit case that way (pre-existing problem, not your patch). We seem to >> have a corresponding internal function, but apparently it is not used until >> expansion (well, maybe during vectorization). > > Yes I noticed that while working on a different patch related to > copysign; The generic version of a*copysign(1.0, b) [see the other > thread where the ARM folks started a patch for it; yes it was by pure > accident that I was working on this and really did not notice that > thread until yesterday]. > I was looking into a nice way of creating copysign without having to > do the switch but I could not find one. In the end I copied was done > already in a different location in match.pd; this is also the reason > why I had the build_one_cst there. > >> >> + /* Transform (X > 0.0 ? -1.0 : 1.0) into copysign(1,-X). */ >> + /* Transform (X >= 0.0 ? -1.0 : 1.0) into copysign(1,-X). */ >> + /* Transform (X < 0.0 ? -1.0 : 1.0) into copysign(1,X). */ >> + /* Transform (X <= 0.0 ? -1.0 : 1.0) into copysign(1,X). */ >> + (simplify >> + (cond (cmp @0 real_zerop) real_minus_onep real_onep) >> + (if (!HONOR_NANS (type) && !HONOR_SIGNED_ZEROS (type) >> + && types_match (type, TREE_TYPE (@0))) >> + (switch >> + (if (types_match (type, float_type_node)) >> + (BUILT_IN_COPYSIGNF { build_one_cst (type); } (outn @0))) >> + (if (types_match (type, double_type_node)) >> + (BUILT_IN_COPYSIGN { build_one_cst (type); } (outn @0))) >> + (if (types_match (type, long_double_type_node)) >> + (BUILT_IN_COPYSIGNL { build_one_cst (type); } (outn @0))))))) >> + >> +/* Transform X * copysign (1.0, X) into abs(X). */ >> +(simplify >> + (mult:c @0 (COPYSIGN real_onep @0)) >> + (if (!HONOR_NANS (type) && !HONOR_SIGNED_ZEROS (type)) >> + (abs @0))) >> >> I would have expected it do to the right thing for signed zero and qNaN. Can >> you describe a case where it would give the wrong result, or are the >> conditions just conservative? > > I was just being conservative; maybe too conservative but I was a bit > worried I could get it incorrect. > >> >> +/* Transform X * copysign (1.0, -X) into -abs(X). */ >> +(simplify >> + (mult:c @0 (COPYSIGN real_onep (negate @0))) >> + (if (!HONOR_NANS (type) && !HONOR_SIGNED_ZEROS (type)) >> + (negate (abs @0)))) >> + >> +/* Transform copysign (-1.0, X) into copysign (1.0, X). */ >> +(simplify >> + (COPYSIGN real_minus_onep @0) >> + (COPYSIGN { build_one_cst (type); } @0)) >> >> (simplify >> (COPYSIGN REAL_CST@0 @1) >> (if (REAL_VALUE_NEGATIVE (TREE_REAL_CST (@0))) >> (COPYSIGN (negate @0) @1))) >> ? Or does that create trouble with sNaN and only the 1.0 case is worth >> the trouble? > > No that is the correct way; I Noticed the other thread about copysign > had something similar as what should be done too. > > I will send out a new patch after testing soon. New patch. OK? Bootstrapped and tested on aarch64-linux-gnu with no regressions. Thanks, Andrew Pinski ChangeLog: * match.pd (X >/>=/ > Thanks, > Andrew > >> >> -- >> Marc Glisse Index: match.pd =================================================================== --- match.pd (revision 249626) +++ match.pd (working copy) @@ -155,6 +155,58 @@ || !COMPLEX_FLOAT_TYPE_P (type))) (negate @0))) +(for cmp (gt ge lt le) + outp (convert convert negate negate) + outn (negate negate convert convert) + /* Transform (X > 0.0 ? 1.0 : -1.0) into copysign(1, X). */ + /* Transform (X >= 0.0 ? 1.0 : -1.0) into copysign(1, X). */ + /* Transform (X < 0.0 ? 1.0 : -1.0) into copysign(1,-X). */ + /* Transform (X <= 0.0 ? 1.0 : -1.0) into copysign(1,-X). */ + (simplify + (cond (cmp @0 real_zerop) real_onep@1 real_minus_onep) + (if (!HONOR_NANS (type) && !HONOR_SIGNED_ZEROS (type) + && types_match (type, TREE_TYPE (@0))) + (switch + (if (types_match (type, float_type_node)) + (BUILT_IN_COPYSIGNF @1 (outp @0))) + (if (types_match (type, double_type_node)) + (BUILT_IN_COPYSIGN @1 (outp @0))) + (if (types_match (type, long_double_type_node)) + (BUILT_IN_COPYSIGNL @1 (outp @0)))))) + /* Transform (X > 0.0 ? -1.0 : 1.0) into copysign(1,-X). */ + /* Transform (X >= 0.0 ? -1.0 : 1.0) into copysign(1,-X). */ + /* Transform (X < 0.0 ? -1.0 : 1.0) into copysign(1,X). */ + /* Transform (X <= 0.0 ? -1.0 : 1.0) into copysign(1,X). */ + (simplify + (cond (cmp @0 real_zerop) real_minus_onep real_onep@1) + (if (!HONOR_NANS (type) && !HONOR_SIGNED_ZEROS (type) + && types_match (type, TREE_TYPE (@0))) + (switch + (if (types_match (type, float_type_node)) + (BUILT_IN_COPYSIGNF @1 (outn @0))) + (if (types_match (type, double_type_node)) + (BUILT_IN_COPYSIGN @1 (outn @0))) + (if (types_match (type, long_double_type_node)) + (BUILT_IN_COPYSIGNL @1 (outn @0))))))) + +/* Transform X * copysign (1.0, X) into abs(X). */ +(simplify + (mult:c @0 (COPYSIGN real_onep @0)) + (if (!HONOR_NANS (type) && !HONOR_SIGNED_ZEROS (type)) + (abs @0))) + +/* Transform X * copysign (1.0, -X) into -abs(X). */ +(simplify + (mult:c @0 (COPYSIGN real_onep (negate @0))) + (if (!HONOR_NANS (type) && !HONOR_SIGNED_ZEROS (type)) + (negate (abs @0)))) + +/* Transform copysign (CST, X) into copysign (ABS(CST), X). */ +(simplify + (COPYSIGN REAL_CST@0 @1) + (if (REAL_VALUE_NEGATIVE (TREE_REAL_CST (@0))) + (COPYSIGN (negate @0) @1))) + /* X * 1, X / 1 -> X. */ (for op (mult trunc_div ceil_div floor_div round_div exact_div) (simplify Index: testsuite/gcc.dg/tree-ssa/copy-sign-1.c =================================================================== --- testsuite/gcc.dg/tree-ssa/copy-sign-1.c (nonexistent) +++ testsuite/gcc.dg/tree-ssa/copy-sign-1.c (working copy) @@ -0,0 +1,35 @@ +/* { dg-options "-O2 -ffast-math -fdump-tree-gimple" } */ +/* { dg-do compile } */ +float f(float x) +{ + return (x > 0.f ? -1.f : 1.f); +} +float f1(float x) +{ + return (x > 0.f ? 1.f : -1.f); +} +float g(float x) +{ + return (x >= 0.f ? -1.f : 1.f); +} +float g1(float x) +{ + return (x >= 0.f ? 1.f : -1.f); +} +float h(float x) +{ + return (x < 0.f ? -1.f : 1.f); +} +float h1(float x) +{ + return (x < 0.f ? 1.f : -1.f); +} +float i(float x) +{ + return (x <= 0.f ? -1.f : 1.f); +} +float i1(float x) +{ + return (x <= 0.f ? 1.f : -1.f); +} +/* { dg-final { scan-tree-dump-times "copysign" 8 "gimple"} } */ Index: testsuite/gcc.dg/tree-ssa/copy-sign-2.c =================================================================== --- testsuite/gcc.dg/tree-ssa/copy-sign-2.c (nonexistent) +++ testsuite/gcc.dg/tree-ssa/copy-sign-2.c (working copy) @@ -0,0 +1,13 @@ +/* { dg-options "-O2 -ffast-math -fdump-tree-optimized" } */ +/* { dg-do compile } */ +float f(float x) +{ + float t = __builtin_copysignf (1.0f, x); + return x * t; +} +float f1(float x) +{ + float t = __builtin_copysignf (1.0f, -x); + return x * t; +} +/* { dg-final { scan-tree-dump-times "ABS" 2 "optimized"} } */ Index: testsuite/gcc.dg/tree-ssa/mult-abs-2.c =================================================================== --- testsuite/gcc.dg/tree-ssa/mult-abs-2.c (nonexistent) +++ testsuite/gcc.dg/tree-ssa/mult-abs-2.c (working copy) @@ -0,0 +1,35 @@ +/* { dg-options "-O2 -ffast-math -fdump-tree-gimple" } */ +/* { dg-do compile } */ +float f(float x) +{ + return x * (x > 0.f ? -1.f : 1.f); +} +float f1(float x) +{ + return x * (x > 0.f ? 1.f : -1.f); +} +float g(float x) +{ + return x * (x >= 0.f ? -1.f : 1.f); +} +float g1(float x) +{ + return x * (x >= 0.f ? 1.f : -1.f); +} +float h(float x) +{ + return x * (x < 0.f ? -1.f : 1.f); +} +float h1(float x) +{ + return x * (x < 0.f ? 1.f : -1.f); +} +float i(float x) +{ + return x * (x <= 0.f ? -1.f : 1.f); +} +float i1(float x) +{ + return x * (x <= 0.f ? 1.f : -1.f); +} +/* { dg-final { scan-tree-dump-times "ABS" 8 "gimple"} } */