| Message ID | BC1D7100-36CA-44B0-80FC-223547481C9E@adacore.com |
|---|---|
| State | New |
| Headers | show |
| Series | Guard aarch64/aapcs64 tests using abitest.S by check_weak_available | expand |
On Mon, Dec 16, 2019 at 1:25 AM Olivier Hainque <hainque@adacore.com> wrote: > > Hello, > > Some tests from gcc/testsuite/gcc.target/aarch64/aapcs64 > resort to the abitest.S source, which defines a few weak symbols: > > ... > .weak testfunc > .weak testfunc_ptr > .weak saved_return_address > > The attached patch is a proposal to prevent the execution of > those tests in configurations where weak symbols are advertised > as not supported. > > This cures a significant number of failures on VxWorks and bootstrap > + regress tests fine on aarch64-linux. > > Is this ok to commit ? Why does VxWorks not have weak symbol support when it is an elf target? I can understand it not having support in a loader but these symbols should all be resolved at build time. Thanks, Andrew Pinski > > Thanks in advance! > > With Kind Regards, > > Olivier > > 2019-12-16 Joel Brobecker <brobecker@adacore.com> > Olivier Hainque <hainque@adacore.com> > > testsuite/ > > * gcc.target/aarch64/aapcs64/aapcs64.exp: Guard tests using > abitest.S by check_weak_available. >
Olivier Hainque <hainque@adacore.com> writes: > Hello, > > Some tests from gcc/testsuite/gcc.target/aarch64/aapcs64 > resort to the abitest.S source, which defines a few weak symbols: > > ... > .weak testfunc > .weak testfunc_ptr > .weak saved_return_address > > The attached patch is a proposal to prevent the execution of > those tests in configurations where weak symbols are advertised > as not supported. > > This cures a significant number of failures on VxWorks and bootstrap > + regress tests fine on aarch64-linux. > > Is this ok to commit ? > > Thanks in advance! > > With Kind Regards, > > Olivier > > 2019-12-16 Joel Brobecker <brobecker@adacore.com> > Olivier Hainque <hainque@adacore.com> > > testsuite/ > > * gcc.target/aarch64/aapcs64/aapcs64.exp: Guard tests using > abitest.S by check_weak_available. OK, thanks. Richard > > .../gcc.target/aarch64/aapcs64/aapcs64.exp | 36 ++++++++++++------- > 1 file changed, 23 insertions(+), 13 deletions(-) > > diff --git a/gcc/testsuite/gcc.target/aarch64/aapcs64/aapcs64.exp b/gcc/testsuite/gcc.target/aarch64/aapcs64/aapcs64.exp > index c17af6c3084..36687800ecd 100644 > --- a/gcc/testsuite/gcc.target/aarch64/aapcs64/aapcs64.exp > +++ b/gcc/testsuite/gcc.target/aarch64/aapcs64/aapcs64.exp > @@ -29,12 +29,16 @@ torture-init > set-torture-options $C_TORTURE_OPTIONS > set additional_flags "-W -Wall -Wno-abi" > > -# Test parameter passing. > -foreach src [lsort [glob -nocomplain $srcdir/$subdir/test_*.c]] { > - if {[runtest_file_p $runtests $src]} { > +# Test parameter passing. This uses abitest.S which relies on weak > +# symbols. > + > +if { [check_weak_available] } { > + foreach src [lsort [glob -nocomplain $srcdir/$subdir/test_*.c]] { > + if {[runtest_file_p $runtests $src]} { > c-torture-execute [list $src \ > $srcdir/$subdir/abitest.S] \ > $additional_flags > + } > } > } > > @@ -48,25 +52,31 @@ foreach src [lsort [glob -nocomplain $srcdir/$subdir/rec_*.c]] { > } > } > > -# Test unnamed argument retrieval via the va_arg macro. > -foreach src [lsort [glob -nocomplain $srcdir/$subdir/va_arg-*.c]] { > - if {[runtest_file_p $runtests $src]} { > +# Test unnamed argument retrieval via the va_arg macro. This uses abitest.S > +# which relies on weak symbols. > +if { [check_weak_available] } { > + foreach src [lsort [glob -nocomplain $srcdir/$subdir/va_arg-*.c]] { > + if {[runtest_file_p $runtests $src]} { > c-torture-execute [list $src \ > $srcdir/$subdir/abitest.S] \ > $additional_flags > + } > } > } > > -# Test function return value. > -# Disable -fipa-ra to prevent the compiler from generating > -# conflicting code. > -set additional_flags_for_func_ret $additional_flags > -append additional_flags_for_func_ret " -fno-ipa-ra" > -foreach src [lsort [glob -nocomplain $srcdir/$subdir/func-ret-*.c]] { > - if {[runtest_file_p $runtests $src]} { > +# Test function return value. This uses abitest.S which relies on > +# weak symbols. > +if { [check_weak_available] } { > + # Disable -fipa-ra to prevent the compiler from generating > + # conflicting code. > + set additional_flags_for_func_ret $additional_flags > + append additional_flags_for_func_ret " -fno-ipa-ra" > + foreach src [lsort [glob -nocomplain $srcdir/$subdir/func-ret-*.c]] { > + if {[runtest_file_p $runtests $src]} { > c-torture-execute [list $src \ > $srcdir/$subdir/abitest.S] \ > $additional_flags_for_func_ret > + } > } > }
Hi Andrew, > On 16 Dec 2019, at 10:54, Andrew Pinski <pinskia@gmail.com> wrote: > > Why does VxWorks not have weak symbol support when it is an elf > target? I can understand it not having support in a loader but these > symbols should all be resolved at build time. VxWorks has so called "Dynamic Kernel Modules", which are partially linked aggregations of object files that one would download and link dynamically with a running kernel on a target. We don't support weak symbols in this mode. Olivier
> On 16 Dec 2019, at 11:19, Richard Sandiford <richard.sandiford@arm.com> wrote: >> * gcc.target/aarch64/aapcs64/aapcs64.exp: Guard tests using >> abitest.S by check_weak_available. > > OK, thanks. Great, thanks for your prompt feedback Richard! As a side question, we have quite a few failures or aarch64 specific tests caused by the use of -fpic or -fPIC in dg-options, also not supported by the VxWorks compilers in kernel mode. We have local patches adding dg-require-effective-target fpic directives to these. Is that the correct thing to do ? Thanks in advance, Regards, Olivier
Olivier Hainque <hainque@adacore.com> writes: >> On 16 Dec 2019, at 11:19, Richard Sandiford <richard.sandiford@arm.com> wrote: > >>> * gcc.target/aarch64/aapcs64/aapcs64.exp: Guard tests using >>> abitest.S by check_weak_available. >> >> OK, thanks. > > Great, thanks for your prompt feedback Richard! > > As a side question, we have quite a few failures or aarch64 > specific tests caused by the use of -fpic or -fPIC in dg-options, > also not supported by the VxWorks compilers in kernel mode. > > We have local patches adding > > dg-require-effective-target fpic > > directives to these. > > Is that the correct thing to do ? Yeah. Adding that to tests that use -fpic or -fPIC is OK/preapproved. Personally, I don't think people can be expected to remember to use this whenever they add a new -fpic or -fPIC test, so it's probably going to be a constant fight to get clean results without PIC support. Maybe we should have a programmatic fix. E.g. we could override dg-options in gcc-dg.exp and make it do the equivalent of: { dg-require-effective-target fpic } whenever -fpic or -fPIC is used. We override it in a few test harnesses already (e.g. mips.exp, which does something more complicated) so it wouldn't be entirely new ground. Thanks, Richard
> On 16 Dec 2019, at 14:54, Richard Sandiford <richard.sandiford@arm.com> wrote: >> We have local patches adding >> >> dg-require-effective-target fpic >> >> directives to these. >> >> Is that the correct thing to do ? > > Yeah. Adding that to tests that use -fpic or -fPIC is OK/preapproved. > > Personally, I don't think people can be expected to remember to use > this whenever they add a new -fpic or -fPIC test, so it's probably > going to be a constant fight to get clean results without PIC support. > > Maybe we should have a programmatic fix. E.g. we could override > dg-options in gcc-dg.exp and make it do the equivalent of: > > { dg-require-effective-target fpic } > > whenever -fpic or -fPIC is used. We override it in a few test harnesses > already (e.g. mips.exp, which does something more complicated) so it > wouldn't be entirely new ground. I see. I'll probably go for the simple approach first, which indeed seems to be commonly used. Thanks for your feedback on this Richard. Best Regards, Olivier
diff --git a/gcc/testsuite/gcc.target/aarch64/aapcs64/aapcs64.exp b/gcc/testsuite/gcc.target/aarch64/aapcs64/aapcs64.exp index c17af6c3084..36687800ecd 100644 --- a/gcc/testsuite/gcc.target/aarch64/aapcs64/aapcs64.exp +++ b/gcc/testsuite/gcc.target/aarch64/aapcs64/aapcs64.exp @@ -29,12 +29,16 @@ torture-init set-torture-options $C_TORTURE_OPTIONS set additional_flags "-W -Wall -Wno-abi" -# Test parameter passing. -foreach src [lsort [glob -nocomplain $srcdir/$subdir/test_*.c]] { - if {[runtest_file_p $runtests $src]} { +# Test parameter passing. This uses abitest.S which relies on weak +# symbols. + +if { [check_weak_available] } { + foreach src [lsort [glob -nocomplain $srcdir/$subdir/test_*.c]] { + if {[runtest_file_p $runtests $src]} { c-torture-execute [list $src \ $srcdir/$subdir/abitest.S] \ $additional_flags + } } } @@ -48,25 +52,31 @@ foreach src [lsort [glob -nocomplain $srcdir/$subdir/rec_*.c]] { } } -# Test unnamed argument retrieval via the va_arg macro. -foreach src [lsort [glob -nocomplain $srcdir/$subdir/va_arg-*.c]] { - if {[runtest_file_p $runtests $src]} { +# Test unnamed argument retrieval via the va_arg macro. This uses abitest.S +# which relies on weak symbols. +if { [check_weak_available] } { + foreach src [lsort [glob -nocomplain $srcdir/$subdir/va_arg-*.c]] { + if {[runtest_file_p $runtests $src]} { c-torture-execute [list $src \ $srcdir/$subdir/abitest.S] \ $additional_flags + } } } -# Test function return value. -# Disable -fipa-ra to prevent the compiler from generating -# conflicting code. -set additional_flags_for_func_ret $additional_flags -append additional_flags_for_func_ret " -fno-ipa-ra" -foreach src [lsort [glob -nocomplain $srcdir/$subdir/func-ret-*.c]] { - if {[runtest_file_p $runtests $src]} { +# Test function return value. This uses abitest.S which relies on +# weak symbols. +if { [check_weak_available] } { + # Disable -fipa-ra to prevent the compiler from generating + # conflicting code. + set additional_flags_for_func_ret $additional_flags + append additional_flags_for_func_ret " -fno-ipa-ra" + foreach src [lsort [glob -nocomplain $srcdir/$subdir/func-ret-*.c]] { + if {[runtest_file_p $runtests $src]} { c-torture-execute [list $src \ $srcdir/$subdir/abitest.S] \ $additional_flags_for_func_ret + } } }
Hello, Some tests from gcc/testsuite/gcc.target/aarch64/aapcs64 resort to the abitest.S source, which defines a few weak symbols: ... .weak testfunc .weak testfunc_ptr .weak saved_return_address The attached patch is a proposal to prevent the execution of those tests in configurations where weak symbols are advertised as not supported. This cures a significant number of failures on VxWorks and bootstrap + regress tests fine on aarch64-linux. Is this ok to commit ? Thanks in advance! With Kind Regards, Olivier 2019-12-16 Joel Brobecker <brobecker@adacore.com> Olivier Hainque <hainque@adacore.com> testsuite/ * gcc.target/aarch64/aapcs64/aapcs64.exp: Guard tests using abitest.S by check_weak_available. .../gcc.target/aarch64/aapcs64/aapcs64.exp | 36 ++++++++++++------- 1 file changed, 23 insertions(+), 13 deletions(-)