Message ID | Yk5ouVh7fw09nXLK@toto.the-meissners.org |
---|---|
State | New |
Headers | show |
Series | Disable float128 tests on VxWorks, PR target/104253. | expand |
> I have run the tests on my usual Linux systems (little endian power10, > little endian power9, big endian power8), but I don't have access to a > VxWorks system. Eric does this fix the failure for you? Yes, if you add '*' at the end of the selector, i.e. [istarget *-*-vxworks*]. > If it does fix the failure, can I apply the patch to the master branch and > backport it to GCC 11 and GCC 10? Sorry about the breakage. That would be perfect, thanks in advance.
On Thu, Apr 07, 2022 at 12:29:45AM -0400, Michael Meissner wrote: > In PR target/104253, it was pointed out the that test case added as part > of fixing the PR does not work on VxWorks because float128 is not > supported on that system. I have modified the three tests for float128 so > that they are manually excluded on VxWorks systems. In looking at the > code, I also added checks in check_effective_target_ppc_ieee128_ok to > disable the systems that will never support VSX instructions which are > required for float128 support (eabi, eabispe, darwin). It's just one extra to the big list here, but, why do we need all these manual exclusions anyway? What is broken about the test itself? It would be so much more useful if the tests would help us, instead of producing a lot of extra busy-work. Segher
On Thu, 2022-04-07 at 06:00 -0500, Segher Boessenkool wrote: > On Thu, Apr 07, 2022 at 12:29:45AM -0400, Michael Meissner wrote: > > In PR target/104253, it was pointed out the that test case added as part > > of fixing the PR does not work on VxWorks because float128 is not > > supported on that system. I have modified the three tests for float128 so > > that they are manually excluded on VxWorks systems. In looking at the > > code, I also added checks in check_effective_target_ppc_ieee128_ok to > > disable the systems that will never support VSX instructions which are > > required for float128 support (eabi, eabispe, darwin). > > It's just one extra to the big list here, but, why do we need all these > manual exclusions anyway? What is broken about the test itself? From the PR, it looks like this test noted an error, not actually a failure. https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104253#c17 cc1: warning: The '-mfloat128' option may not be fully supported which comes out of gcc/config/rs6000/rs6000.cc rs6000_option_override_internal() via /* IEEE 128-bit floating point requires VSX support. */ if (TARGET_FLOAT128_KEYWORD) { if (!TARGET_VSX) { <snip> } else if (!TARGET_FLOAT128_TYPE) { TARGET_FLOAT128_TYPE = 1; warning (0, "The %<-mfloat128%> option may not be fully supported"); } } > > It would be so much more useful if the tests would help us, instead of > producing a lot of extra busy-work. > > > Segher
On Thu, Apr 07, 2022 at 08:50:53AM -0500, will schmidt wrote: > On Thu, 2022-04-07 at 06:00 -0500, Segher Boessenkool wrote: > > On Thu, Apr 07, 2022 at 12:29:45AM -0400, Michael Meissner wrote: > > > In PR target/104253, it was pointed out the that test case added as part > > > of fixing the PR does not work on VxWorks because float128 is not > > > supported on that system. I have modified the three tests for float128 so > > > that they are manually excluded on VxWorks systems. In looking at the > > > code, I also added checks in check_effective_target_ppc_ieee128_ok to > > > disable the systems that will never support VSX instructions which are > > > required for float128 support (eabi, eabispe, darwin). > > > > It's just one extra to the big list here, but, why do we need all these > > manual exclusions anyway? What is broken about the test itself? > > >From the PR, it looks like this test noted an error, not actually a > failure. > > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104253#c17 > > cc1: warning: The '-mfloat128' option may not be fully supported Aha, thanks for checking. If that is what causes the error, the test in target-supports needs robustifying. > > It would be so much more useful if the tests would help us, instead of > > producing a lot of extra busy-work. (^^^ to help improve this) Segher
On Thu, Apr 07, 2022 at 06:00:51AM -0500, Segher Boessenkool wrote: > On Thu, Apr 07, 2022 at 12:29:45AM -0400, Michael Meissner wrote: > > In PR target/104253, it was pointed out the that test case added as part > > of fixing the PR does not work on VxWorks because float128 is not > > supported on that system. I have modified the three tests for float128 so > > that they are manually excluded on VxWorks systems. In looking at the > > code, I also added checks in check_effective_target_ppc_ieee128_ok to > > disable the systems that will never support VSX instructions which are > > required for float128 support (eabi, eabispe, darwin). > > It's just one extra to the big list here, but, why do we need all these > manual exclusions anyway? What is broken about the test itself? > > It would be so much more useful if the tests would help us, instead of > producing a lot of extra busy-work. Those lines were copied from other lines in the power7 era, and have just been copied since then. I agree it is perhaps time to remove these in GCC 13, but I would be hesitant to remove them now. I can not put in the eabi, eabispe and darwin lines in check_effective_target_ppc_ieee128_ok, and just add the vsxworks lines. With these changes can I check these in and then do a backport later?
On Thu, Apr 07, 2022 at 12:47:27PM +0200, Eric Botcazou wrote: > > I have run the tests on my usual Linux systems (little endian power10, > > little endian power9, big endian power8), but I don't have access to a > > VxWorks system. Eric does this fix the failure for you? > > Yes, if you add '*' at the end of the selector, i.e. [istarget *-*-vxworks*]. Ok. > > If it does fix the failure, can I apply the patch to the master branch and > > backport it to GCC 11 and GCC 10? Sorry about the breakage. > > That would be perfect, thanks in advance. > > -- > Eric Botcazou > >
Hi! On Thu, Apr 07, 2022 at 02:59:55PM -0400, Michael Meissner wrote: > On Thu, Apr 07, 2022 at 06:00:51AM -0500, Segher Boessenkool wrote: > > On Thu, Apr 07, 2022 at 12:29:45AM -0400, Michael Meissner wrote: > > > In PR target/104253, it was pointed out the that test case added as part > > > of fixing the PR does not work on VxWorks because float128 is not > > > supported on that system. I have modified the three tests for float128 so > > > that they are manually excluded on VxWorks systems. In looking at the > > > code, I also added checks in check_effective_target_ppc_ieee128_ok to > > > disable the systems that will never support VSX instructions which are > > > required for float128 support (eabi, eabispe, darwin). > > > > It's just one extra to the big list here, but, why do we need all these > > manual exclusions anyway? What is broken about the test itself? > > > > It would be so much more useful if the tests would help us, instead of > > producing a lot of extra busy-work. > > Those lines were copied from other lines in the power7 era, and have just been > copied since then. And never updated or given any (second) thought :-( > I agree it is perhaps time to remove these in GCC 13, but I > would be hesitant to remove them now. I can not put in the eabi, eabispe and > darwin lines in check_effective_target_ppc_ieee128_ok, and just add the > vsxworks lines. What I want to see is the tests (in target-supports) to just work, without manual intervention for targets that are even mildly interesting :-( (Btw, powerpc*-*-eabi* would be simpler, more compact, and more correct here!) > With these changes can I check these in and then do a backport later? Eric already approved it. It is fine with me of course, but I do want things to get better eventually! Segher
diff --git a/gcc/testsuite/lib/target-supports.exp b/gcc/testsuite/lib/target-supports.exp index ff8edbd3e17..a4142eaee27 100644 --- a/gcc/testsuite/lib/target-supports.exp +++ b/gcc/testsuite/lib/target-supports.exp @@ -2318,10 +2318,12 @@ proc check_ppc_mma_hw_available { } { proc check_ppc_float128_sw_available { } { return [check_cached_effective_target ppc_float128_sw_available { # Some simulators are known to not support VSX/power8/power9 - # instructions. For now, disable on Darwin. + # instructions. For now, disable on Darwin. Disable on VxWorks as + # well. if { [istarget powerpc-*-eabi] || [istarget powerpc*-*-eabispe] - || [istarget *-*-darwin*]} { + || [istarget *-*-darwin*] + || [istarget *-*-vxworks]} { expr 0 } else { set options "-mfloat128 -mvsx" @@ -2344,10 +2346,11 @@ proc check_ppc_float128_sw_available { } { proc check_ppc_float128_hw_available { } { return [check_cached_effective_target ppc_float128_hw_available { # Some simulators are known to not support VSX/power8/power9 - # instructions. For now, disable on Darwin. + # instructions. For now, disable on Darwin and VxWorks. if { [istarget powerpc-*-eabi] || [istarget powerpc*-*-eabispe] - || [istarget *-*-darwin*]} { + || [istarget *-*-darwin*] + || [istarget *-*-vxworks]} { expr 0 } else { set options "-mfloat128 -mvsx -mfloat128-hardware -mpower9-vector" @@ -2370,8 +2373,12 @@ proc check_ppc_float128_hw_available { } { # See if the __ieee128 keyword is understood. proc check_effective_target_ppc_ieee128_ok { } { return [check_cached_effective_target ppc_ieee128_ok { - # disable on AIX. - if { [istarget *-*-aix*] } { + # disable on AIX, Darwin, VxWorks and targets that don't support VSX. + if { [istarget *-*-aix*] + || [istarget powerpc-*-eabi] + || [istarget powerpc*-*-eabispe] + || [istarget *-*-darwin*] + || [istarget *-*-vxworks]} { expr 0 } else { set options "-mfloat128"