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 ?
Attached patch passes regression and also fixes the original ARM
kernel build issue with tumb2.
Thanks,
Kugan
>
> Richard.
>
> > Thanks,
> > Kugan
> >
> >
> >
> > >
> > > Richard.
> > >
> > > > Thanks,
> > > > Kugan
> > > >
> > > >
> > > > gcc/ChangeLog:
> > > >
> > > > 2019-10-02 kugan.vivekanandarajah <kugan.vivekanandara...@linaro.org>
> > > >
> > > > PR lto/78353
> > > > * gcc.c (putenv_COLLECT_AS_OPTION): New to set COLLECT_AS_OPTION in env.
> > > > (driver::main): Call putenv_COLLECT_AS_OPTION.
> > > > * lto-wrapper.c (run_gcc): use COLLECT_AS_OPTION from env.
> > > >
> > > > gcc/testsuite/ChangeLog:
> > > >
> > > > 2019-10-02 kugan.vivekanandarajah <kugan.vivekanandara...@linaro.org>
> > > >
> > > > PR lto/78353
> > > > * gcc.target/arm/pr78353-1.c: New test.
> > > > * gcc.target/arm/pr78353-2.c: New test.
gcc/testsuite/ChangeLog:
2019-10-21 Kugan Vivekanandarajah <kugan.vivekanandara...@linaro.org>
PR lto/78353
* gcc.target/arm/pr78353-1.c: New test.
* gcc.target/arm/pr78353-2.c: New test.
gcc/ChangeLog:
2019-10-21 Kugan Vivekanandarajah <kugan.vivekanandara...@linaro.org>
PR lto/78353
* gcc.c (putenv_COLLECT_AS_OPTIONS): New to set COLLECT_AS_OPTIONS in
env.
(driver::main): Call putenv_COLLECT_AS_OPTIONS.
* lto-opts.c (lto_write_options): Stream COLLECT_AS_OPTIONS.
* lto-wrapper.c (merge_and_complain): Get COLLECT_AS_OPTIONS and
make sure they are the same from all TUs.
(find_and_merge_options): Get COLLECT_AS_OPTIONS.
(run_gcc): Likewise.
From 120d61790236cbde5a5d0cb8455b0e3583dd90b2 Mon Sep 17 00:00:00 2001
From: Kugan <kugan.vivekanandarajah@linaro.org>
Date: Sat, 28 Sep 2019 02:11:49 +1000
Subject: [PATCH] COLLECT_AS support
Change-Id: Ic05ab2cec895fd64bec75449785e42b809aa73cc
---
gcc/gcc.c | 29 +++++++++++++++++++++++
gcc/lto-opts.c | 16 +++++++++++--
gcc/lto-wrapper.c | 30 ++++++++++++++++++++----
gcc/testsuite/gcc.target/arm/pr78353-1.c | 9 +++++++
gcc/testsuite/gcc.target/arm/pr78353-2.c | 9 +++++++
5 files changed, 86 insertions(+), 7 deletions(-)
create mode 100644 gcc/testsuite/gcc.target/arm/pr78353-1.c
create mode 100644 gcc/testsuite/gcc.target/arm/pr78353-2.c
diff --git a/gcc/gcc.c b/gcc/gcc.c
index 1216cdd505a..dede47fb055 100644
--- a/gcc/gcc.c
+++ b/gcc/gcc.c
@@ -5239,6 +5239,34 @@ do_specs_vec (vec<char_p> vec)
}
}
+/* 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 *));
+}
+
/* Process the sub-spec SPEC as a portion of a larger spec.
This is like processing a whole spec except that we do
not initialize at the beginning and we do not supply a
@@ -7360,6 +7388,7 @@ driver::main (int argc, char **argv)
global_initializations ();
build_multilib_strings ();
set_up_specs ();
+ putenv_COLLECT_AS_OPTIONS (assembler_options);
putenv_COLLECT_GCC (argv[0]);
maybe_putenv_COLLECT_LTO_WRAPPER ();
maybe_putenv_OFFLOAD_TARGETS ();
diff --git a/gcc/lto-opts.c b/gcc/lto-opts.c
index 0e9f24e1189..d89fe5cc4f9 100644
--- a/gcc/lto-opts.c
+++ b/gcc/lto-opts.c
@@ -166,8 +166,20 @@ lto_write_options (void)
obstack_grow (&temporary_obstack, "\0", 1);
args = XOBFINISH (&temporary_obstack, char *);
lto_write_data (args, strlen (args) + 1);
- lto_end_section ();
-
obstack_free (&temporary_obstack, NULL);
+ const char *collect_as_options = getenv ("COLLECT_AS_OPTIONS");
+
+ if (collect_as_options)
+ {
+ obstack_init (&temporary_obstack);
+ append_to_collect_gcc_options (&temporary_obstack, &first_p,
+ collect_as_options);
+ obstack_grow (&temporary_obstack, "\0", 1);
+ args = XOBFINISH (&temporary_obstack, char *);
+ lto_write_data (args, strlen (args) + 1);
+ obstack_free (&temporary_obstack, NULL);
+ }
+
+ lto_end_section ();
free (section_name);
}
diff --git a/gcc/lto-wrapper.c b/gcc/lto-wrapper.c
index 9a7bbd0c022..c07a6f23125 100644
--- a/gcc/lto-wrapper.c
+++ b/gcc/lto-wrapper.c
@@ -219,7 +219,8 @@ static void
merge_and_complain (struct cl_decoded_option **decoded_options,
unsigned int *decoded_options_count,
struct cl_decoded_option *fdecoded_options,
- unsigned int fdecoded_options_count)
+ unsigned int fdecoded_options_count,
+ char **collect_as)
{
unsigned int i, j;
struct cl_decoded_option *pic_option = NULL;
@@ -253,6 +254,19 @@ merge_and_complain (struct cl_decoded_option **decoded_options,
break;
default:
+ if (foption->opt_index == OPT_Wa_)
+ {
+ const char *args_text = foption->orig_option_with_args_text;
+ if (*collect_as)
+ {
+ if (strcmp (*collect_as, args_text) != 0)
+ fatal_error (input_location, "-Wa, options does not match");
+ break;
+ }
+ *collect_as = XNEWVEC (char, strlen (args_text) + 1);
+ strcpy (*collect_as, args_text);
+ break;
+ }
if (!(cl_options[foption->opt_index].flags & CL_TARGET))
break;
@@ -975,7 +989,8 @@ find_crtoffloadtable (void)
static bool
find_and_merge_options (int fd, off_t file_offset, const char *prefix,
struct cl_decoded_option **opts,
- unsigned int *opt_count, const char *collect_gcc)
+ unsigned int *opt_count, const char *collect_gcc,
+ char **collect_as)
{
off_t offset, length;
char *data;
@@ -1020,7 +1035,8 @@ find_and_merge_options (int fd, off_t file_offset, const char *prefix,
else
merge_and_complain (&fdecoded_options,
&fdecoded_options_count,
- f2decoded_options, f2decoded_options_count);
+ f2decoded_options, f2decoded_options_count,
+ collect_as);
fopts += strlen (fopts) + 1;
}
@@ -1251,6 +1267,7 @@ run_gcc (unsigned argc, char *argv[])
char *list_option_full = NULL;
const char *linker_output = NULL;
const char *collect_gcc, *collect_gcc_options;
+ char *collect_as_options;
int parallel = 0;
int jobserver = 0;
int auto_parallel = 0;
@@ -1283,6 +1300,7 @@ run_gcc (unsigned argc, char *argv[])
get_options_from_collect_gcc_options (collect_gcc, collect_gcc_options,
&decoded_options,
&decoded_options_count);
+ collect_as_options = getenv ("COLLECT_AS_OPTIONS");
/* Allocate array for input object files with LTO IL,
and for possible preceding arguments. */
@@ -1333,7 +1351,7 @@ run_gcc (unsigned argc, char *argv[])
if (find_and_merge_options (fd, file_offset, LTO_SECTION_NAME_PREFIX,
&fdecoded_options, &fdecoded_options_count,
- collect_gcc))
+ collect_gcc, &collect_as_options))
{
have_lto = true;
ltoobj_argv[ltoobj_argc++] = argv[i];
@@ -1350,6 +1368,8 @@ run_gcc (unsigned argc, char *argv[])
append_compiler_options (&argv_obstack, fdecoded_options,
fdecoded_options_count);
append_linker_options (&argv_obstack, decoded_options, decoded_options_count);
+ if (collect_as_options)
+ obstack_ptr_grow (&argv_obstack, collect_as_options);
/* Scan linker driver arguments for things that are of relevance to us. */
for (j = 1; j < decoded_options_count; ++j)
@@ -1537,7 +1557,7 @@ cont1:
OFFLOAD_SECTION_NAME_PREFIX,
&offload_fdecoded_options,
&offload_fdecoded_options_count,
- collect_gcc))
+ collect_gcc, &collect_as_options))
fatal_error (input_location, "cannot read %s: %m", filename);
close (fd);
if (filename != offload_argv[i])
diff --git a/gcc/testsuite/gcc.target/arm/pr78353-1.c b/gcc/testsuite/gcc.target/arm/pr78353-1.c
new file mode 100644
index 00000000000..bba81ee50c3
--- /dev/null
+++ b/gcc/testsuite/gcc.target/arm/pr78353-1.c
@@ -0,0 +1,9 @@
+/* { dg-do compile } */
+/* { dg-options "-march=armv7-a -mthumb -O2 -flto -Wa,-mimplicit-it=always" } */
+
+int main(int x)
+{
+ asm("teq %0, #0; addne %0, %0, #1" : "=r" (x));
+ return x;
+}
+
diff --git a/gcc/testsuite/gcc.target/arm/pr78353-2.c b/gcc/testsuite/gcc.target/arm/pr78353-2.c
new file mode 100644
index 00000000000..776eb64b8c7
--- /dev/null
+++ b/gcc/testsuite/gcc.target/arm/pr78353-2.c
@@ -0,0 +1,9 @@
+/* { dg-do compile } */
+/* { dg-options "-march=armv7-a -mthumb -O2 -flto -Wa,-mimplicit-it=always,-mthumb" } */
+
+int main(int x)
+{
+ asm("teq %0, #0; addne %0, %0, #1" : "=r" (x));
+ return x;
+}
+
--
2.17.1
