On Wed, 30 Oct 2019 at 03:11, H.J. Lu <hjl.to...@gmail.com> wrote: > > On Sun, Oct 27, 2019 at 6:33 PM Kugan Vivekanandarajah > <kugan.vivekanandara...@linaro.org> wrote: > > > > Hi Richard, > > > > Thanks for the review. > > > > On Wed, 23 Oct 2019 at 23:07, Richard Biener <richard.guent...@gmail.com> > > wrote: > > > > > > On Mon, Oct 21, 2019 at 10:04 AM Kugan Vivekanandarajah > > > <kugan.vivekanandara...@linaro.org> wrote: > > > > > > > > Hi Richard, > > > > > > > > Thanks for the pointers. > > > > > > > > > > > > > > > > On Fri, 11 Oct 2019 at 22:33, Richard Biener > > > > <richard.guent...@gmail.com> wrote: > > > > > > > > > > On Fri, Oct 11, 2019 at 6:15 AM Kugan Vivekanandarajah > > > > > <kugan.vivekanandara...@linaro.org> wrote: > > > > > > > > > > > > Hi Richard, > > > > > > Thanks for the review. > > > > > > > > > > > > On Wed, 2 Oct 2019 at 20:41, Richard Biener > > > > > > <richard.guent...@gmail.com> wrote: > > > > > > > > > > > > > > On Wed, Oct 2, 2019 at 10:39 AM Kugan Vivekanandarajah > > > > > > > <kugan.vivekanandara...@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. I therefore propose that we take the simpler approach first and improve it by modifying the LTO partitioning. Any thoughts? Thanks, Kugan > > -- > H.J.
