From patchwork Wed Nov 13 01:50:27 2013 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Sriraman Tallam X-Patchwork-Id: 290812 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)) (Client did not present a certificate) by ozlabs.org (Postfix) with ESMTPS id 2E1DD2C007C for ; Wed, 13 Nov 2013 12:50:48 +1100 (EST) DomainKey-Signature: a=rsa-sha1; c=nofws; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender :mime-version:in-reply-to:references:date:message-id:subject :from:to:cc:content-type; q=dns; s=default; b=g2xqHnRVNzNRvDpJDJ S2rRPrEC38rw5MJXMy5721mKAabhmcBZCnI/mmvpEx4ko1jWVLm+RdH/uCPk4qAl g4F2fppxPdYxE7VelEPPJLGJ2OUFObRVGeuZt37Pk0V0sPSwLmhdUy/yf+xpoHuW tmRH+8TtJDYnhYks3hKTtubsA= 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 :mime-version:in-reply-to:references:date:message-id:subject :from:to:cc:content-type; s=default; bh=B9Obq0dRflysIlL9C/FM+WF1 kTA=; b=oHX4uQpwcuFkXU7/TXlSqbTDmxINeYAxaH2aIULWa1GdcPUAsI0UtM7N zfxUjOL43TCPhjD8AS4+zpaTXBLwfOZrQlwBLT+i95XPi1zuauimZhPPE1sggfeY oPWJy1UcStIHQEYhgGlKb819+HFIaxB/8XozuVZa8nqnunFcS4E= Received: (qmail 13777 invoked by alias); 13 Nov 2013 01:50:38 -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 13622 invoked by uid 89); 13 Nov 2013 01:50:37 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-0.9 required=5.0 tests=AWL, BAYES_50, RDNS_NONE, SPF_PASS, URIBL_BLOCKED autolearn=no version=3.3.2 X-HELO: mail-ve0-f177.google.com Received: from Unknown (HELO mail-ve0-f177.google.com) (209.85.128.177) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES128-SHA encrypted) ESMTPS; Wed, 13 Nov 2013 01:50:36 +0000 Received: by mail-ve0-f177.google.com with SMTP id jz11so3898254veb.36 for ; Tue, 12 Nov 2013 17:50:28 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:mime-version:in-reply-to:references:date :message-id:subject:from:to:cc:content-type; bh=bJ3k0Z6iIr2ys9/Eelnrr06O5ba53Bl+kxF3p+yhjXk=; b=JC9lwMQD5G1c+OXljDAxxHYZV1BZVM3Ep0lzNG8Cff4gj1AlACPyA/FJUDG7lebFl+ pLG0STsPFt6ilpl7TWg8KJPQfq5zJi5WofpqRayYn5Ol6uGL/AvygyT/AlTXswy8pbtR 5+2XnwPI2Hbs/ueNLHOdd7Pss93tf3s0GB/w7fgpOkcESKqCq0WVMZVqJ08O6ggSj9Ss FfQC68LtQPvDb1bI+GDNOig1Ndwe9bCEep1OJOP96SRaeMWQ3A9KYl55P9l1HcYUIbKE WfSmlfpUtrffOmeOT4uuMhs1tYAOac4MosSUNIRK2G7jLBDfvSy8mbJXX8VPceqKlbcM KP9g== X-Gm-Message-State: ALoCoQk68ohkw1KA0PgSsXkdA5bnqUAup2tYaVVKhsEqmmC+z9fMJpIGtsaCtgJvy7O9/L8a6q74j9/6aKifmJ3thmWXCWKMZZHThQbwE9PZkYu4UbP6dWF96e2XlittaFM7l3WcsnDVHtD9nIqg+aIv5ljoI+fewUyF634/uGQ1puCMT2dXJffbAuqGtWxxYniELh/2Dgx24zZlDHHyrJXSR7zmugE6Kw== MIME-Version: 1.0 X-Received: by 10.58.144.168 with SMTP id sn8mr64993veb.33.1384307428171; Tue, 12 Nov 2013 17:50:28 -0800 (PST) Received: by 10.52.167.35 with HTTP; Tue, 12 Nov 2013 17:50:27 -0800 (PST) In-Reply-To: References: Date: Tue, 12 Nov 2013 17:50:27 -0800 Message-ID: Subject: Re: [PATCH, i386] Fix -mpreferred-stack-boundary From: Sriraman Tallam To: Bernd Edlinger Cc: Uros Bizjak , "gcc-patches@gcc.gnu.org" X-IsSubscribed: yes On Tue, Nov 12, 2013 at 5:17 PM, Sriraman Tallam wrote: > On Tue, Nov 12, 2013 at 2:53 PM, Bernd Edlinger > wrote: >> Hi, >> >> >> On Tue, 12 Nov 2013 10:30:16, Sriraman Tallam wrote: >>> >>> On Mon, Nov 11, 2013 at 11:30 PM, Uros Bizjak wrote: >>>> There was something wrong with Bernd's address, retrying. >>>> >>>>>> Currently on trunk the option -mpreferred-stack-boundary does not work >>>>>> together with #pragma GCC target("sse") or __attribute__((target("sse"))). >>>>>> >>>>>> There is already a test case that detects this: gcc.target/i386/fastcall-sseregparm.c >>>>>> >>>>>> The attached patch fixes this test case under i686-pc-linux-gnu. >>>>>> >>>>>> Boot-strapped and regression-tested under i686-pc-linux-gnu. >>>>>> >>>>>> OK for trunk? > > No, this is not what I had in mind. This is simply reverting my > refactoring work which was to make ix86_option_override_internal get > rid of the global_options dependency. Here is the problem: > global_options gets some flags set after command-line options are read > (ix86_preferred_stack_boundary_arg in this case). But, this does not > get saved into target_option_default_node because there is no > corresponding field in cl_target_option for > ix86_preferred_stack_boundary_arg. So, when you restore > target_option_default_node to func_options in > ix86_valid_target_attribute_p, this particular flag does not get > copied. So, you can either copy this explicitly to func_options which > was your first patch or you could extend cl_target_option to include > this field too which is done by making > ix86_preferred_stack_boundary_arg a Variable in i386.opt. The latter > is cleaner because it always saves the default flags into > target_option_default_node. I quickly hacked up what I had in mind and attached the patch. Can you check if this fixes your problem? Thanks Sri > > Thanks > Sri > > >>>>> >>>>> I'm not experienced enough in this new option handling stuff, let's >>>>> ask Sriraman for his opinion on the patch. >>> >>> >>> I do not think this is the right fix, I am wondering how many other >>> target flags we may have to copy this way from global_options. I >>> notice that other flags like ix86_regparm and >>> ix86_incoming_stack_boundary_arg are very similar. Why should this >>> need to be restored from global_options explicitly? This patch may fix >>> the issue but it seems like a hack to me. We should be able to restore >>> whatever we need from target_option_default_node via >>> ix86_function_specific_restore. Maybe modifying the i386.opt file to >>> make ix86_preferred_stack_boundary_arg a variable might fix it. See >>> ix86_isa_flags for instance in i386.opt. >>> >>> >>> Please let me know what you think. >> >> Thanks, now I see what you mean. I can change it the other way round >> and implement ix86_preferred_stack_boundary like ix86_incoming_stack_boundary. >> >> using this define in options.h: >> #define ix86_preferred_stack_boundary_arg global_options.x_ix86_preferred_stack_boundary_arg >> >> the global option is never copied into function specific options. >> >> Attached is the updated patch. >> >> OK for trunk after boot-strap and regression-testing? >> >> Bernd. >> >> >> PS: I have one more patch pending, and would like to know what you think >> about it: http://gcc.gnu.org/ml/gcc-patches/2013-11/msg00133.html >> That has also to do with global state variables that are modified due to >> target options, and initially I was expecting that the patch for PR57756 would >> be fixing it automatically, but I was wrong... >> >>> >>> Thanks >>> Sri >>> >>> >>>> >>>> Uros. Index: config/i386/i386.c =================================================================== --- config/i386/i386.c (revision 204712) +++ config/i386/i386.c (working copy) @@ -4182,6 +4182,7 @@ ix86_function_specific_save (struct cl_target_opti ptr->x_ix86_isa_flags_explicit = opts->x_ix86_isa_flags_explicit; ptr->x_ix86_target_flags_explicit = opts->x_ix86_target_flags_explicit; ptr->x_recip_mask_explicit = opts->x_recip_mask_explicit; + ptr->x_ix86_preferred_stack_boundary_arg = opts->x_ix86_preferred_stack_boundary_arg; /* The fields are char but the variables are not; make sure the values fit in the fields. */ @@ -4211,6 +4212,7 @@ ix86_function_specific_restore (struct gcc_options opts->x_ix86_isa_flags_explicit = ptr->x_ix86_isa_flags_explicit; opts->x_ix86_target_flags_explicit = ptr->x_ix86_target_flags_explicit; opts->x_recip_mask_explicit = ptr->x_recip_mask_explicit; + opts->x_ix86_preferred_stack_boundary_arg = ptr->x_ix86_preferred_stack_boundary_arg; /* Recreate the arch feature tests if the arch changed */ if (old_arch != ix86_arch) Index: config/i386/i386.opt =================================================================== --- config/i386/i386.opt (revision 204712) +++ config/i386/i386.opt (working copy) @@ -64,6 +64,12 @@ HOST_WIDE_INT x_ix86_isa_flags_explicit Variable int ix86_target_flags_explicit +Variable +int ix86_preferred_stack_boundary_arg + +TargetSave +int x_ix86_preferred_stack_boundary_arg + ;; which flags were passed by the user TargetSave HOST_WIDE_INT x_ix86_target_flags_explicit