From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 20063 invoked by alias); 29 Jun 2017 18:49:50 -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 20040 invoked by uid 89); 29 Jun 2017 18:49:49 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.4 required=5.0 tests=AWL,BAYES_00,FREEMAIL_FROM,RCVD_IN_DNSWL_NONE,RCVD_IN_SORBS_SPAM,SPF_PASS autolearn=no version=3.3.2 spammy= X-HELO: mail-oi0-f43.google.com Received: from mail-oi0-f43.google.com (HELO mail-oi0-f43.google.com) (209.85.218.43) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Thu, 29 Jun 2017 18:49:47 +0000 Received: by mail-oi0-f43.google.com with SMTP id c189so74216531oia.2 for ; Thu, 29 Jun 2017 11:49:47 -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:cc; bh=hXnb/q9ONttuH+pX2Up42aosHCBtUKUzr583FP4W/xg=; b=lMIYH/2HnQM5CVGlNZRyq05cPOWV+lcV/3Ebu4qF7LVXY1wsqoZR37QwR1j4NKLyPb iadCZV7yOKmXnw4PqnoibSj0CjeFbG553f0TQpfQVzCy14b8x8M9Rfg+RrcTZmQYveWL cNujcZjGpCyNsN2JzPFtaTdE2nVhqYFmjENTMlIfYDb1biKprcHvAhoV61DeTtoj5zr+ JbJM2NxZbS+kXXkSRZb5gpQwyraY+bgS/18Yh2b8e+6uwDMl0AD7H5TElsqi3mBzlonb Q/XWgVxzIWgqQ7EVBI3iM9gz1IfQOy7r+/jPxf8nybnXstf4v43zxyBsFSLUvt4IZhCw Px4A== X-Gm-Message-State: AKS2vOzj6AJ3VF/WTDd43m3iKFFlpHq+PlKaVjjHWUEvkd4LGpTVOfIB a4AROz+4NKCI+AUQZND3hD4McY757A== X-Received: by 10.202.84.214 with SMTP id i205mr5852702oib.158.1498762186182; Thu, 29 Jun 2017 11:49:46 -0700 (PDT) MIME-Version: 1.0 Received: by 10.74.141.84 with HTTP; Thu, 29 Jun 2017 11:49:45 -0700 (PDT) In-Reply-To: References: From: "H.J. Lu" Date: Thu, 29 Jun 2017 18:49:00 -0000 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: Andrew Pinski Cc: GCC Patches Content-Type: text/plain; charset="UTF-8" X-IsSubscribed: yes X-SW-Source: 2017-06/txt/msg02321.txt.bz2 On Sun, Jun 25, 2017 at 2:28 PM, Andrew Pinski wrote: > 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 >/>=/ (X * copysign (1.0, X)): New pattern. > (X * copysign (1.0, -X)): New pattern. > (copysign (-1.0, CST)): New pattern. > > testsuite/ChangeLog: > * gcc.dg/tree-ssa/copy-sign-1.c: New testcase. > * gcc.dg/tree-ssa/copy-sign-2.c: New testcase. > * gcc.dg/tree-ssa/mult-abs-2.c: New testcase. > This caused: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=81255 -- H.J.