Hi,
On 23/06/26 10:57 AM, Michael Meissner wrote:
> On Thu, Jun 11, 2026 at 02:33:57PM +0530, jeevitha wrote:
>> Hi All,
>>
>> The following changes have been bootstrapped and regression tested on
>> powerpc64le-linux. Is this okay for trunk?
>
> This is ok, with a nit that you can fix before checkin. However, there
> might be an issue in the tests down the road.
>
>> @@ -421,6 +421,9 @@
>> (and (eq_attr "isa" "p10")
>> (match_test "TARGET_POWER10"))
>> (const_int 1)
>> + (and (eq_attr "isa" "future")
>> + (match_test "TARGET_FUTURE"))
>> + (const_int 1)
>> ] (const_int 0)))
>>
>
> All of the other tests for setting "enabled" in rs6000.md have a blank
> line before the (and (eq_attr ..). This makes it a little easier to
> read. I.e. after the patch it should be:
>
> (and (eq_attr "isa" "p9tf")
> (match_test "FLOAT128_VECTOR_P (TFmode)"))
> (const_int 1)
>
> (and (eq_attr "isa" "p10")
> (match_test "TARGET_POWER10"))
> (const_int 1)
>
> (and (eq_attr "isa" "future")
> (match_test "TARGET_FUTURE"))
> (const_int 1)
>
> One note, that isn't an issue at the moment, given you ordered the
> xvadduwm, etc. code in the alternatives before the vadduwm pattern.
> But if we ever re-order the alternatives to prefer the altivec
> instruction first, these tests will fail.
If the alternatives get reordered, we do want the tests to fail so
that this change in behaviour is caught, esp if the alternatives got
reordered erroneously.
>
> The way I've dealt with this in the past is to use __asm__ to tell the
> register allocator that it should move the value to a traditional
> floating point register. I.e. code like:
>
> /* { dg-do compile } */
> /* { dg-options "-mdejagnu-cpu=future -O2" } */
> /* { dg-require-effective-target powerpc_future_compile_ok } */
>
> #include <altivec.h>
>
> typedef vector signed int v4si_t;
> typedef vector signed short v8hi_t;
> typedef vector signed long long v2di_t;
>
> v4si_t int_add (v4si_t x, v4si_t y)
> {
> /* force x & y into FPRs to prefer xvadduwm over vadduwm. */
> __asm__ (" # %x0,%x0" : "+f" (x), "+f" (y));
>
> return vec_add (x, y); /* xvadduwm */
We need two tests then: one to check if vec_add emits the
expected instruction, another to check if vec_add can emit
the xvadduwm instruction.
-Surya
> }
>
> Another minor note for +, -, and multiply, you might want to also
> verify that the vector arithmetic overloading generates the appropriate
> instructions, probably as a separate test. It should generate the
> correct code, but it doesn't hurt to have another test, just in case. Note,
> mulh can't be done:
>
>
> /* { dg-do compile } */
> /* { dg-options "-mdejagnu-cpu=future -O2" } */
> /* { dg-require-effective-target powerpc_future_compile_ok } */
>
> #include <altivec.h>
>
> typedef vector signed int v4si_t;
> typedef vector signed short v8hi_t;
> typedef vector signed long long v2di_t;
>
> v4si_t int_add (v4si_t x, v4si_t y)
> {
> /* force x & y into FPRs to prefer xvadduwm over vadduwm. */
> __asm__ (" # %x0,%x0" : "+f" (x), "+f" (y));
>
> return x + y; /* xvadduwm */
> }
>
> Finally in the tests with the options -mcpu=future -mno-vsx, please put
> a comment in the test to say that the -mno-vsx option turns off
> generation of the VSX instructions (like xvadduwm) in favor of the
> traditional Altivec instructions (like vadduwm).
>
> I don't see that these changes to the tests requires another patch
> review cycle.
>
