From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 39192 invoked by alias); 25 Jun 2017 18:18:07 -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 39174 invoked by uid 89); 25 Jun 2017 18:18:06 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.5 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=Noticed, H*f:sk:Sn1m-pM, yesterday X-HELO: mail-yw0-f182.google.com Received: from mail-yw0-f182.google.com (HELO mail-yw0-f182.google.com) (209.85.161.182) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Sun, 25 Jun 2017 18:18:05 +0000 Received: by mail-yw0-f182.google.com with SMTP id s127so2503838ywg.1 for ; Sun, 25 Jun 2017 11:18:05 -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=UeKUnjJrEtNWNXeLzb6wkq6xwLRHio83SUDuasXxRbc=; b=nkdih9MSQYrRdxxc3Gk4K+bTyaZSbnT6FtzCoiEXYB+SLi6tcgeGk/U38f5aeDrISn yfG0FOorMmfQ1Q9LpdAflQz0OGNNhhVvpCCl0LeEDgyNhX6rVa4otAOgJLhYvxrH3UUg FcRaYTWNYmnCWEnKV9gKpmPmVKZx8m/DG2tGUfQcAHbILzKPjx/fYTC6Ymmj6rDZ2uAx LJMFLytZwnWjgL6ggnd1la9BURy8rPRq5idJm0ps/UuOw5EkZ2wKiKl5OHllAIxs+qBQ RER0OZm3pliNm1mSHW428AeZhYDY8+kR6SEIYs1mm9yA+RB2dSVL049+EzCs98DAoDMN 8vxA== X-Gm-Message-State: AKS2vOyUWjDeDcjBm36yh0okzmjtv5oo8d2q4Fc35dE8O2E0BHapXcsW OqEixPGLFa/dEDvAaNEyAHrYIQy6iQ== X-Received: by 10.13.236.131 with SMTP id v125mr14293810ywe.77.1498414683203; Sun, 25 Jun 2017 11:18:03 -0700 (PDT) MIME-Version: 1.0 Received: by 10.129.47.200 with HTTP; Sun, 25 Jun 2017 11:18:02 -0700 (PDT) In-Reply-To: References: From: Andrew Pinski Date: Sun, 25 Jun 2017 18:18: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: GCC Patches Content-Type: text/plain; charset="UTF-8" X-IsSubscribed: yes X-SW-Source: 2017-06/txt/msg01867.txt.bz2 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. Thanks, Andrew > > -- > Marc Glisse