From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 15258 invoked by alias); 17 Aug 2007 04:50:05 -0000 Received: (qmail 15173 invoked by uid 22791); 17 Aug 2007 04:50:04 -0000 X-Spam-Check-By: sourceware.org Received: from mail.codesourcery.com (HELO mail.codesourcery.com) (65.74.133.4) by sourceware.org (qpsmtpd/0.31) with ESMTP; Fri, 17 Aug 2007 04:50:02 +0000 Received: (qmail 7212 invoked from network); 17 Aug 2007 04:50:00 -0000 Received: from unknown (HELO ?192.168.0.3?) (mitchell@127.0.0.2) by mail.codesourcery.com with ESMTPA; 17 Aug 2007 04:50:00 -0000 Message-ID: <46C528F0.5040101@codesourcery.com> Date: Fri, 17 Aug 2007 04:50:00 -0000 From: Mark Mitchell User-Agent: Thunderbird 2.0.0.6 (Windows/20070728) MIME-Version: 1.0 To: Andrew Pinski CC: Sandra Loosemore , GCC Patches , Nigel Stephens , Guy Morrogh , David Ung , Thiemo Seufer , Richard Sandiford Subject: Re: PATCH: fine-tuning for can_store_by_pieces References: <46C3343A.5080407@codesourcery.com> In-Reply-To: Content-Type: text/plain; charset=UTF-8 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: 2007-08/txt/msg01075.txt.bz2 Andrew Pinski wrote: > On 8/15/07, Sandra Loosemore wrote: >> On MIPS we can get better code by having can_store_by_pieces >> differentiate between the cases where it's used for memset >> operations, and those where it's used to copy a string constant. >> This patch introduces new SET_RATIO and SET_BY_PIECES_P macros, >> with appropriate defaults to preserve the existing behavior. I >> checked other targets and made the ones that override the default >> STORE_BY_PIECES_P clone the same definition for SET_BY_PIECES_P. > It seems if you gave a testcase where this is profitable, it would be > better to judge this patch (and maybe a testcase for the testsuite > also). Sandra's now posted such a testcase, and it looks compelling to me. Certainly, setting all bytes to a single value is different from copying one series of bytes to another, so it doesn't surprise me that one wants different rules for the different cases. Sandra, I think the generic parts of the patch are OK, with a few nits: > ! void *constfundata, unsigned int align, int memsetp) MEMSETP should be a bool, not an int. And, when literal values are passed to it, they should be "true" and "false", not "1" and "0". > * config/sh/sh.h (SET_BY_PIECES_P): Clone from STORE_BY_PIECES_P. In the various places where you clone the macro, I think you should just do: #define SET_BY_PIECES_P(SIZE, ALIGN) STORE_BY_PIECES_P(SIZE, ALIGN) In fact, if you do that in expr.c, under #ifndef SET_BY_PIECES_P, I think you can void changing the other backends at all, as you'll automatically pick up their STORE_BY_PIECES_P definitions. If that all works, then the target-independent parts of the patch are OK with those changes. Thanks, -- Mark Mitchell CodeSourcery mark@codesourcery.com (650) 331-3385 x713