On Fri, Mar 25, 2022 at 06:15:56PM -0500, Peter Bergner wrote: > On 3/25/22 4:08 PM, Segher Boessenkool wrote: > > On Fri, Mar 25, 2022 at 02:51:38PM -0500, Peter Bergner wrote: > > It seems likely many of these tests should move to g++.target/powerpc . > > Probably, that can be a follow on patch. Maybe a good first patch for Surya.
Certainly, it should be a separate patch no matter what; just an obvious improvement, that your patch exposes :-) It is quite unusual to see -mcpu= in target-independent testcases: if this particular -mcpu= (or any other machine flag) exposes the problem, you typically want to build the testcase everywhere else anyway, for increased coverage :-) > > Those that should not should likely not use -mcpu= in the first place > > (instead, those tests should use has_arch_pwrN). > > If the test cases explicitly added -mcpu=, I'm guessing they need them > to test whatever the test case is checking for. If we remove the -mcpu= > and reply on dg-require has_arch_pwrN or whatever, then the test case > would only run whenever the default flags match that, right? So it > would seem we'd get less test coverage than before. It comes down to what the test wants to test. For target tests -m options are fine and usual, in most cases you want to test a bug or some code generation that only happens with those flags; for generic tests, -m options are unusual. > >> +++ b/gcc/testsuite/gcc.target/powerpc/int_128bit-runnable.c > >> +++ b/gcc/testsuite/gcc.target/powerpc/test_mffsl.c > > > > I missed these two in reviewing when the -mcpu= was introduced, oops. > > It's WAY too easy to miss those, since -mcpu= is such a common option > that we see and use everyday, we almost expect to see it, so it doesn't > look out of place or wrong. Yes it is very easy. But I trained myself pretty well apparently :-) > > Okay for trunk. Also okay for backports if you want / if you think it > > useful. Thanks! > > Thanks, commit pushed. I had not thought about backports, but if it > helps stabilize our test results there, it can't hurt. I'll have a > look and see if the tests even exist there or not. It is mostly useful because this causes many more things to be tested. In the grand scheme of things this is just a few tests, so if it is hard, just don't bother :-) Segher
