Hi,
on 2024/6/18 00:08, Peter Bergner wrote:
> On 6/14/24 1:37 PM, Carl Love wrote:
>> Per the additional feedback after patch:
>>
>> commit c892525813c94b018464d5a4edc17f79186606b7
>> Author: Carl Love <c...@linux.ibm.com>
>> Date: Tue Jun 11 14:01:16 2024 -0400
>>
>> rs6000, altivec-2-runnable.c should be a runnable test
>>
>> The test case has "dg-do compile" set not "dg-do run" for a runnable
>> test. This patch changes the dg-do command argument to run.
>>
>> gcc/testsuite/ChangeLog:gcc/testsuite/ChangeLog:
>> * gcc.target/powerpc/altivec-2-runnable.c: Change dg-do
>> argument to run.
>
> Test case altivec-1-runnable.c seems to have the same issue, in that it
> is currently a dg-do compile test case rather than the intended dg-do run.
Good catch!
> Can you have a look at changing that to dg-do run too? My guess it that
> this one will want something similar to some other altivec test cases, ala:
>
> /* { dg-do run { target vmx_hw } } */
> /* { dg-do compile { target { ! vmx_hw } } } */
> /* { dg-require-effective-target powerpc_altivec_ok } */
> /* { dg-options "-O2 -maltivec -mabi=altivec" } */
I'd expect the "-runnable" test case focuses on testing for run. Normally,
the one without "-runnable" would focus on testing for compiling (scan some
desired insn), but this altivec-1.c and altivec-1-runnable.c seems to test
for different things, maybe we should separate them into different names
if they don't test for a same test point.
>
> That said, I don't like not having a -mdejagnu-cpu=... here.
> I think for our server cpus, this is fine, but on an embedded system
> with a old ISA default for -mcpu=... (so we be doing a dg-do compile),
> just adding -maltivec to that default may not make much sense for that
> default and probably should be an error. Maybe something like:
Yes, for some embedded cpus, there will be some error messages, but since
we have powerpc_altivec_ok effective target, the error would make that
effective target checking fail so I'd expect it'll stop it being tested
(unsupported).
>
> /* { dg-do run { target vmx_hw } } */
> /* { dg-do compile { target { ! vmx_hw } } } */
> /* { dg-require-effective-target powerpc_altivec_ok } */
> /* { dg-options "-O2 -mdejagnu=power7" } */
>
> ...makes more sense? Ke Wen & Segher, thoughts on that?
> Ke Wen, should powerpc_altivec_ok be powerpc_altivec here???
Yes, I just pushed r15-1390 for this change.
BR,
Kewen
