[Bug target/111645] New: Intrinsics vec_sldb /vec_srdb fail with __vector unsigned __int128

[Bug target/111645] New: Intrinsics vec_sldb /vec_srdb fail with __vector unsigned __int128

[munroesj at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=111645

            Bug ID: 111645
           Summary: Intrinsics vec_sldb /vec_srdb fail with __vector
                    unsigned __int128
           Product: gcc
           Version: 13.2.1
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: target
          Assignee: unassigned at gcc dot gnu.org
          Reporter: munroesj at gcc dot gnu.org
  Target Milestone: ---

Created attachment 56018
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=56018&action=edit
example of the problem. Compile with  gcc -m64 -O3 -mcpu=power10 -c sldbi.c

GCC 12 and 13 fail to compile vector intrisic vec_sldb / vec_srdb as required
by the Power Vector Intrinsic Programming Reference.

error: invalid parameter combination for AltiVec intrinsic ‘__builtin_vec_sldb’

Both the Programming Reference and the GCC documentation state that vector
(unsigned/signed) __int128 are valid operands. But they fail with a 

error: invalid parameter combination for AltiVec intrinsic ‘__builtin_vec_sldb’
or
error: invalid parameter combination for AltiVec intrinsic ‘__builtin_vec_srdb’

[Bug target/111645] Intrinsics vec_sldb /vec_srdb fail with __vector unsigned __int128

[bergner at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=111645

Peter Bergner <bergner at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
     Ever confirmed|0                           |1
             Status|UNCONFIRMED                 |NEW
   Last reconfirmed|                            |2023-09-30
                 CC|                            |bergner at gcc dot gnu.org,
                   |                            |dje at gcc dot gnu.org,
                   |                            |linkw at gcc dot gnu.org,
                   |                            |meissner at gcc dot gnu.org,
                   |                            |segher at gcc dot gnu.org

--- Comment #1 from Peter Bergner <bergner at gcc dot gnu.org> ---
I see that we have created built-in overloads for signed and unsigned vector
char through vector long long.  That said, the rs6000-builtins.def only seems
to support the signed vector types though, which is why you're seeing an error.
 So confirmed.

That said, I believe your 3rd argument needs to be a real constant integer,
since the vsldbi instruction requires that.  It doesn't allow for a const int
variable.  I notice some older (not trunk) gcc versions are ICEing with that,
so another bug to look at.

I do not see any documentation that says we support the vector __int128 type. 
Where exactly did you see that?  However, from the instruction description, it
seems like the hw instruction could support that.

[Bug target/111645] Intrinsics vec_sldb /vec_srdb fail with __vector unsigned __int128

[munroesj at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=111645

Steven Munroe <munroesj at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #56018|0                           |1
        is obsolete|                            |

--- Comment #2 from Steven Munroe <munroesj at gcc dot gnu.org> ---
Created attachment 56019
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=56019&action=edit
Updated test case with static inline functions

[Bug target/111645] Intrinsics vec_sldb /vec_srdb fail with __vector unsigned __int128

[munroesj at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=111645

--- Comment #3 from Steven Munroe <munroesj at gcc dot gnu.org> ---
(In reply to Peter Bergner from comment #1)
> I see that we have created built-in overloads for signed and unsigned vector
> char through vector long long.  That said, the rs6000-builtins.def only
> seems to support the signed vector types though, which is why you're seeing
> an error.  So confirmed.
> 
> That said, I believe your 3rd argument needs to be a real constant integer,
> since the vsldbi instruction requires that.  It doesn't allow for a const
> int variable.  I notice some older (not trunk) gcc versions are ICEing with
> that, so another bug to look at.
The original code is static inline, so the const int parm should transfer
intact to the builtin const.

It seems I over-simplified the deduced test case.
> 
> I do not see any documentation that says we support the vector __int128
> type.  Where exactly did you see that?  However, from the instruction
> description, it seems like the hw instruction could support that.

I stand corrected. The documentation only describes vector unsigned long long.
But the instruction is like vsldoi and does not really care what the type is.

[Bug target/111645] Intrinsics vec_sldb /vec_srdb fail with __vector unsigned __int128

[munroesj at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=111645

--- Comment #4 from Steven Munroe <munroesj at gcc dot gnu.org> ---
Actually shift/rotate intrinsic: ,vec_rl, vec_rlmi, vec_rlnm, vec_sl, vec_sr,
vec_sra

Support vector __int128 as required for the PowerISA 3.1 POWER vector
shift/rotate quadword instructions 

But: vec_sld, vec_sldb, vec_sldw, vec_sll, vec_slo, vec_srdb, vec_srl, vec_sro

Do not. 

There is no obvious reason for this inconstancy as the target instructions are
effectively 128/256-bit operations returning a 128-bit result.The type of the
inputs is incidental to the operation.

Any restrictions imposed by the original Altivec.h PIM was broken long ago by
VSX and PowerISA 2.07.

Net: the Power Vector Intrinsic Programming Reference and the compilers should
support the vector __int128 type for any instruction where it makes sense as a
input or result.

[Bug target/111645] Intrinsics vec_sldb /vec_srdb fail with __vector unsigned __int128

[carll at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=111645

Carl Love <carll at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |carll at gcc dot gnu.org

--- Comment #5 from Carl Love <carll at gcc dot gnu.org> ---

There are a couple of issues with the test case in the attachment.  For example
one of the tests is:


static inline vui64_t
vec_vsldbi_64 (vui64_t vra, vui64_t vrb, const unsigned int shb)
{
 return vec_sldb (vra, vrb, shb);
}

When I tried to compile it, it seemed to compile.  However if I take off the
static inline, then I get an error about in compatible arguments.  The built-in
requires an explicit integer be based in the third argument.  The following
worked for me:


static inline vui64_t
vec_vsldbi_64 (vui64_t vra, vui64_t vrb, const unsigned int shb)
{
 return vec_sldb (vra, vrb, 1);
}

The compiler/assembler needs an explicit value for the third argument as it has
to generate the instruction with the immediate shift value as part of the
instruction.  Hence a variable for the third argument will not work.

Agreed that the __int128 arguments can and should be supported.  Patch to add
that support is in progress but will require getting the LLVM/OpenXL team to
agree to adding the __128int variants as well.

[Bug target/111645] Intrinsics vec_sldb /vec_srdb fail with __vector unsigned __int128

[munroesj at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=111645

--- Comment #6 from Steven Munroe <munroesj at gcc dot gnu.org> ---
(In reply to Carl Love from comment #5)
> There are a couple of issues with the test case in the attachment.  For
> example one of the tests is:
> 
> 
> static inline vui64_t
> vec_vsldbi_64 (vui64_t vra, vui64_t vrb, const unsigned int shb)
> {
>  return vec_sldb (vra, vrb, shb);
> }
> 
> When I tried to compile it, it seemed to compile.  However if I take off the
> static inline, then I get an error about in compatible arguments.  The
> built-in requires an explicit integer be based in the third argument.  The
> following worked for me:
> 
> 
> static inline vui64_t
> vec_vsldbi_64 (vui64_t vra, vui64_t vrb, const unsigned int shb)
> {
>  return vec_sldb (vra, vrb, 1);
> }
> 
> The compiler/assembler needs an explicit value for the third argument as it
> has to generate the instruction with the immediate shift value as part of
> the instruction.  Hence a variable for the third argument will not work.
> 
> Agreed that the __int128 arguments can and should be supported.  Patch to
> add that support is in progress but will require getting the LLVM/OpenXL
> team to agree to adding the __128int variants as well.

Yes I know. in the PVECLIB case these functions will always be static inline.
So this is not issue for me.