Bernd Edlinger <bernd.edlin...@hotmail.de> writes:
> Hello,
>
> on i686-pc-linux-gnu the test case gcc.target/i386/intrinsics_4.c fails
> because of
> an internal compiler error, see PR58155.
>
> The reason for this is that the optab CODE_FOR_movv8sf is disabled when it
> should be enabled.
>
> This happens because invoke_set_current_function_hook changes the pointer
> "this_fn_optabs" after targetm.set_current_function has already modified the
> optab to enable/disable CODE_FOR_movv8sf, leaving that optab entry
> in an undefined state.
>
> Boot-strapped and regression-tested on i686-pc-linux-gnu.
>
> Ok for trunk?
>
> Regards
> Bernd.
>
>
> Index: gcc/function.c
> ===================================================================
> --- gcc/function.c (revision 204101)
> +++ gcc/function.c (working copy)
> @@ -4426,7 +4426,6 @@ invoke_set_current_function_hook (tree fndecl)
> cl_optimization_restore (&global_options, TREE_OPTIMIZATION (opts));
> }
>
> - targetm.set_current_function (fndecl);
> this_fn_optabs = this_target_optabs;
>
> if (opts != optimization_default_node)
> @@ -4436,6 +4435,8 @@ invoke_set_current_function_hook (tree fndecl)
> this_fn_optabs = (struct target_optabs *)
> TREE_OPTIMIZATION_OPTABS (opts);
> }
> +
> + targetm.set_current_function (fndecl);
> }
> }
>
Sorry for only realising now, but this breaks the mips16 attribute.
The idea with the old order was that set_current_function would set
up this_target, then that choice would percolate through.
I don't think the patch is really correct for i386 either. Although
it fixes the intrinsics_4.c test for -m32, it fails for this extended
version, both with and without -m32:
#include <immintrin.h>
__m256 a[10], b[10], c[10];
void __attribute__((target ("avx")))
foo (void)
{
a[0] = _mm256_and_ps (b[0], c[0]);
}
void __attribute__((target ("avx"), optimize (3)))
bar (void)
{
a[0] = _mm256_and_ps (b[0], c[0]);
}
The failure without -m32 is a regression from before the patch.
i386.c tries to detect when we're moving between different target options
and when a target_reinit is needed, but it doesn't take into account the
fact that optabs are cached on a per-optimisation-node basis. It's
possible to move between functions with the same target node but
different optimisation nodes. With the old order this wasn't
really something i386.c should have to worry about, since it was
invoke_set_current_function that handled optimisation nodes.
But with the new order i386.c would need to reinitialise at least
the optabs when moving between functions with the same target node
but different optimisation nodes. And I'd really rather not force
every target that uses DECL_FUNCTION_SPECIFIC_TARGET to copy-&-paste
the code to do that.
Of course, IMO, the cleanest fix would be to use switchable targets
for i386...
Thanks,
Richard
