From patchwork Sat Jul 7 23:02:34 2012 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Hans-Peter Nilsson X-Patchwork-Id: 169618 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]) by ozlabs.org (Postfix) with SMTP id 4538C2C020A for ; Sun, 8 Jul 2012 09:03:06 +1000 (EST) Comment: DKIM? See http://www.dkim.org DKIM-Signature: v=1; a=rsa-sha1; c=relaxed/relaxed; d=gcc.gnu.org; s=default; x=1342306987; h=Comment: DomainKey-Signature:Received:Received:Received:Received:Received: Received:Received:Received:Received:Date:Message-Id:From:To:CC: In-reply-to:Subject:MIME-Version:Content-Type: Content-Transfer-Encoding:Mailing-List:Precedence:List-Id: List-Unsubscribe:List-Archive:List-Post:List-Help:Sender: Delivered-To; bh=hCyoFos/wbtdd8XU+aBDCfz0Rx0=; b=Csen5HFHeTFMSH0 xuHZAEldjoLuktbxjqki2/slzTnwNNrdvi34+BaBCSoAc/GUZRQQDyYyj40CiwRV aJsjEoMlidZBrQTGfPrKF+9LDCzriPN7+L9rlmeVra9J99PcoRKW3NONyses24Nj xK34qG67/6ZYmhceSluDp6S5wCD4= Comment: DomainKeys? See http://antispam.yahoo.com/domainkeys DomainKey-Signature: a=rsa-sha1; q=dns; c=nofws; s=default; d=gcc.gnu.org; h=Received:Received:X-SWARE-Spam-Status:X-Spam-Check-By:Received:Received:Received:Received:Received:Received:Received:Date:Message-Id:From:To:CC:In-reply-to:Subject:MIME-Version:Content-Type:Content-Transfer-Encoding:Mailing-List:Precedence:List-Id:List-Unsubscribe:List-Archive:List-Post:List-Help:Sender:Delivered-To; b=fZHRqJ/7AswGbHd2HmgtUyZEpfoMYdPEkJfDTgA3bN/ISjryNqlHcpYfwzFwjA 6cq8RSkvd4jf74XpzXBnLKe5ezpMmc85VLRS303mYSrnQFkoBcKlW2w7yHCxixXv oSl0IEPBEHdAlYl4pnGhgf58yIv0QfeQDGWX6kwIt/EeM=; Received: (qmail 8744 invoked by alias); 7 Jul 2012 23:03:01 -0000 Received: (qmail 8736 invoked by uid 22791); 7 Jul 2012 23:03:00 -0000 X-SWARE-Spam-Status: No, hits=-2.9 required=5.0 tests=AWL, BAYES_00, KHOP_THREADED, RCVD_IN_HOSTKARMA_NO, T_RP_MATCHES_RCVD X-Spam-Check-By: sourceware.org Received: from anubis.se.axis.com (HELO anubis.se.axis.com) (195.60.68.12) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Sat, 07 Jul 2012 23:02:40 +0000 Received: from localhost (localhost [127.0.0.1]) by anubis.se.axis.com (Postfix) with ESMTP id 23EE519DE2; Sun, 8 Jul 2012 01:02:36 +0200 (CEST) Received: from anubis.se.axis.com ([127.0.0.1]) by localhost (anubis.se.axis.com [127.0.0.1]) (amavisd-new, port 10024) with LMTP id LhDj83XZVgbV; Sun, 8 Jul 2012 01:02:35 +0200 (CEST) Received: from thoth.se.axis.com (thoth.se.axis.com [10.0.2.173]) by anubis.se.axis.com (Postfix) with ESMTP id ECEFE19DE1; Sun, 8 Jul 2012 01:02:34 +0200 (CEST) Received: from ignucius.se.axis.com (ignucius.se.axis.com [10.88.21.50]) by thoth.se.axis.com (Postfix) with ESMTP id D9A7D34081; Sun, 8 Jul 2012 01:02:34 +0200 (CEST) Received: from ignucius.se.axis.com (localhost [127.0.0.1]) by ignucius.se.axis.com (8.12.8p1/8.12.8/Debian-2woody1) with ESMTP id q67N2Ymt016921; Sun, 8 Jul 2012 01:02:34 +0200 Received: (from hp@localhost) by ignucius.se.axis.com (8.12.8p1/8.12.8/Debian-2woody1) id q67N2YEL016917; Sun, 8 Jul 2012 01:02:34 +0200 Date: Sun, 8 Jul 2012 01:02:34 +0200 Message-Id: <201207072302.q67N2YEL016917@ignucius.se.axis.com> From: Hans-Peter Nilsson To: rdsandiford@googlemail.com CC: gcc-patches@gcc.gnu.org In-reply-to: (message from Richard Sandiford on Wed, 9 May 2012 11:14:49 +0200) Subject: Fix gcc.dg/lower-subreg-1.c failure, revisited. MIME-Version: 1.0 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 > From: Richard Sandiford > Date: Wed, 9 May 2012 11:14:49 +0200 > Hans-Peter Nilsson writes: > >> From: Richard Sandiford > >> Date: Tue, 1 May 2012 16:46:38 +0200 > > > >> To repeat: as things stand, very few targets define proper rtx costs > >> for SET. > > > > IMHO it's wrong to start blaming targets when rtx_cost doesn't > > take the mode in account in the first place, for the default > > cost. (Well, except for the modes-tieable subreg special-case.) > > The targets where an operation in N * word_mode costs no more > > than one in word_mode, if there even is one, is a minority, > > let's adjust the defaults to that. > > I'll pass on approving or disapproving this patch, Here's an update. Could you please reconsider? I have to appeal to your sense of fairness: after all you were involved in the breaking patch. > but for the record: > a factor of word_mode seems a bit too simplistic. It's OK for moves > and logic ops, but addition of multiword modes is a bit more complicated. > Multiplication and division by multiword modes is even more so, of course. I adjusted multiplication and division to the more realistic O(n*n). That's just for show of course; we're comparing 2**2 to instead of 2*2 as the focus is on the double size. I did not add specific adjustments for arithmetic codes not already there, as I did not think such an adjustment to be appropriate. The intent is just to adapt the existing default costs to the change of focus due to the lower-subreg change. But if it's necessary for approval, I'm willing to fix that. > As things stand, rtx_cost is intentionally used for more than > just valid target insns. One of the main uses has always been > to decide whether it is better to implement multiplications by a > shift-and-add sequence, or whether to use multiplication instructions. > The associated expmed code has never tried to decide which shifts are > actually representable as matching rtl insns and which aren't. > The same goes for division-using-multiplication. > > So I think this patch (referring to the commit that caused the major gcc.dg/lower-subreg-1.c regression in May, not the predecessor to *this* patch) > is using rtx_cost according to its current > interface. Regardless of how "interface" is defined, that commit put much more focus on the mode and made it about necessary to handle SET, something that the default costs don't do without the patch below. > > Isn't the below better than doing virtually the same in each > > target's rtx_costs? > > FWIW, MIPS, SH and x86 all used something slightly different (and more > complicated). I imagine PowerPC and SPARC will too. So "each" seems > a bit strong. Just strong enough: this patch indeed would have fixed the gcc.dg/lower-subreg-1.c regression for MIPS and x86 (no regressions) that was present at revision 187212, and no doubt the predecessor patch too. (I had to add commits 187299 and 187582 for MIPS and I couldn't get a baseline with java or fortran enabled, thus --enable-languages=c,c++,lto,objc). Also similarly tested arm-eabi, sh-elf (though I couldn't repeat the regression for those at the baseline) and cris-elf; no regressions. Fixes PR53176 for cris-elf. Ok to commit? * rtlanal.c (rtx_cost): Adjust default cost for X with a UNITS_PER_WORD factor for all X according to the size of its mode, not just for SUBREGs with untieable modes. Handle SET. Use factor * factor for MULT, DIV, UDIV, MOD, UMOD. brgds, H-P Index: gcc/rtlanal.c =================================================================== --- gcc/rtlanal.c (revision 188403) +++ gcc/rtlanal.c (working copy) @@ -3755,10 +3755,17 @@ rtx_cost (rtx x, enum rtx_code outer_cod enum rtx_code code; const char *fmt; int total; + int factor; if (x == 0) return 0; + /* A size N times larger than UNITS_PER_WORD likely needs N times as + many insns, taking N times as long. */ + factor = GET_MODE_SIZE (GET_MODE (x)) / UNITS_PER_WORD; + if (factor == 0) + factor = 1; + /* Compute the default costs of certain things. Note that targetm.rtx_costs can override the defaults. */ @@ -3766,20 +3773,31 @@ rtx_cost (rtx x, enum rtx_code outer_cod switch (code) { case MULT: - total = COSTS_N_INSNS (5); + /* Multiplication has time-complexity O(N*N), where N is the + number of units (translated from digits) when using + schoolbook long multiplication. */ + total = factor * factor * COSTS_N_INSNS (5); break; case DIV: case UDIV: case MOD: case UMOD: - total = COSTS_N_INSNS (7); + /* Similarly, complexity for schoolbook long division. */ + total = factor * factor * COSTS_N_INSNS (7); break; case USE: /* Used in combine.c as a marker. */ total = 0; break; + case SET: + /* A SET doesn't have a mode, so let's look at the SET_DEST to get + the mode for the factor. */ + factor = GET_MODE_SIZE (GET_MODE (SET_DEST (x))) / UNITS_PER_WORD; + if (factor == 0) + factor = 1; + /* Pass through. */ default: - total = COSTS_N_INSNS (1); + total = factor * COSTS_N_INSNS (1); } switch (code) @@ -3792,8 +3810,7 @@ rtx_cost (rtx x, enum rtx_code outer_cod /* If we can't tie these modes, make this expensive. The larger the mode, the more expensive it is. */ if (! MODES_TIEABLE_P (GET_MODE (x), GET_MODE (SUBREG_REG (x)))) - return COSTS_N_INSNS (2 - + GET_MODE_SIZE (GET_MODE (x)) / UNITS_PER_WORD); + return COSTS_N_INSNS (2 + factor); break; default: