On Sun, Sep 13, 2020 at 10:29:22AM +0200, Jakub Jelinek via Gcc-patches wrote:
> On Fri, Sep 11, 2020 at 11:29:52AM +0200, Jakub Jelinek via Gcc-patches wrote:
> > On Fri, Sep 11, 2020 at 09:46:37AM +0200, Christophe Lyon via Gcc-patches
> > wrote:
> > > I'm seeing an ICE with this new test on most of my arm configurations,
> > > for instance:
> > > --target arm-none-linux-gnueabi --with-cpu cortex-a9
> > > /aci-gcc-fsf/builds/gcc-fsf-gccsrc/obj-arm-none-linux-gnueabi/gcc3/gcc/xgcc
> > > -B/aci-gcc-fsf/builds/gcc-fsf-gccsrc/obj-ar
> > > m-none-linux-gnueabi/gcc3/gcc/ c_lto_pr96939_0.o c_lto_pr96939_1.o
> > > -fdiagnostics-plain-output -flto -O2 -o
> > > gcc-target-arm-lto-pr96939-01.exe
> >
> > Seems a latent issue.
> > Neither cl_optimization_{save,restore} nor cl_target_option_{save,restore}
> > (nor any of the target hooks they call) saves or restores any opts_set
> > values, so I think opts_set can be trusted only during option processing (if
> > at all), but not later.
> > So, short term a fix would be IMHO just stop using opts_set altogether in
> > arm_configure_build_target, it doesn't make much sense to me, it should test
> > if those strings are non-NULL instead, or at least do that when it is
> > invoked from arm_option_restore (e.g. could be done by calling it with
> > opts instead of &global_options_set ).
> > Longer term, the question is if cl_optimization_{save,restore} and
> > cl_target_option_{save,restore} shouldn't be changed not to only
> > save/restore the options, but also save the opts_set flags.
> > It could be done e.g. by adding a bool array or set of bool members
> > to struct cl_optimization and struct cl_target_option , or even more compact
> > by using bitmasks, pack each 64 adjacent option flags into a UHWI element
> > of an array.
>
> So, I've tried under debugger how it behaves and seems global_options_set
> is really an or of whether an option has been ever seen as explicit, either
> on the command line or in any of the option pragmas or optimize/target
> attributes seen so far, so it isn't something that can be relied on.
>
> The following patch implements the saving/restoring of the opts_set bits
> (though only for the options/variables saved by the generic options-save.c
> code, for the target specific stuff that isn't handled by the generic code
> the opts_set argument is now passed to the hook and the backends can choose
> e.g. to use a TargetSave variable to save the flags either individually or
> together in some bitmask (or ignore it if they never need opts_set for the
> options).
>
> Bootstrapped/regtested on x86_64-linux, i686-linux, armv7hl-linux-gnueabi,
> aarch64-linux, powerpc64le-linux and lto bootstrapped on x86_64-linux, ok
> for trunk?
This patch breaks quite a view test cases (target-attribute/tattr-*) on
IBM Z. Having a look at function cl_target_option_restore reveals that
some members of opts_set are reduced to 1 or 0 depending on whether a
member was set before or not, e.g. for target_flags we have
opts_set->x_target_flags = (mask & 1) != 0;
whereas previously those members where not touched by
cl_target_option_restore.
My intuition of this whole option evaluation is still pretty vague.
Basically opts_set is a set of options enabled via command line and/or
via pragmas/attributes whereas opts is the set of options which are
implied by opts_set.
What puzzles me right now is that in cl_target_option_save we save in
ptr only options from opts but not from opts_set whereas in
cl_target_option_restore we override some members of opts_set. Thus it
is unclear to me how a backend should restore opts_set then.
I'm probably missing something. Any hints on how to restore opts_set
and especially target_flags?
Cheers,
Stefan
