From patchwork Tue Jun 23 15:42:44 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: James Greenhalgh X-Patchwork-Id: 487694 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 CCA6C140082 for ; Wed, 24 Jun 2015 01:43:05 +1000 (AEST) Authentication-Results: ozlabs.org; dkim=pass (1024-bit key; unprotected) header.d=gcc.gnu.org header.i=@gcc.gnu.org header.b=L/79tptG; 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=rhzJvCr4d5oFL7/d SVSR4sTHNSHhcdFZopuNeWSlWN0aGdkVBgEdaxUMtzxmpqehAIGIMHAhfw/e5aeI kyi6FWphdbiJQAsxnz1IchLLUAmgLeI0FZIEnk0FwuvfpHuIm/YMNoKwXLdXTBea iKXnXk143dzdmf2iuiigBKhdYos= 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=bMjzQLHZ70REsnnPMxyGGm IZmgs=; b=L/79tptGZXgVAH7/fVMZy9bcTKlCQHJ+/nKMh8YsFjOFbq3ZE3rM8e 1z1Jo9CBv9kKPDFoG2CYuo7JJRdf9f2eQRbE7qBNDKZegrE5sgPR/tBgvwKp+RNE Enk2NYvvKePaRPXjxSG3+ycBddI/jLCZNQVMJ5/EKLzcg12kEgLkU= Received: (qmail 108832 invoked by alias); 23 Jun 2015 15:42:56 -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 108819 invoked by uid 89); 23 Jun 2015 15:42:56 -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) (146.101.78.143) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Tue, 23 Jun 2015 15:42:54 +0000 Received: from cam-owa2.Emea.Arm.com (fw-tnat.cambridge.arm.com [217.140.96.140]) by eu-smtp-1.mimecast.com with ESMTP id uk-mta-27-8wgmrIg8QuGG1OuCIIZPqw-1 Received: from e106375-lin.cambridge.arm.com ([10.1.2.79]) by cam-owa2.Emea.Arm.com with Microsoft SMTPSVC(6.0.3790.3959); Tue, 23 Jun 2015 16:42:50 +0100 From: James Greenhalgh To: gcc-patches@gcc.gnu.org Cc: jakub@redhat.com, rguenther@suse.de Subject: Re: [Patch SRA] Fix PR66119 by calling get_move_ratio in SRA Date: Tue, 23 Jun 2015 16:42:44 +0100 Message-Id: <1435074164-7206-1-git-send-email-james.greenhalgh@arm.com> In-Reply-To: <20150623085201.GH10247@tucnak.redhat.com> References: <20150623085201.GH10247@tucnak.redhat.com> MIME-Version: 1.0 X-MC-Unique: 8wgmrIg8QuGG1OuCIIZPqw-1 X-IsSubscribed: yes On Tue, Jun 23, 2015 at 09:52:01AM +0100, Jakub Jelinek wrote: > On Tue, Jun 23, 2015 at 09:18:52AM +0100, James Greenhalgh wrote: > > This patch fixes the issue by always calling get_move_ratio in the SRA > > code, ensuring that an up-to-date value is used. > > > > Unfortunately, this means we have to use 0 as a sentinel value for > > the parameter - indicating no user override of the feature - and > > therefore cannot use it to disable scalarization. However, there > > are other ways to disable scalarazation (-fno-tree-sra) so this is not > > a great loss. > > You can handle even that. > > enum compiler_param param > = optimize_function_for_size_p (cfun) > ? PARAM_SRA_MAX_SCALARIZATION_SIZE_SIZE > : PARAM_SRA_MAX_SCALARIZATION_SIZE_SPEED; > unsigned max_scalarization_size = PARAM_VALUE (param) * BITS_PER_UNIT; > if (!max_scalarization_size && !global_options_set.x_param_values[param]) > > Then it will handle explicit --param sra-max-scalarization-size-Os*=0 > differently from implicit 0. Ah hah! OK, I've respun the patch removing this extra justification in the documentation and reshuffling the logic a little. > OT, shouldn't max_scalarization_size be at least unsigned HOST_WIDE_INT, > so that it doesn't overflow for larger values (0x40000000 etc.)? > Probably need some cast in the multiplication to avoid UB in the compiler. I've increased the size of max_scalarization_size to a UHWI in this spin. Bootstrapped and tested on AArch64 and x86-64 with no issues and checked to see the PR is fixed. OK for trunk, and gcc-5 in a few days? Thanks, James --- gcc/ 2015-06-23 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. diff --git a/gcc/toplev.c b/gcc/toplev.c index 2f43a89..902bfc7 100644 --- a/gcc/toplev.c +++ b/gcc/toplev.c @@ -1301,20 +1301,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 8e34244..5f573f6 100644 --- a/gcc/tree-sra.c +++ b/gcc/tree-sra.c @@ -2549,11 +2549,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)