| Message ID | 87h7sg42ng.fsf@oracle.com |
|---|---|
| State | New |
| Headers | show |
| Series | dg-options after board/cflags | expand |
On Wed, Sep 2, 2020 at 8:31 AM Jose E. Marchesi via Gcc-patches <gcc-patches@gcc.gnu.org> wrote: > > > Hi people! > > While adding a bpf-sim.exp to dejagnu, I noticed that the flags in > board/cflags were included in the final compilation line _after_ the > flags in the test's dg-options. > > Since the test options are more particular than the board options, I > would expect them to be placed after any board-defined flags, so I > prepared the patch below for dejagnu, which does the right thing for the > gcc.target/bpf testsuite. > > However: > > 1. There could be tests around that depend (erroneously) on some of > their dg-options to not have effect (or a different effect) because > they are annulled (or modified) by some flag in a board file. > > 2. This could also impact other programs using dejagnu. > > How do you people recommend to proceed? > Should we fix dejagnu and then fix buggy tests? > Or the other way around? :-) > > diff --git a/lib/target.exp b/lib/target.exp > index 36ae639..f0bfe20 100644 > --- a/lib/target.exp > +++ b/lib/target.exp > @@ -455,7 +455,7 @@ proc default_target_compile {source destfile type options} { > } > if {[regexp "^additional_flags=" $i]} { > regsub "^additional_flags=" $i "" tmp > - append add_flags " $tmp" > + append additional_flags " $tmp" > } > if {[regexp "^ldflags=" $i]} { > regsub "^ldflags=" $i "" tmp > @@ -703,6 +703,8 @@ proc default_target_compile {source destfile type options} { > } > } > > + append add_flags " $additional_flags" > + > verbose "doing compile" > > set sources "" > @@ -728,7 +730,7 @@ proc default_target_compile {source destfile type options} { > append add_flags " -o $destfile" > } > } > - > + > # This is obscure: we put SOURCES at the end when building an > # object, because otherwise, in some situations, libtool will > # become confused about the name of the actual source file. Does your dejagnu contain commit 5256bd82343000c76bc0e48139003f90b6184347 Author: H.J. Lu <hjl.tools@gmail.com> Date: Thu Feb 26 17:53:48 2015 +1100 * lib/target.exp (default_target_compile): Prepend multilib_flags, instead of appending it. Some GCC testcases need explicit GCC options to properly run. For example gcc.target/i386/pr32219-1.c has -fpie specified explicitly: /* { dg-options "-O2 -fpie" } */ But with multlib, eg: make check-gcc RUNTESTFLAGS="--target_board='unix{-fpic}'" -fpic is appended to the command line options, which overrides the command line options specified by dg-options. multlib flags should be placed at the beginning of the command line options, not at the end. This patch updates default_target_compile to prepend multilib_flags, instead of appending it.
> On Wed, Sep 2, 2020 at 8:31 AM Jose E. Marchesi via Gcc-patches > <gcc-patches@gcc.gnu.org> wrote: >> >> >> Hi people! >> >> While adding a bpf-sim.exp to dejagnu, I noticed that the flags in >> board/cflags were included in the final compilation line _after_ the >> flags in the test's dg-options. >> >> Since the test options are more particular than the board options, I >> would expect them to be placed after any board-defined flags, so I >> prepared the patch below for dejagnu, which does the right thing for the >> gcc.target/bpf testsuite. >> >> However: >> >> 1. There could be tests around that depend (erroneously) on some of >> their dg-options to not have effect (or a different effect) because >> they are annulled (or modified) by some flag in a board file. >> >> 2. This could also impact other programs using dejagnu. >> >> How do you people recommend to proceed? >> Should we fix dejagnu and then fix buggy tests? >> Or the other way around? :-) >> >> diff --git a/lib/target.exp b/lib/target.exp >> index 36ae639..f0bfe20 100644 >> --- a/lib/target.exp >> +++ b/lib/target.exp >> @@ -455,7 +455,7 @@ proc default_target_compile {source destfile type options} { >> } >> if {[regexp "^additional_flags=" $i]} { >> regsub "^additional_flags=" $i "" tmp >> - append add_flags " $tmp" >> + append additional_flags " $tmp" >> } >> if {[regexp "^ldflags=" $i]} { >> regsub "^ldflags=" $i "" tmp >> @@ -703,6 +703,8 @@ proc default_target_compile {source destfile type options} { >> } >> } >> >> + append add_flags " $additional_flags" >> + >> verbose "doing compile" >> >> set sources "" >> @@ -728,7 +730,7 @@ proc default_target_compile {source destfile type options} { >> append add_flags " -o $destfile" >> } >> } >> - >> + >> # This is obscure: we put SOURCES at the end when building an >> # object, because otherwise, in some situations, libtool will >> # become confused about the name of the actual source file. > > Does your dejagnu contain > > commit 5256bd82343000c76bc0e48139003f90b6184347 > Author: H.J. Lu <hjl.tools@gmail.com> > Date: Thu Feb 26 17:53:48 2015 +1100 > > * lib/target.exp (default_target_compile): Prepend multilib_flags, > instead of appending it. > > Some GCC testcases need explicit GCC options to properly run. For > example gcc.target/i386/pr32219-1.c has -fpie specified explicitly: > > /* { dg-options "-O2 -fpie" } */ > > But with multlib, eg: > make check-gcc RUNTESTFLAGS="--target_board='unix{-fpic}'" > > -fpic is appended to the command line options, which overrides the command > line options specified by dg-options. multlib flags should be placed at > the beginning of the command line options, not at the end. This patch > updates default_target_compile to prepend multilib_flags, instead of > appending it. Yeah, this is dejagnu master. Your patch dealt with board/multilib_flags, but the same problem exists for board/cflags and many other flag-containing options.
On Wed, 2 Sep 2020, Jose E. Marchesi via Gcc-patches wrote: > Your patch dealt with board/multilib_flags, but the same problem exists > for board/cflags and many other flag-containing options. What's the use case for that? IIUC board flags are supposed to be ones that are absolutely required for executables to run with a given board, such as multilib selection, special linker scripts, non-standard run-time library paths, etc. These must not be overridden by test cases or they will inevitably fail. Maciej
>> Your patch dealt with board/multilib_flags, but the same problem exists >> for board/cflags and many other flag-containing options. > > What's the use case for that? IIUC board flags are supposed to be ones > that are absolutely required for executables to run with a given board, > such as multilib selection, special linker scripts, non-standard run-time > library paths, etc. These must not be overridden by test cases or they > will inevitably fail. We want to build most bpf-unknown-none test files with -mxbpf, but not all. I wasn't aware of the "absolutely require" nature of the board flags... then it is clear we need to find another strategy, like specifying it in the individual test files/dg-options.
diff --git a/lib/target.exp b/lib/target.exp index 36ae639..f0bfe20 100644 --- a/lib/target.exp +++ b/lib/target.exp @@ -455,7 +455,7 @@ proc default_target_compile {source destfile type options} { } if {[regexp "^additional_flags=" $i]} { regsub "^additional_flags=" $i "" tmp - append add_flags " $tmp" + append additional_flags " $tmp" } if {[regexp "^ldflags=" $i]} { regsub "^ldflags=" $i "" tmp @@ -703,6 +703,8 @@ proc default_target_compile {source destfile type options} { } } + append add_flags " $additional_flags" + verbose "doing compile" set sources "" @@ -728,7 +730,7 @@ proc default_target_compile {source destfile type options} { append add_flags " -o $destfile" } } - + # This is obscure: we put SOURCES at the end when building an # object, because otherwise, in some situations, libtool will # become confused about the name of the actual source file.