This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PR47785] COLLECT_AS_OPTIONS
- From: "H.J. Lu" <hjl dot tools at gmail dot com>
- To: Kugan Vivekanandarajah <kugan dot vivekanandarajah at linaro dot org>
- Cc: Richard Biener <richard dot guenther at gmail dot com>, GCC Patches <gcc-patches at gcc dot gnu dot org>
- Date: Fri, 1 Nov 2019 08:49:15 -0700
- Subject: Re: [PR47785] COLLECT_AS_OPTIONS
- References: <CAELXzTO=23NOehpZV0AijTJyh45j6YvdqCeH1vi-C8mKM4xngQ@mail.gmail.com> <CAFiYyc1hTeZdA=FaUi_sD=q1O4_A0qZA1ug+tnN=q0Wwd5OQrg@mail.gmail.com> <CAELXzTPd3ADJMuLUQtprkvHhSFOtujN_AwVSwqVyNB-57Vs+eQ@mail.gmail.com> <CAFiYyc1L_cLFTGx8JKFLjx7fOzCFA1rNEmWvh+MigXwOaAckJQ@mail.gmail.com> <CAELXzTP34rNP4DBAbMiWi13+RgEO8-xDHoyeRT1vyt4HPEZtTA@mail.gmail.com> <CAFiYyc0s8A8WQDKAERcdXZ2g7E4BT341tyGbsz8psjnt8hq6Ag@mail.gmail.com> <CAELXzTPNeXjRUYGmExzobs6+WiEpNx_5SBuDyz7D7t-xhSBMvw@mail.gmail.com> <CAMe9rOqcw_n2koqMH_4V28K15mpFXAyyCLzux+8aLxzJ1a-x2A@mail.gmail.com> <CAELXzTMBEXqghuZGRZptY+oJmdt3yxTtK15Swo3svxLQYiYMsw@mail.gmail.com>
On Thu, Oct 31, 2019 at 6:33 PM Kugan Vivekanandarajah
<kugan.vivekanandarajah@linaro.org> wrote:
>
> On Wed, 30 Oct 2019 at 03:11, H.J. Lu <hjl.tools@gmail.com> wrote:
> >
> > On Sun, Oct 27, 2019 at 6:33 PM Kugan Vivekanandarajah
> > <kugan.vivekanandarajah@linaro.org> wrote:
> > >
> > > Hi Richard,
> > >
> > > Thanks for the review.
> > >
> > > On Wed, 23 Oct 2019 at 23:07, Richard Biener <richard.guenther@gmail.com> wrote:
> > > >
> > > > On Mon, Oct 21, 2019 at 10:04 AM Kugan Vivekanandarajah
> > > > <kugan.vivekanandarajah@linaro.org> wrote:
> > > > >
> > > > > Hi Richard,
> > > > >
> > > > > Thanks for the pointers.
> > > > >
> > > > >
> > > > >
> > > > > On Fri, 11 Oct 2019 at 22:33, Richard Biener <richard.guenther@gmail.com> wrote:
> > > > > >
> > > > > > On Fri, Oct 11, 2019 at 6:15 AM Kugan Vivekanandarajah
> > > > > > <kugan.vivekanandarajah@linaro.org> wrote:
> > > > > > >
> > > > > > > Hi Richard,
> > > > > > > Thanks for the review.
> > > > > > >
> > > > > > > On Wed, 2 Oct 2019 at 20:41, Richard Biener <richard.guenther@gmail.com> wrote:
> > > > > > > >
> > > > > > > > On Wed, Oct 2, 2019 at 10:39 AM Kugan Vivekanandarajah
> > > > > > > > <kugan.vivekanandarajah@linaro.org> wrote:
> > > > > > > > >
> > > > > > > > > Hi,
> > > > > > > > >
> > > > > > > > > As mentioned in the PR, attached patch adds COLLECT_AS_OPTIONS for
> > > > > > > > > passing assembler options specified with -Wa, to the link-time driver.
> > > > > > > > >
> > > > > > > > > The proposed solution only works for uniform -Wa options across all
> > > > > > > > > TUs. As mentioned by Richard Biener, supporting non-uniform -Wa flags
> > > > > > > > > would require either adjusting partitioning according to flags or
> > > > > > > > > emitting multiple object files from a single LTRANS CU. We could
> > > > > > > > > consider this as a follow up.
> > > > > > > > >
> > > > > > > > > Bootstrapped and regression tests on arm-linux-gcc. Is this OK for trunk?
> > > > > > > >
> > > > > > > > While it works for your simple cases it is unlikely to work in practice since
> > > > > > > > your implementation needs the assembler options be present at the link
> > > > > > > > command line. I agree that this might be the way for people to go when
> > > > > > > > they face the issue but then it needs to be documented somewhere
> > > > > > > > in the manual.
> > > > > > > >
> > > > > > > > That is, with COLLECT_AS_OPTION (why singular? I'd expected
> > > > > > > > COLLECT_AS_OPTIONS) available to cc1 we could stream this string
> > > > > > > > to lto_options and re-materialize it at link time (and diagnose mismatches
> > > > > > > > even if we like).
> > > > > > > OK. I will try to implement this. So the idea is if we provide
> > > > > > > -Wa,options as part of the lto compile, this should be available
> > > > > > > during link time. Like in:
> > > > > > >
> > > > > > > arm-linux-gnueabihf-gcc -march=armv7-a -mthumb -O2 -flto
> > > > > > > -Wa,-mimplicit-it=always,-mthumb -c test.c
> > > > > > > arm-linux-gnueabihf-gcc -flto test.o
> > > > > > >
> > > > > > > I am not sure where should we stream this. Currently, cl_optimization
> > > > > > > has all the optimization flag provided for compiler and it is
> > > > > > > autogenerated and all the flags are integer values. Do you have any
> > > > > > > preference or example where this should be done.
> > > > > >
> > > > > > In lto_write_options, I'd simply append the contents of COLLECT_AS_OPTIONS
> > > > > > (with -Wa, prepended to each of them), then recover them in lto-wrapper
> > > > > > for each TU and pass them down to the LTRANS compiles (if they agree
> > > > > > for all TUs, otherwise I'd warn and drop them).
> > > > >
> > > > > Attached patch streams it and also make sure that the options are the
> > > > > same for all the TUs. Maybe it is a bit restrictive.
> > > > >
> > > > > What is the best place to document COLLECT_AS_OPTIONS. We don't seem
> > > > > to document COLLECT_GCC_OPTIONS anywhere ?
> > > >
> > > > Nowhere, it's an implementation detail then.
> > > >
> > > > > Attached patch passes regression and also fixes the original ARM
> > > > > kernel build issue with tumb2.
> > > >
> > > > Did you try this with multiple assembler options? I see you stream
> > > > them as -Wa,-mfpu=xyz,-mthumb but then compare the whole
> > > > option strings so a mismatch with -Wa,-mthumb,-mfpu=xyz would be
> > > > diagnosed. If there's a spec induced -Wa option do we get to see
> > > > that as well? I can imagine -march=xyz enabling a -Wa option
> > > > for example.
> > > >
> > > > + *collect_as = XNEWVEC (char, strlen (args_text) + 1);
> > > > + strcpy (*collect_as, args_text);
> > > >
> > > > there's strdup. Btw, I'm not sure why you don't simply leave
> > > > the -Wa option in the merged options [individually] and match
> > > > them up but go the route of comparing strings and carrying that
> > > > along separately. I think that would be much better.
> > >
> > > Is attached patch which does this is OK?
> > >
> >
> > Don't you need to also handle -Xassembler? Since -Wa, doesn't work with comma in
> > assembler options, like -mfoo=foo1,foo2, one needs to use
> >
> > -Xassembler -mfoo=foo1,foo2
> >
> > to pass -mfoo=foo1,foo2 to assembler.
>
>
> gcc -flto -O2 -Wa,-mcpu=zzz1 -mcpu=xxx1 -c foo.c
> gcc -flto -O2 -Wa,-mcpu=zzz2 -mcpu=xxx2 -c bar.c
>
> What should be the option we should provide for the final
> gcc -flto foo.o bar.o -o out
>
> I think our ultimate aim is to handle this in LTO partitioning. That
> is, we should create partitioning such that each partition has the
> same -Wa options. This could also handle -Xassembler -mfoo=foo1,foo2
> which H.J. Lu wanted. We need to modify the heuristics and do some
> performance testing.
>
> In the meantime we could push a simpler solution which is to accept
> -Wa option if they are identical. This would fix at least some of the
> reported cases. Trying to work out what is compatible options, even if
> we ask the back-end to do this, is not a straightforward strategy and
> can be a maintenance nightmare. Unless we can query GNU AS somehow. If
> I am missing something please let me know.
+/* Store switches specified for as with -Wa in COLLECT_AS_OPTIONS
+ and place that in the environment. */
+static void
+putenv_COLLECT_AS_OPTIONS (vec<char_p> vec)
+{
+ unsigned ix;
+ char *opt;
+ int len = vec.length ();
+
+ if (!len)
+ return;
+
+ obstack_init (&collect_obstack);
+ obstack_grow (&collect_obstack, "COLLECT_AS_OPTIONS=",
+ sizeof ("COLLECT_AS_OPTIONS=") - 1);
+ obstack_grow (&collect_obstack, "-Wa,", strlen ("-Wa,"));
+
+ FOR_EACH_VEC_ELT (vec, ix, opt)
+ {
+ obstack_grow (&collect_obstack, opt, strlen (opt));
+ --len;
+ if (len)
+ obstack_grow (&collect_obstack, ",", strlen (","));
+ }
+
+ xputenv (XOBFINISH (&collect_obstack, char *));
This missed the null terminator.
> I therefore propose that we take the simpler approach first and
It works for me.
> improve it by modifying the LTO partitioning. Any thoughts?
>
Thanks.
--
H.J.