From patchwork Mon Feb 11 15:43:18 2013 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Christophe Lyon X-Patchwork-Id: 14734 Return-Path: X-Original-To: patchwork@peony.canonical.com Delivered-To: patchwork@peony.canonical.com Received: from fiordland.canonical.com (fiordland.canonical.com [91.189.94.145]) by peony.canonical.com (Postfix) with ESMTP id BE0B223E2E for ; Mon, 11 Feb 2013 15:43:21 +0000 (UTC) Received: from mail-ve0-f172.google.com (mail-ve0-f172.google.com [209.85.128.172]) by fiordland.canonical.com (Postfix) with ESMTP id 41DCFA18480 for ; Mon, 11 Feb 2013 15:43:21 +0000 (UTC) Received: by mail-ve0-f172.google.com with SMTP id cz11so5140381veb.31 for ; Mon, 11 Feb 2013 07:43:20 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20120113; h=x-received:x-forwarded-to:x-forwarded-for:delivered-to:x-received :received-spf:mime-version:x-received:in-reply-to:references:date :message-id:subject:from:to:cc:content-type:x-gm-message-state; bh=lfG6UIwGEinsl9DflEJJ0eSEqMpy0gL0CTfw2XmBfAU=; b=dp73Hsj3+LbGnj30DgQbO7yiRPHb+mfR5qXVqSlopXeOEGzZiKrhyPLAmMTFiMMJ3J fdJvNrPh3/SCDItxdy3EiNkIkBvc0xPhq4UdzsYhFkpfwkTwVXbY/grH8cqtJd28chbr 5IRzoFMywV1w1ox4FvyFsnvtlXrSvF4iTDZDRZAPweRmaUBp9KvfefTYke/Ltkvk7MGA KBXl9GydxX+PP+oly7PzRRfZeSz9FdM8JPIVTyHy/+i8OGYoZbV+BCPKe4S6EnFhBdpY u9s8kd1G5pOGg/MHZ4Y39F/2umlEfGDEkzm1i7TKrnwoxLud9SUJ0lfZXX3mNrXetcjh kNBQ== X-Received: by 10.58.76.193 with SMTP id m1mr1535686vew.4.1360597400749; Mon, 11 Feb 2013 07:43:20 -0800 (PST) X-Forwarded-To: linaro-patchwork@canonical.com X-Forwarded-For: patch@linaro.org linaro-patchwork@canonical.com Delivered-To: patches@linaro.org Received: by 10.58.252.8 with SMTP id zo8csp103423vec; Mon, 11 Feb 2013 07:43:19 -0800 (PST) X-Received: by 10.182.226.103 with SMTP id rr7mr10796234obc.76.1360597399208; Mon, 11 Feb 2013 07:43:19 -0800 (PST) Received: from mail-ob0-f182.google.com (mail-ob0-f182.google.com [209.85.214.182]) by mx.google.com with ESMTPS id w6si37415181obc.117.2013.02.11.07.43.18 (version=TLSv1 cipher=ECDHE-RSA-RC4-SHA bits=128/128); Mon, 11 Feb 2013 07:43:19 -0800 (PST) Received-SPF: neutral (google.com: 209.85.214.182 is neither permitted nor denied by best guess record for domain of christophe.lyon@linaro.org) client-ip=209.85.214.182; Authentication-Results: mx.google.com; spf=neutral (google.com: 209.85.214.182 is neither permitted nor denied by best guess record for domain of christophe.lyon@linaro.org) smtp.mail=christophe.lyon@linaro.org Received: by mail-ob0-f182.google.com with SMTP id va7so6205522obc.13 for ; Mon, 11 Feb 2013 07:43:18 -0800 (PST) MIME-Version: 1.0 X-Received: by 10.60.7.129 with SMTP id j1mr10799200oea.54.1360597398745; Mon, 11 Feb 2013 07:43:18 -0800 (PST) Received: by 10.60.35.202 with HTTP; Mon, 11 Feb 2013 07:43:18 -0800 (PST) In-Reply-To: <5118CE98.4040802@arm.com> References: <5118CE98.4040802@arm.com> Date: Mon, 11 Feb 2013 16:43:18 +0100 Message-ID: Subject: Re: [PATCH][ARM] Implement vectorizer cost hooks From: Christophe Lyon To: Richard Earnshaw Cc: "gcc-patches@gcc.gnu.org" , Patch Tracking X-Gm-Message-State: ALoCoQkfIk0YQGa9PyyuXQjRT8d7+8mWQO6HU8okNh1UgG1IusQxUYiOkEYKj9ApY12Wvp4nJjwH Richard, Thanks for your comments. Here a new version with the changes you suggested. Christophe On 11 February 2013 11:57, Richard Earnshaw wrote: > On 05/02/13 18:18, Christophe Lyon wrote: >> >> Hi, >> >> Following the discussion about "disable peeling" [1] a few weeks ago, >> it turned out that the vectorizer cost model needed some >> implementation for ARM. >> >> The attached patch implements arm_builtin_vectorization_cost and >> arm_add_stmt_cost, providing default costs when aligned and unaligned >> loads/stores have the same cost (=1). init_cost and finish_cost still >> use the default implementation (I noticed that x86 has chosen to >> duplicate the default implementation without changing it, why?) >> >> Benchmarking shows very little variation, expect a noticeable +1.6% on >> coremark. >> >> If this is OK, we can then discuss how to disable peeling completely >> when aligned and unaligned accesses have the same cost (and thus where >> peeling is a loss of performance). I think adding a new hook is >> necessary, since target descriptions may use different models for >> these costs (eg x86 makes no difference between unaligned loads and >> unaligned stores). >> >> Thanks, >> >> Christophe. >> >> [1] http://gcc.gnu.org/ml/gcc/2012-12/msg00036.html >> >> 2013-02-05 Christophe Lyon >> >> * config/arm/arm.c (arm_builtin_vectorization_cost) >> (arm_add_stmt_cost): New functions. >> (TARGET_VECTORIZE_BUILTIN_VECTORIZATION_COST) >> (TARGET_VECTORIZE_ADD_STMT_COST): Define. >> (struct processor_costs): New struct type. >> (default_arm_cost): New struct of type processor_costs.= >> > > Christophe, > > Thanks for the patch. This is mostly OK, but please can you make the > following changes. > > +struct processor_costs { > > Please name this something like cpu_vec_costs. It's not the only cost table > in the back-end. > > +struct processor_costs default_arm_cost = { /* arm generic costs. */ > > Similarly, use something like default_arm_vec_cost. > > +const struct processor_costs *arm_cost = &default_arm_cost; > > And here. But better still, link this through the current_tune table rather > than introducing a new global. > > Finally, > > @@ -27256,4 +27272,130 @@ arm_validize_comparison (rtx *comparison, rtx * > op1, rtx * op2) > > } > > +/* Vectorizer cost model implementation. */ > > > Please put the patch in a more suitable location rather than just dumping it > at the end of the file. There are already numerous functions related to > costs that are mostly grouped together. I suggest this goes near the > rtx_costs code. > > R. > > diff --git a/gcc/ChangeLog b/gcc/ChangeLog index bfb857d..56fde74 100644 --- a/gcc/ChangeLog +++ b/gcc/ChangeLog @@ -1,3 +1,17 @@ +2013-02-05 Christophe Lyon + + * config/arm/arm-protos.h (struct cpu_vec_costs): New struct type. + (struct tune_params): Add vec_costs field. + * config/arm/arm.c (arm_builtin_vectorization_cost) + (arm_add_stmt_cost): New functions. + (TARGET_VECTORIZE_BUILTIN_VECTORIZATION_COST) + (TARGET_VECTORIZE_ADD_STMT_COST): Define. + (arm_default_vec_cost): New struct of type cpu_vec_costs. + (arm_slowmul_tune, arm_fastmul_tune, arm_strongarm_tune) + (arm_xscale_tune, arm_9e_tune, arm_v6t2_tune, arm_cortex_tune) + (arm_cortex_a15_tune, arm_cortex_a5_tune, arm_cortex_a9_tune) + (arm_v6m_tune, arm_fa726te_tune): Define new vec_costs field. + 2013-02-04 Alexander Potapenko Jack Howarth Jakub Jelinek