On Sat, Oct 30, 2021 at 11:16:41AM +0100, Roger Sayle wrote:
> 2021-10-30 Roger Sayle <ro...@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
