From patchwork Fri May 31 15:02:38 2013 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Tobias Burnus X-Patchwork-Id: 247971 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 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (Client CN "localhost", Issuer "www.qmailtoaster.com" (not verified)) by ozlabs.org (Postfix) with ESMTPS id 0DB662C0300 for ; Sat, 1 Jun 2013 01:02:49 +1000 (EST) DomainKey-Signature: a=rsa-sha1; c=nofws; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender :message-id:date:from:mime-version:to:cc:subject:references :in-reply-to:content-type; q=dns; s=default; b=Y3aaVETACQeNWyxKs 1kC2smbuUC4lK4j5QtEHKxBojRo59gIZF6UThIxs2O1QewrjRvBtb3Ss18YRrHcS xD2NvdQzfAQEYaJaa/azoHpL9juGBczM01YY47/rDKTGyNBjGmMGrMd1QytUyEHj 6Jo2Ze5OGa+gIobTgMmwwteHaQ= 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 :message-id:date:from:mime-version:to:cc:subject:references :in-reply-to:content-type; s=default; bh=yYeXhem9WVFPDFFVDgz4IjY G7jQ=; b=c+rdoa37/KGle2t+0IYZePgekTcwp3WddWgzBj5OHlhs1ZqohBLPNOs 9yxKp2lY5XdApgKcoVWCvX3n9r37mj/UtPz4uWx9/BTK9p/4Sl4aA6QMXlr1aO0P LSIrWXkBO25EWG4WoKdvJYKCr8Bzd/rDVAooFPY7jdjDY58iMWBg= Received: (qmail 23672 invoked by alias); 31 May 2013 15:02:43 -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 23662 invoked by uid 89); 31 May 2013 15:02:43 -0000 X-Spam-SWARE-Status: No, score=-2.9 required=5.0 tests=AWL, BAYES_00, KHOP_THREADED, RCVD_IN_DNSWL_NONE autolearn=ham version=3.3.1 Received: from mx01.qsc.de (HELO mx01.qsc.de) (213.148.129.14) by sourceware.org (qpsmtpd/0.84/v0.84-167-ge50287c) with ESMTP; Fri, 31 May 2013 15:02:42 +0000 Received: from archimedes.net-b.de (port-92-195-106-97.dynamic.qsc.de [92.195.106.97]) by mx01.qsc.de (Postfix) with ESMTP id 6674B3CE33; Fri, 31 May 2013 17:02:38 +0200 (CEST) Message-ID: <51A8BB8E.9050905@net-b.de> Date: Fri, 31 May 2013 17:02:38 +0200 From: Tobias Burnus User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:17.0) Gecko/20130510 Thunderbird/17.0.6 MIME-Version: 1.0 To: Richard Biener CC: Jeff Law , gcc patches Subject: Re: PR57073 - Optimize __builtin_powif (-1.0, k) to k & 1 ? -1.0 : 1.0 References: <51A79580.2010001@net-b.de> <51A79A25.6080607@redhat.com> <51A7B8D7.2010001@net-b.de> <51A7BC87.1030801@redhat.com> In-Reply-To: X-Virus-Found: No Am 31.05.2013 10:24, schrieb Richard Biener: > On Thu, May 30, 2013 at 10:54 PM, Jeff Law wrote: >> Don't worry about it. The patch is good as-is. > Why sink the !host_integerp check? Please keep it where it is now. > Answer: Because it doesn't work. And if I had a cup of coffee and didn't mess up my regtesting (by excluding the newly added test case), I had also seen that. The very old code had (assume: powi(x,n)): if (n is not a constant) break; expand powi to "n" multiplications. The original patch changed it to: if (n is a not constant and x == -1) result = n & 1 ? -1.0 : 1.0 else { if (n is not a constant) break; expand powi to "n" multiplications. } Thus, if one moves up the condition if (n is not a constant) break; the newly added code becomes unreachable. However, I think the code is more readable if one simply removes the "&& !host_integerp (arg1,0)" from the x==1 case. Due to fold_builtin_powi having x==-1 and n == const should not happen - and if, the "n & 1 ? -1.0 : 1.0" is also not worse than an expanded multiplication (if n is large). [Alternatively, one can also keep (re-add) the "&& !host_integerp (arg1,0)".] OK? (After successful bootstrap and regtesting.) Tobias diff --git a/gcc/tree-ssa-math-opts.c b/gcc/tree-ssa-math-opts.c index b4de411..e9c32b3 100644 --- a/gcc/tree-ssa-math-opts.c +++ b/gcc/tree-ssa-math-opts.c @@ -1447,9 +1447,6 @@ execute_cse_sincos (void) arg1 = gimple_call_arg (stmt, 1); loc = gimple_location (stmt); - if (!host_integerp (arg1, 0)) - break; - if (real_minus_onep (arg0)) { tree t0, t1, cond, one, minus_one; @@ -1477,6 +1474,9 @@ execute_cse_sincos (void) } else { + if (!host_integerp (arg1, 0)) + break; + n = TREE_INT_CST_LOW (arg1); result = gimple_expand_builtin_powi (&gsi, loc, arg0, n); }