[PATCH Take #2] x86_64: Expand ashrv1ti (and PR target/102986)

[Roger Sayle]

Very many thanks to Jakub for proof-reading my patch, catching my silly
GNU-style
mistakes and making excellent suggestions.  This revised patch incorporates
all of
his feedback, and has been tested on x86_64-pc-linux-gnu with make bootstrap
and
make -k check with no new failures.

2021-10-31  Roger Sayle  <roger@nextmovesoftware.com>
	    Jakub Jelinek  <jakub@redhat.com>

gcc/ChangeLog
	PR target/102986
	* config/i386/i386-expand.c (ix86_expand_v1ti_to_ti,
	ix86_expand_ti_to_v1ti): New helper functions.
	(ix86_expand_v1ti_shift): Check if the amount operand is an
	integer constant, and expand as a TImode shift if it isn't.
	(ix86_expand_v1ti_rotate): Check if the amount operand is an
	integer constant, and expand as a TImode rotate if it isn't.
	(ix86_expand_v1ti_ashiftrt): New function to expand arithmetic
	right shifts of V1TImode quantities.
	* config/i386/i386-protos.h (ix86_expand_v1ti_ashift): Prototype.
	* config/i386/sse.md (ashlv1ti3, lshrv1ti3): Change constraints
	to QImode general_operand, and let the helper functions lower
	shifts by non-constant operands, as TImode shifts.  Make
	conditional on TARGET_64BIT.
	(ashrv1ti3): New expander calling ix86_expand_v1ti_ashiftrt.
	(rotlv1ti3, rotrv1ti3): Change shift operand to QImode.
	Make conditional on TARGET_64BIT.

gcc/testsuite/ChangeLog
	PR target/102986
	* gcc.target/i386/sse2-v1ti-ashiftrt-1.c: New test case.
	* gcc.target/i386/sse2-v1ti-ashiftrt-2.c: New test case.
	* gcc.target/i386/sse2-v1ti-ashiftrt-3.c: New test case.
	* gcc.target/i386/sse2-v1ti-shift-2.c: New test case.
	* gcc.target/i386/sse2-v1ti-shift-3.c: New test case.

Thanks.
Roger
--

-----Original Message-----
From: Jakub Jelinek <jakub@redhat.com> 
Sent: 30 October 2021 11:30
To: Roger Sayle <roger@nextmovesoftware.com>
Cc: 'GCC Patches' <gcc-patches@gcc.gnu.org>; 'Uros Bizjak'
<ubizjak@gmail.com>
Subject: Re: [PATCH] x86_64: Expand ashrv1ti (and PR target/102986)

On Sat, Oct 30, 2021 at 11:16:41AM +0100, Roger Sayle wrote:
> 2021-10-30  Roger Sayle  <roger@nextmovesoftware.com>
> 
> gcc/ChangeLog
> 	PR target/102986
> 	* config/i386/i386-expand.c (ix86_expand_v1ti_to_ti,
> 	ix86_expand_ti_to_v1ti): New helper functions.
> 	(ix86_expand_v1ti_shift): Check if the amount operand is an
> 	integer constant, and expand as a TImode shift if it isn't.
> 	(ix86_expand_v1ti_rotate): Check if the amount operand is an
> 	integer constant, and expand as a TImode rotate if it isn't.
> 	(ix86_expand_v1ti_ashiftrt): New function to expand arithmetic
> 	right shifts of V1TImode quantities.
> 	* config/i386/i386-protos.h (ix86_expand_v1ti_ashift): Prototype.
> 	* config/i386/sse.md (ashlv1ti3, lshrv1ti3): Change constraints
> 	to QImode general_operand, and let the helper functions lower
> 	shifts by non-constant operands, as TImode shifts.
> 	(ashrv1ti3): New expander calling ix86_expand_v1ti_ashiftrt.
> 	(rotlv1ti3, rotrv1ti3): Change shift operand to QImode.
> 
> gcc/testsuite/ChangeLog
> 	PR target/102986
> 	* gcc.target/i386/sse2-v1ti-ashiftrt-1.c: New test case.
> 	* gcc.target/i386/sse2-v1ti-ashiftrt-2.c: New test case.
> 	* gcc.target/i386/sse2-v1ti-ashiftrt-3.c: New test case.
> 	* gcc.target/i386/sse2-v1ti-shift-2.c: New test case.
> 	* gcc.target/i386/sse2-v1ti-shift-3.c: New test case.
> 
> Sorry again for the breakage in my last patch.   I wasn't testing things
> that shouldn't have been affected/changed.

Not a review, will defer that to Uros, but just nits:

> +/* Expand move of V1TI mode register X to a new TI mode register.  */ 
> +static rtx ix86_expand_v1ti_to_ti (rtx x)

ix86_expand_v1ti_to_ti should be at the start of next line, so static rtx
ix86_expand_v1ti_to_ti (rtx x)

Ditto for other functions and also in functions you've added by the previous
patch.
> +      emit_insn (code == ASHIFT ? gen_ashlti3(tmp2, tmp1, operands[2])
> +				: gen_lshrti3(tmp2, tmp1, operands[2]));

Space before ( twice.

> +      emit_insn (code == ROTATE ? gen_rotlti3(tmp2, tmp1, operands[2])
> +				: gen_rotrti3(tmp2, tmp1, operands[2]));

Likewise.

> +      emit_insn (gen_ashrti3(tmp2, tmp1, operands[2]));

Similarly.

Also, I wonder for all these patterns (previously and now added), shouldn't
they have && TARGET_64BIT in conditions?  I mean, we don't really support
scalar TImode for ia32, but VALID_SSE_REG_MODE includes V1TImode and while
the constant shifts can be done, I think the variable shifts can't, there
are no TImode shift patterns...

	Jakub

-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: patchv4.txt
URL: <https://gcc.gnu.org/pipermail/gcc-patches/attachments/20211031/118a74a0/attachment-0001.txt>