This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH] Fix PR58115
- From: Richard Biener <richard dot guenther at gmail dot com>
- To: Bernd Edlinger <bernd dot edlinger at hotmail dot de>, Richard Biener <richard dot guenther at gmail dot com>, Jakub Jelinek <jakub at redhat dot com>, "gcc-patches at gcc dot gnu dot org" <gcc-patches at gcc dot gnu dot org>, Richard Sandiford <rdsandiford at googlemail dot com>
- Date: Tue, 7 Jan 2014 15:10:20 +0100
- Subject: Re: [PATCH] Fix PR58115
- Authentication-results: sourceware.org; auth=none
- References: <DUB122-W8CEA24B3CD3CF0DE08FBAE4F70 at phx dot gbl> <87iotxv211 dot fsf at talisman dot default> <20140106103000 dot GG892 at tucnak dot redhat dot com> <f1eb6537-6c08-437c-9150-938fddf37d6e at email dot android dot com> <DUB122-W13171ACEDEEC745B906363E4B70 at phx dot gbl> <87eh4kvs2e dot fsf at talisman dot default> <DUB122-W3371E3A0858FA6FEE57114E4B70 at phx dot gbl> <87wqict2i0 dot fsf at talisman dot default>
On Tue, Jan 7, 2014 at 1:12 PM, Richard Sandiford
<rdsandiford@googlemail.com> wrote:
> Bernd Edlinger <bernd.edlinger@hotmail.de> writes:
>>> How about this patch for the big comment?
>>>
>>
>> The comment should say that target_set_current_function()
>> cannot call target_reinit() because:
>>
>> target_reinit()=>lang_dependent_init_target()
>> =>init_optabs()=>init_all_optabs(this_fn_optabs);
>>
>> uses this_fn_optabs which is undefined here.
>>
>> However many targets (nios2, rx, i386, rs6000) do exactly that.
>>
>> Is there currently any target, that sets this_target_optab in the
>> target_set_current_function?
>
> MIPS :-) (via save_target_globals_default_opts=>save_target_globals)
>
> I think other targets need to do the same thing in order for tests
> like that extended intrinsics_4.c to work. How does this patch look?
> Tested on x86_64-linux-gnu.
>
> I didn't remove save_target_globals_default_opts because there the
> temporary optimization_current_node also protects a call to init_reg_sets.
Well, if it works the patch is ok. You're way more familiar with the
details of this machinery.
Richard.
> Thanks,
> Richard
>
>
> gcc/
> PR target/58115
> * target-globals.c (save_target_globals): Remove this_fn_optab
> handling.
> * toplev.c: Include optabs.h.
> (target_reinit): Temporarily restore the global options if another
> set of options are in force.
>
> gcc/testsuite/
> * gcc.target/i386/intrinsics_4.c (bar): New function.
>
> Index: gcc/target-globals.c
> ===================================================================
> --- gcc/target-globals.c 2014-01-02 22:16:03.042278971 +0000
> +++ gcc/target-globals.c 2014-01-07 12:08:33.569900970 +0000
> @@ -68,7 +68,6 @@ struct target_globals *
> save_target_globals (void)
> {
> struct target_globals *g;
> - struct target_optabs *saved_this_fn_optabs = this_fn_optabs;
>
> g = ggc_alloc_target_globals ();
> g->flag_state = XCNEW (struct target_flag_state);
> @@ -88,10 +87,8 @@ save_target_globals (void)
> g->bb_reorder = XCNEW (struct target_bb_reorder);
> g->lower_subreg = XCNEW (struct target_lower_subreg);
> restore_target_globals (g);
> - this_fn_optabs = this_target_optabs;
> init_reg_sets ();
> target_reinit ();
> - this_fn_optabs = saved_this_fn_optabs;
> return g;
> }
>
> Index: gcc/toplev.c
> ===================================================================
> --- gcc/toplev.c 2014-01-07 08:11:43.888058805 +0000
> +++ gcc/toplev.c 2014-01-07 12:10:19.448096479 +0000
> @@ -78,6 +78,7 @@ Software Foundation; either version 3, o
> #include "diagnostic-color.h"
> #include "context.h"
> #include "pass_manager.h"
> +#include "optabs.h"
>
> #if defined(DBX_DEBUGGING_INFO) || defined(XCOFF_DEBUGGING_INFO)
> #include "dbxout.h"
> @@ -1752,6 +1753,23 @@ target_reinit (void)
> {
> struct rtl_data saved_x_rtl;
> rtx *saved_regno_reg_rtx;
> + tree saved_optimization_current_node;
> + struct target_optabs *saved_this_fn_optabs;
> +
> + /* Temporarily switch to the default optimization node, so that
> + *this_target_optabs is set to the default, not reflecting
> + whatever a previous function used for the optimize
> + attribute. */
> + saved_optimization_current_node = optimization_current_node;
> + saved_this_fn_optabs = this_fn_optabs;
> + if (saved_optimization_current_node != optimization_default_node)
> + {
> + optimization_current_node = optimization_default_node;
> + cl_optimization_restore
> + (&global_options,
> + TREE_OPTIMIZATION (optimization_default_node));
> + }
> + this_fn_optabs = this_target_optabs;
>
> /* Save *crtl and regno_reg_rtx around the reinitialization
> to allow target_reinit being called even after prepare_function_start. */
> @@ -1769,7 +1787,16 @@ target_reinit (void)
> /* Reinitialize lang-dependent parts. */
> lang_dependent_init_target ();
>
> - /* And restore it at the end, as free_after_compilation from
> + /* Restore the original optimization node. */
> + if (saved_optimization_current_node != optimization_default_node)
> + {
> + optimization_current_node = saved_optimization_current_node;
> + cl_optimization_restore (&global_options,
> + TREE_OPTIMIZATION (optimization_current_node));
> + }
> + this_fn_optabs = saved_this_fn_optabs;
> +
> + /* Restore regno_reg_rtx at the end, as free_after_compilation from
> expand_dummy_function_end clears it. */
> if (saved_regno_reg_rtx)
> {
> Index: gcc/testsuite/gcc.target/i386/intrinsics_4.c
> ===================================================================
> --- gcc/testsuite/gcc.target/i386/intrinsics_4.c 2013-06-27 22:30:03.877275875 +0100
> +++ gcc/testsuite/gcc.target/i386/intrinsics_4.c 2014-01-07 08:15:51.509627927 +0000
> @@ -12,3 +12,10 @@ foo (void)
> {
> a[0] = _mm256_and_ps (b[0], c[0]);
> }
> +
> +/* Try again with a combination of target and optimization attributes. */
> +void __attribute__((target ("avx"), optimize(3)))
> +bar (void)
> +{
> + a[0] = _mm256_and_ps (b[0], c[0]);
> +}