| Message ID | 20170627235321.GA13753@ibm-tiger.the-meissners.org |
|---|---|
| State | New |
| Headers | show |
On 06/27/2017 05:53 PM, Michael Meissner wrote: > The PowerPC __builtin_cpu_supports and __builtin_cpu_is built-in functions > require GLIBC 2.23, since they use fixed words at the end of thread control > area to store the HWCAP and HWCAP2 bits. If the compiler was not configured > with the appropriate GLIBC, the compiler will generate a 0 as the result of the > built-in function call. > > I've been adding the target_clone attribute support to GCC, and the resolver > function uses __builtin_cpu_supports to detect which hardware ISA is being > used. On systems with an older GLIBC, only the default clone function will get > called because __builtin_cpu_supports returns 0. > > This adds a target supports option in dejagnu so that future tests can use this > to determine whether or not to test target_clones. > > I have verified that this patch works with the patches I plan to submit > tomorrow for enhancing the PowerPC target_clone support. > > Can I install this into the trunk? > > Given that GCC 7 supports __builtin_cpu_is and __builtin_cpu_supports, I would > ask if I could backport this to GCC 7.x as well to allow future tests to be > back ported. > > 2017-06-27 Michael Meissner <meissner@linux.vnet.ibm.com> > > PR target/81193 > * lib/target-supports.exp > (check_ppc_cpu_supports_hw_available): New test to make sure > __builtin_cpu_supports works on power7 and newer. OK for the trunk. It's not my call on the release branches though. jeff
On 6/27/17 6:53 PM, Michael Meissner wrote: > This adds a target supports option in dejagnu so that future tests can use this > to determine whether or not to test target_clones. I like the idea, but some comments... > + #ifdef __MACH__ > + asm volatile ("xxlor vs0,vs0,vs0"); > + #else > + asm volatile ("xxlor 0,0,0"); > + #endif What is this hunk for? We're only interested in the return value from the builtin below, correct? > + if (!__builtin_cpu_supports ("vsx")) > + return 1; > + return 0; ...and more importantly, why limit us to testing "vsx"? Why not test for "ppc32", which should be true for *all* kernels we'd ever run on, including ppc32, ppc64 and ppc64le? Peter
On Wed, Jun 28, 2017 at 09:58:40AM -0500, Peter Bergner wrote: > On 6/27/17 6:53 PM, Michael Meissner wrote: > > This adds a target supports option in dejagnu so that future tests can use this > > to determine whether or not to test target_clones. > > I like the idea, but some comments... > > > > > + #ifdef __MACH__ > > + asm volatile ("xxlor vs0,vs0,vs0"); > > + #else > > + asm volatile ("xxlor 0,0,0"); > > + #endif > > What is this hunk for? We're only interested in the return value from > the builtin below, correct? > > > > > + if (!__builtin_cpu_supports ("vsx")) > > + return 1; > > + return 0; > > ...and more importantly, why limit us to testing "vsx"? Why not test > for "ppc32", which should be true for *all* kernels we'd ever run on, > including ppc32, ppc64 and ppc64le? I wanted to make sure the test actually returned true in the correct test. I.e. guarantee that the machine had VSX instructions (i.e. the asm) and that __builtin_cpu_supports returned true. But if you wanted to use a previous ISA flag, feel free to submit patches. I just wanted a quick way to make sure the clone tests were run properly when GCC was configured with a 2.23 GLIBC or newer.
Hi! On Tue, Jun 27, 2017 at 07:53:21PM -0400, Michael Meissner wrote: > --- gcc/testsuite/lib/target-supports.exp (revision 249606) > +++ gcc/testsuite/lib/target-supports.exp (working copy) > @@ -1930,6 +1930,37 @@ proc check_effective_target_powerpc64_no > } {-O2}] > } > > +# Return 1 if the target supports the __builtin_cpu_supports built-in, > +# including having a new enough library to support the test. Cache the result. > +# Require at least a power7 to run on. > + > +proc check_ppc_cpu_supports_hw_available { } { This probably shouldn't have "ppc" in the name? Or "hw"? > + return [check_cached_effective_target ppc_cpu_supports_hw_available { > + # Some simulators are known to not support VSX/power8 instructions. > + # For now, disable on Darwin > + if { [istarget powerpc-*-eabi] > + || [istarget powerpc*-*-eabispe] > + || [istarget *-*-darwin*]} { > + expr 0 > + } else { > + set options "-mvsx" > + check_runtime_nocache ppc_cpu_supports_hw_available { > + int main() > + { > + #ifdef __MACH__ > + asm volatile ("xxlor vs0,vs0,vs0"); > + #else > + asm volatile ("xxlor 0,0,0"); > + #endif > + if (!__builtin_cpu_supports ("vsx")) > + return 1; > + return 0; > + } > + } $options > + } > + }] > +} As Peter said, I'd rather test for "ppc32", so this works anywhere. That would give proc check_cpu_supports_available { } { if { [istarget powerpc*-*-*] } { return [check_runtime cpu_supports_available { int main() { return !__builtin_cpu_supports ("ppc32"); } }] } return 0 } (and other archs can add their stuff then). Why did you use check_runtime_nocache btw? Is that just copy-paste? Segher
On Wed, Jun 28, 2017 at 03:48:50PM -0500, Segher Boessenkool wrote: > As Peter said, I'd rather test for "ppc32", so this works anywhere. Fair enough. > That would give > > proc check_cpu_supports_available { } { > if { [istarget powerpc*-*-*] } { > return [check_runtime cpu_supports_available { > int main() { return !__builtin_cpu_supports ("ppc32"); } > }] > } > > return 0 > } > > (and other archs can add their stuff then). > > Why did you use check_runtime_nocache btw? Is that just copy-paste? Just copy-paste. Like the target_clones stuff, right now, it is only x86 and PowerPC that supports __builtin_cpu*. I don't really see the point of having a machine independent test for __builtin_cpu_*, but if you feel strongly about it go for it.
On Wed, Jun 28, 2017 at 05:04:37PM -0400, Michael Meissner wrote: > > Why did you use check_runtime_nocache btw? Is that just copy-paste? > > Just copy-paste. > > Like the target_clones stuff, right now, it is only x86 and PowerPC that > supports __builtin_cpu*. > > I don't really see the point of having a machine independent test for > __builtin_cpu_*, but if you feel strongly about it go for it. Why have divergent names for things, if all targets could use the same? Yes I realise almost all uses will be in target-specific testcases. Getting the interface right up front is more important than the implementation, it's a pain to change the interface later (have to modify a lot of testcases, etc.) Segher
Index: gcc/testsuite/lib/target-supports.exp =================================================================== --- gcc/testsuite/lib/target-supports.exp (revision 249606) +++ gcc/testsuite/lib/target-supports.exp (working copy) @@ -1930,6 +1930,37 @@ proc check_effective_target_powerpc64_no } {-O2}] } +# Return 1 if the target supports the __builtin_cpu_supports built-in, +# including having a new enough library to support the test. Cache the result. +# Require at least a power7 to run on. + +proc check_ppc_cpu_supports_hw_available { } { + return [check_cached_effective_target ppc_cpu_supports_hw_available { + # Some simulators are known to not support VSX/power8 instructions. + # For now, disable on Darwin + if { [istarget powerpc-*-eabi] + || [istarget powerpc*-*-eabispe] + || [istarget *-*-darwin*]} { + expr 0 + } else { + set options "-mvsx" + check_runtime_nocache ppc_cpu_supports_hw_available { + int main() + { + #ifdef __MACH__ + asm volatile ("xxlor vs0,vs0,vs0"); + #else + asm volatile ("xxlor 0,0,0"); + #endif + if (!__builtin_cpu_supports ("vsx")) + return 1; + return 0; + } + } $options + } + }] +} + # Return 1 if the target supports executing power8 vector instructions, 0 # otherwise. Cache the result. @@ -6922,6 +6953,7 @@ proc is-effective-target { arg } { "ppc_float128_sw" { set selected [check_ppc_float128_sw_available] } "ppc_float128_hw" { set selected [check_ppc_float128_hw_available] } "ppc_recip_hw" { set selected [check_ppc_recip_hw_available] } + "ppc_cpu_supports_hw" { set selected [check_ppc_cpu_supports_hw_available] } "dfp_hw" { set selected [check_dfp_hw_available] } "htm_hw" { set selected [check_htm_hw_available] } "named_sections" { set selected [check_named_sections_available] }