From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 27556 invoked by alias); 17 May 2011 11:41:34 -0000 Received: (qmail 27542 invoked by uid 22791); 17 May 2011 11:41:32 -0000 X-SWARE-Spam-Status: No, hits=-1.6 required=5.0 tests=AWL,BAYES_00,TW_TM,T_RP_MATCHES_RCVD X-Spam-Check-By: sourceware.org Received: from e5.ny.us.ibm.com (HELO e5.ny.us.ibm.com) (32.97.182.145) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Tue, 17 May 2011 11:41:14 +0000 Received: from d01relay03.pok.ibm.com (d01relay03.pok.ibm.com [9.56.227.235]) by e5.ny.us.ibm.com (8.14.4/8.13.1) with ESMTP id p4HBEF7d009846 for ; Tue, 17 May 2011 07:14:15 -0400 Received: from d01av03.pok.ibm.com (d01av03.pok.ibm.com [9.56.224.217]) by d01relay03.pok.ibm.com (8.13.8/8.13.8/NCO v10.0) with ESMTP id p4HBfDbV121558 for ; Tue, 17 May 2011 07:41:13 -0400 Received: from d01av03.pok.ibm.com (loopback [127.0.0.1]) by d01av03.pok.ibm.com (8.14.4/8.13.1/NCO v10.0 AVout) with ESMTP id p4H7f10Z021155 for ; Tue, 17 May 2011 04:41:01 -0300 Received: from [9.65.173.222] (sig-9-65-173-222.mts.ibm.com [9.65.173.222]) by d01av03.pok.ibm.com (8.14.4/8.13.1/NCO v10.0 AVin) with ESMTP id p4H7ex9a021058; Tue, 17 May 2011 04:41:00 -0300 Subject: Re: [PATCH] Fix PR46728 (move pow/powi folds to tree phases) From: "William J. Schmidt" To: Richard Guenther Cc: gcc-patches@gcc.gnu.org In-Reply-To: References: <1305298365.4889.6.camel@L3G5336.ibm.com> <1305567053.5000.19.camel@L3G5336.ibm.com> Content-Type: text/plain Date: Tue, 17 May 2011 14:11:00 -0000 Message-Id: <1305632470.5000.29.camel@L3G5336.ibm.com> Mime-Version: 1.0 Content-Transfer-Encoding: 7bit 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 X-SW-Source: 2011-05/txt/msg01215.txt.bz2 On Tue, 2011-05-17 at 11:03 +0200, Richard Guenther wrote: > On Mon, May 16, 2011 at 7:30 PM, William J. Schmidt > wrote: > > Richi, thank you for the detailed review! > > > > I'll plan to move the power-series expansion into the existing IL walk > > during pass_cse_sincos. As part of this, I'll move > > tree_expand_builtin_powi and its subfunctions from builtins.c into > > tree-ssa-math-opts.c. I'll submit this as a separate patch. > > > > I will also stop attempting to make code generation match completely at > > -O0. If there are tests in the test suite that fail only at -O0 due to > > these changes, I'll modify the tests to require -O1 or higher. > > > > I understand that you'd prefer that I leave the existing > > canonicalization folds in place, and only un-canonicalize them during my > > new pass (now, during cse_sincos). Actually, that was my first approach > > to this issue. The problem that I ran into is that the various folds > > are not performed just by the front end, but can pop up later, after my > > pass is done. In particular, pass_fold_builtins will undo my changes, > > turning expressions involving roots back into expressions involving > > pow/powi. It wasn't clear to me whether the folds could kick in > > elsewhere as well, so I took the approach of shutting them down. I see > > now that this does lose some optimizations such as > > pow(sqrt(cbrx(x)),6.0), as you pointed out. > > Yeah, it's always a delicate balance between canonicalization > and optimal form for further optimization. Did you really see > sqrt(cbrt(x)) being transformed back to pow()? I would doubt that, > as on gimple the foldings that require two function calls to match > shouldn't trigger. Or do you see sqrt(x) turned into pow(x,0.5)? > I see that the vectorizer for example handles both pow(x,0.5) and > pow(x,2), so indeed that might happen. Yes, I was seeing sqrt(x) turned back to pow(x,0.5), and even x*x turning back into pow(x,2.0). I don't specifically recall the sqrt(cbrt(x)) case; you're probably right about that one. But I had several test cases break because of this. > > I think what we might want to do is limit what the generic > gimple fold_stmt folding does to function calls, also to avoid > building regular generic call statements again. But that might > be a bigger project and certainly should be done separately. > > So I'd say don't worry about this issue for the initial patch but > instead deal with it separately. Agreed... > > We also repeatedly thought about whether canonicalizing > everything to pow is a good idea or not, especially our > canonicalizing of x * x to pow (x, 2) leads to interesting > effects in some cases, as several passes do not handle > function calls very well. So I also thought about introducing > a POW_EXPR tree code that would be easier in this > regard and would be a more IL friendly canonical form > of the power-related functions. > > > Should I attempt to leave the folds in place, and screen out the > > particular cases that are causing trouble in pass_fold_builtins? Or is > > it too fragile to try to catch all places where folds occur? If there's > > a flag that indicates parsing is complete, I suppose I could disable > > individual folds once we're into the optimizer. I'd appreciate your > > guidance. > > Indeed restricting canonicalization to earlier passes would be the > way to go I think. I will think of the best way to achieve this. Thanks. I think we need to address this as part of this patch, unless you're willing to live with a number of broken test cases in the meanwhile. If I only do the un-canonicalization in the new pass and let some of the folds be re-done later, some will fail. I'll start experimenting and see how many. Bill > > Richard. > > > Thanks, > > Bill > > > > > >