| Message ID | 5693D0AC.50307@bell.net |
|---|---|
| State | New |
| Headers | show |
On 11 January 2016 at 16:56, John David Anglin <dave.anglin@bell.net> wrote: > On 2016-01-11 8:24 AM, Jakub Jelinek wrote: >> >> On Mon, Jan 11, 2016 at 02:16:31PM +0100, Christophe Lyon wrote: >>>>> >>>>> In any case, we have no_c99_libc_has_function on hpux and everything on >>>>> linux. So, I >>>>> don't think testing with function_c99_misc on hppa will show any >>>>> difference. >>>>> >>>>> Okay with function_c99_misc? >>>> >>>> Ok (but please make sure to adjust ChangeLog too). >>>> >>> This patch made gcc.dg/torture/builtin-integral-1.c FAIL on >>> bare metal targets (e.g. arm-non-eabi, aarch64-none-elf, >>> using Newlib). >>> The logs show link errors: >>> /ccOMzAOC.o: In function `test': >>> builtin-integral-1.c:(.text+0x3c): undefined reference to `link_failure' >>> collect2: error: ld returned 1 exit status >> >> I'd say you want to either split that test into the double and float+long >> double functions and limit the latter only to c99_runtime targets >> (and add add_options_for_c99_runtime), or just guard the whole test >> with c99_runtime and add_options_for_c99_runtime. > > Attached is untested patch implementing the latter option. I tend to think > there's not > much benefit in testing these tests on non c99 targets. > > Will test when current tests complete. > I tested a similar version on my side. It just makes the test become UNSUPPORTED for arm/aarch64 + newlib. They used to pass, though. I saw that Andre is looking at that from the Newlib pov, too. Christophe. > > Dave > > -- > John David Anglin dave.anglin@bell.net >
On Mon, Jan 11, 2016 at 05:11:21PM +0100, Christophe Lyon wrote: > I tested a similar version on my side. It just makes the test become > UNSUPPORTED for arm/aarch64 + newlib. They used to pass, though. Is anything bad on that? The test tests functions that newlib does not implement, so it is not wrong not to optimize those. Jakub
On 11/01/16 16:39, Jakub Jelinek wrote: > On Mon, Jan 11, 2016 at 05:11:21PM +0100, Christophe Lyon wrote: >> I tested a similar version on my side. It just makes the test become >> UNSUPPORTED for arm/aarch64 + newlib. They used to pass, though. > > Is anything bad on that? The test tests functions that newlib does not > implement, so it is not wrong not to optimize those. > hm i didn't set function_* support for musl libc because previously these were only used for invalid optimizations (fast-math). if they are used for valid optimizations too then they should be defined correctly for other libcs too i guess.
On 11/01/16 16:39, Jakub Jelinek wrote: > On Mon, Jan 11, 2016 at 05:11:21PM +0100, Christophe Lyon wrote: >> I tested a similar version on my side. It just makes the test become >> UNSUPPORTED for arm/aarch64 + newlib. They used to pass, though. > > Is anything bad on that? The test tests functions that newlib does not > implement, so it is not wrong not to optimize those. > > Jakub > Unfortunately c99_functions is a very wide net. For instance, newlib supports the ceill, but doesn't support wscanf_s nor any bounds checking function I think. I extracted all function names from the C99 standard and did a quick nm and grep to look into whether newlib defined these for arm-none-eabi. The functions I found missing fall into the following sections: - Complex Arithmetic (which fall under the function_c99_math_complex class) - floating-point environment - Functions for greatest-width integer types - atomics (missing atomic_is_lock_free and atomic_fetch_key) - Bounds-checking interfaces So arm-none-eabi used to be able to "legally" perform the transformation that we are speaking of. Though since that optimization is now guarded with the function_c99_misc class it is no longer performed, since we can not claim newlib supports all functions that are caught by function_c99_misc. I don't quite know how to proceed. I suspect a new function class for C99 math functions (excluding complex) would help here and probably more places too. Though, I don't know how much work it would be to split function_c99_misc in that manner. Opinions welcome!! Cheers, Andre
On 14/01/16 15:02, Andre Vieira (lists) wrote: > Unfortunately c99_functions is a very wide net. For instance, newlib supports the ceill, but doesn't support > wscanf_s nor any bounds checking function I think. > wscanf_s is not c99 (it is in the optional annex k of c11, which is likely to be removed from the standard) > I extracted all function names from the C99 standard and did a quick nm and grep to look into whether newlib > defined these for arm-none-eabi. > > The functions I found missing fall into the following sections: > - Complex Arithmetic (which fall under the function_c99_math_complex class) that's ok to skip (complex become optional in c11) > - floating-point environment > - Functions for greatest-width integer types these are not ok to skip from c99 > - atomics (missing atomic_is_lock_free and atomic_fetch_key) > - Bounds-checking interfaces > these are optional c11 features > I don't quite know how to proceed. I suspect a new function class for C99 math functions (excluding complex) > would help here and probably more places too. Though, I don't know how much work it would be to split > function_c99_misc in that manner. > > Opinions welcome!! i think newlib should implement the missing c99 apis otherwise don't expect any c99 optimizations (and should be considered broken with -std=c99 or higher)
On 2016-01-11, at 10:56 AM, John David Anglin wrote: > On 2016-01-11 8:24 AM, Jakub Jelinek wrote: >> On Mon, Jan 11, 2016 at 02:16:31PM +0100, Christophe Lyon wrote: >>>>> In any case, we have no_c99_libc_has_function on hpux and everything on linux. So, I >>>>> don't think testing with function_c99_misc on hppa will show any difference. >>>>> >>>>> Okay with function_c99_misc? >>>> Ok (but please make sure to adjust ChangeLog too). >>>> >>> This patch made gcc.dg/torture/builtin-integral-1.c FAIL on >>> bare metal targets (e.g. arm-non-eabi, aarch64-none-elf, >>> using Newlib). >>> The logs show link errors: >>> /ccOMzAOC.o: In function `test': >>> builtin-integral-1.c:(.text+0x3c): undefined reference to `link_failure' >>> collect2: error: ld returned 1 exit status >> I'd say you want to either split that test into the double and float+long >> double functions and limit the latter only to c99_runtime targets >> (and add add_options_for_c99_runtime), or just guard the whole test >> with c99_runtime and add_options_for_c99_runtime. > Attached is untested patch implementing the latter option. I tend to think there's not > much benefit in testing these tests on non c99 targets. Committed. Dave -- John David Anglin dave.anglin@bell.net
Index: gcc.dg/torture/builtin-integral-1.c =================================================================== --- gcc.dg/torture/builtin-integral-1.c (revision 232191) +++ gcc.dg/torture/builtin-integral-1.c (working copy) @@ -10,6 +10,8 @@ that various math functions are marked const/pure and can be folded. */ /* { dg-options "-ffinite-math-only -fno-math-errno" } */ +/* { dg-add-options c99_runtime } */ +/* { dg-require-effective-target c99_runtime } */ /* { dg-skip-if "" { *-*-* } { "-O0" } { "" } } */ extern int link_failure (int);