From patchwork Tue Jun 30 13:32:50 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: James Greenhalgh X-Patchwork-Id: 489718 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.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id B34A61402B6 for ; Tue, 30 Jun 2015 23:33:18 +1000 (AEST) Authentication-Results: ozlabs.org; dkim=pass (1024-bit key; unprotected) header.d=gcc.gnu.org header.i=@gcc.gnu.org header.b=p6rA/HTY; dkim-atps=neutral DomainKey-Signature: a=rsa-sha1; c=nofws; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender:from :to:cc:subject:date:message-id:in-reply-to:references :mime-version:content-type; q=dns; s=default; b=x51E3yT79hahBbjZ s72sS1i/zAFNgf9OAw7HAXDQvdARbN76Qi+1sxG6S7OTFG4Xwc0mAxLsH2QVR1Xy UV1JSB0aj5e7QBRBhVGVXdXtUk+HtAtP7GTYXo580GCyc8sdZ7b8xlUmX8vVwCr6 PVsmn7Ub9e2crqlLgrBIR3Ov/CM= 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:from :to:cc:subject:date:message-id:in-reply-to:references :mime-version:content-type; s=default; bh=MAbs/dv9V8OFJ1/YQuT8FJ f1OM8=; b=p6rA/HTYLbx/iIv88UaVv2RUR9dEFkwH9dNVFvGLxFgtcOV6P28Hri f99FezR4IroN/3S1SlvfYEHsHbcS5BUWOibhPlj09zw1VWy5SYuUAZbQqK2HJJoG HlXHUwiqWdb2IBpe6X5TkVhfhQkg6Kmdr83EmPTbWNIet67h2ooNg= Received: (qmail 83742 invoked by alias); 30 Jun 2015 13:33:10 -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 82731 invoked by uid 89); 30 Jun 2015 13:33:09 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.9 required=5.0 tests=AWL, BAYES_00, SPF_PASS autolearn=ham version=3.3.2 X-HELO: eu-smtp-delivery-143.mimecast.com Received: from eu-smtp-delivery-143.mimecast.com (HELO eu-smtp-delivery-143.mimecast.com) (207.82.80.143) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Tue, 30 Jun 2015 13:33:06 +0000 Received: from cam-owa1.Emea.Arm.com (fw-tnat.cambridge.arm.com [217.140.96.140]) by eu-smtp-1.mimecast.com with ESMTP id uk-mta-24-FwKvcxMOSPSz0nKfGH6L3w-1 Received: from e106375-lin.cambridge.arm.com ([10.1.2.79]) by cam-owa1.Emea.Arm.com with Microsoft SMTPSVC(6.0.3790.3959); Tue, 30 Jun 2015 14:32:59 +0100 From: James Greenhalgh To: gcc-patches@gcc.gnu.org Cc: rguenther@suse.de, law@redhat.com, jakub@redhat.com Subject: Re: [Patch SRA] Fix PR66119 by calling get_move_ratio in SRA Date: Tue, 30 Jun 2015 14:32:50 +0100 Message-Id: <1435671170-5281-1-git-send-email-james.greenhalgh@arm.com> In-Reply-To: <20150626171000.GX10247@tucnak.redhat.com> References: <20150626171000.GX10247@tucnak.redhat.com> MIME-Version: 1.0 X-MC-Unique: FwKvcxMOSPSz0nKfGH6L3w-1 X-IsSubscribed: yes On Fri, Jun 26, 2015 at 06:10:00PM +0100, Jakub Jelinek wrote: > On Fri, Jun 26, 2015 at 06:03:34PM +0100, James Greenhalgh wrote: > > --- /dev/null > > +++ b/gcc/testsuite/g++.dg/pr66119.C > > I think generally testcases shouldn't be added into g++.dg/ directly, > but subdirectories. So g++.dg/opt/ ? > > > @@ -0,0 +1,69 @@ > > +/* PR66119 - MOVE_RATIO is not constant in a compiler run, so Scalar > > + Reduction of Aggregates must ask the back-end more than once what > > + the value of MOVE_RATIO now is. */ > > + > > +/* { dg-do compile { target i?86-*-* x86_64-*-* } } */ > > In g++.dg/, dejagnu cycles through all 3 major -std=c* versions, > thus using -std=c++11 is inappropriate. > If the test requires c++11, instead you do > // { dg-do compile { target { { i?86-*-* x86_64-*-* } && c++11 } } } > > > +/* { dg-options "-std=c++11 -O3 -mavx -fdump-tree-sra -march=slm" { target avx_runtime } } */ > > and remove -std=c++11 here. I don't see any point in guarding it with > avx_runtime, after all, if not avx_runtime, the test will be compiled with > -O0 and thus very likely fail the scan-tree-dump test. > > As it is dg-do compile test only, you have no dependency on assembler nor > linker nor runtime. > But I'd add -mtune=slm too. Thanks, I'm used to the dance we try to do to get Neon enabled/disabled correctly when testing multilib environments on ARM so tried to overengineer things! I've updated the testcase as you suggested, and moved it to g++.dg/opt. OK? Thanks, James --- gcc/ 2015-06-30 James Greenhalgh PR tree-optimization/66119 * toplev.c (process_options): Don't set up default values for the sra_max_scalarization_size_{speed,size} parameters. * tree-sra (analyze_all_variable_accesses): If no values have been set for the sra_max_scalarization_size_{speed,size} parameters, call get_move_ratio to get target defaults. gcc/testsuite/ 2015-06-30 James Greenhalgh * g++.dg/opt/pr66119.C: New. diff --git a/gcc/testsuite/g++.dg/opt/pr66119.C b/gcc/testsuite/g++.dg/opt/pr66119.C new file mode 100644 index 0000000..5b420c2 --- /dev/null +++ b/gcc/testsuite/g++.dg/opt/pr66119.C @@ -0,0 +1,69 @@ +/* PR66119 - MOVE_RATIO is not constant in a compiler run, so Scalar + Reduction of Aggregates must ask the back-end more than once what + the value of MOVE_RATIO now is. */ + +/* { dg-do compile { target { { i?86-*-* x86_64-*-* } && c++11 } } } */ +/* { dg-options "-O3 -mavx -fdump-tree-sra -march=slm -mtune=slm" } */ + +#include + +class MyAVX +{ + __m256d data; +public: + MyAVX () = default; + MyAVX (const MyAVX &) = default; + MyAVX (__m256d _data) : data(_data) { ; } + + MyAVX & operator= (const MyAVX &) = default; + + operator __m256d () const { return data; } + MyAVX operator+ (MyAVX s2) { return data+s2.data; } +}; + +template class AVX_trait { ; }; + +template <> class AVX_trait { +public: + typedef __m256d TSIMD; +}; + + +template +class MyTSIMD +{ + typename AVX_trait::TSIMD data; + +public: + MyTSIMD () = default; + MyTSIMD (const MyTSIMD &) = default; + // MyTSIMD (const MyTSIMD & s2) : data(s2.data) { ; } + MyTSIMD (typename AVX_trait::TSIMD _data) : data(_data) { ; } + + operator typename AVX_trait::TSIMD() const { return data; } + MyTSIMD operator+ (MyTSIMD s2) { return data+s2.data; } +}; + +// using MyVec = MyAVX; +using MyVec = MyTSIMD; + +class Vec2 +{ + MyVec a, b; +public: + Vec2 (MyVec aa, MyVec ab) : a(aa), b(ab) { ; } + Vec2 operator+ (Vec2 v2) { return Vec2(a+v2.a, b+v2.b); } +}; + +inline __attribute__ ((__always_inline__)) +Vec2 ComputeSomething (Vec2 a, Vec2 b) +{ + return a+b; +} + +Vec2 TestFunction (Vec2 a, Vec2 b) +{ + return ComputeSomething (a,b); +} + +/* { dg-final { scan-tree-dump "Created a replacement for b" "sra" } } */ diff --git a/gcc/toplev.c b/gcc/toplev.c index 573b144..d97d852 100644 --- a/gcc/toplev.c +++ b/gcc/toplev.c @@ -1299,20 +1299,6 @@ process_options (void) so we can correctly initialize debug output. */ no_backend = lang_hooks.post_options (&main_input_filename); - /* Set default values for parameters relation to the Scalar Reduction - of Aggregates passes (SRA and IP-SRA). We must do this here, rather - than in opts.c:default_options_optimization as historically these - tuning heuristics have been based on MOVE_RATIO, which on some - targets requires other symbols from the backend. */ - maybe_set_param_value - (PARAM_SRA_MAX_SCALARIZATION_SIZE_SPEED, - get_move_ratio (true) * UNITS_PER_WORD, - global_options.x_param_values, global_options_set.x_param_values); - maybe_set_param_value - (PARAM_SRA_MAX_SCALARIZATION_SIZE_SIZE, - get_move_ratio (false) * UNITS_PER_WORD, - global_options.x_param_values, global_options_set.x_param_values); - /* Some machines may reject certain combinations of options. */ targetm.target_option.override (); diff --git a/gcc/tree-sra.c b/gcc/tree-sra.c index 7f242f7..e648061 100644 --- a/gcc/tree-sra.c +++ b/gcc/tree-sra.c @@ -2545,11 +2545,20 @@ analyze_all_variable_accesses (void) bitmap tmp = BITMAP_ALLOC (NULL); bitmap_iterator bi; unsigned i; - unsigned max_scalarization_size - = (optimize_function_for_size_p (cfun) - ? PARAM_VALUE (PARAM_SRA_MAX_SCALARIZATION_SIZE_SIZE) - : PARAM_VALUE (PARAM_SRA_MAX_SCALARIZATION_SIZE_SPEED)) - * BITS_PER_UNIT; + bool optimize_speed_p = !optimize_function_for_size_p (cfun); + + enum compiler_param param = optimize_speed_p + ? PARAM_SRA_MAX_SCALARIZATION_SIZE_SPEED + : PARAM_SRA_MAX_SCALARIZATION_SIZE_SIZE; + + /* If the user didn't set PARAM_SRA_MAX_SCALARIZATION_SIZE_<...>, + fall back to a target default. */ + unsigned HOST_WIDE_INT max_scalarization_size + = global_options_set.x_param_values[param] + ? PARAM_VALUE (param) + : get_move_ratio (optimize_speed_p) * UNITS_PER_WORD; + + max_scalarization_size *= BITS_PER_UNIT; EXECUTE_IF_SET_IN_BITMAP (candidate_bitmap, 0, i, bi) if (bitmap_bit_p (should_scalarize_away_bitmap, i)