On Tue, Oct 24, 2017 at 05:44:44AM -0700, H.J. Lu wrote:
> > What I can see from config/atom.md:
> > ;; if palignr or psrldq
> > (define_insn_reservation "atom_sseishft_2" 1
> > (and (eq_attr "cpu" "atom")
> > (and (eq_attr "type" "sseishft")
> > (and (eq_attr "atom_unit" "sishuf")
> > (match_operand 2 "immediate_operand"))))
> > "atom-simple-0")
> >
> > This leads back to initial commit of atom.md.
> > So, discrimination of psrldq and pslldq looks intentional.
> >
> > On the over hand, I see in Software Optimization Guide, Table 14-2 that
> > PSRLDQ and PSLLDQ occupy same line which directs both insns to port-0 (p
> > 14-18).
> > So, looking from that point, definition for PSLLDQ which allow either of
> > port-0
> > and port-1 looks wrong (atom-simple-either reservation).
> >
> > In absence of other information, I'd play on safe side and leave things as
> > they
> > occur right now.
> >
>
> I prefer to leave atom.md ASIS. As for (set_attr "atom_unit"
> "sishuf"), it was added
> for
>
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=44615
>
> You can drop (set_attr "atom_unit" "sishuf") if gcc.target/i386/sse2-vec-2a.c
> still compiles.
No, it was added earlier than that, that PR was about insns with psrldq with
implicit immediate (which don't have a CONST_INT operands[2]). This insn
does have it, the testcase passes regardless of whether sishuf or other is
used, it is purely a tuning thing.
In any case, here is an updated patch that just preserves the status quo
(psrldq having the sishuf unit, pslldq not) using a simple code attribute.
2017-10-24 Jakub Jelinek <ja...@redhat.com>
PR target/82370
* config/i386/sse.md (VIMAX_AVX2): Remove V4TImode.
(VIMAX_AVX2_AVX512BW, VIMAX_AVX512VL): New mode iterators.
(vec_shl_<mode>): Remove unused expander.
(avx512bw_<shift_insn><mode>3): New define_insn.
(atom_shift_unit): New code iterator.
(<sse2_avx2>_ashl<mode>3, <sse2_avx2>_lshr<mode>3): Replaced by ...
(<sse2_avx2>_<shift_insn><mode>3): ... this. New define_insn.
* gcc.target/i386/pr82370.c: New test.
--- gcc/config/i386/sse.md.jj 2017-10-20 16:30:35.286208652 +0200
+++ gcc/config/i386/sse.md 2017-10-24 16:29:54.848934888 +0200
@@ -371,10 +371,17 @@ (define_mode_iterator V16FI
[V16SF V16SI])
;; ??? We should probably use TImode instead.
-(define_mode_iterator VIMAX_AVX2
+(define_mode_iterator VIMAX_AVX2_AVX512BW
[(V4TI "TARGET_AVX512BW") (V2TI "TARGET_AVX2") V1TI])
-;; ??? This should probably be dropped in favor of VIMAX_AVX2.
+;; Suppose TARGET_AVX512BW as baseline
+(define_mode_iterator VIMAX_AVX512VL
+ [V4TI (V2TI "TARGET_AVX512VL") (V1TI "TARGET_AVX512VL")])
+
+(define_mode_iterator VIMAX_AVX2
+ [(V2TI "TARGET_AVX2") V1TI])
+
+;; ??? This should probably be dropped in favor of VIMAX_AVX2_AVX512BW.
(define_mode_iterator SSESCALARMODE
[(V4TI "TARGET_AVX512BW") (V2TI "TARGET_AVX2") TI])
@@ -10778,9 +10785,9 @@ (define_insn "<shift_insn><mode>3<mask_n
(set_attr "mode" "<sseinsnmode>")])
-(define_expand "vec_shl_<mode>"
+(define_expand "vec_shr_<mode>"
[(set (match_dup 3)
- (ashift:V1TI
+ (lshiftrt:V1TI
(match_operand:VI_128 1 "register_operand")
(match_operand:SI 2 "const_0_to_255_mul_8_operand")))
(set (match_operand:VI_128 0 "register_operand") (match_dup 4))]
@@ -10791,48 +10798,26 @@ (define_expand "vec_shl_<mode>"
operands[4] = gen_lowpart (<MODE>mode, operands[3]);
})
-(define_insn "<sse2_avx2>_ashl<mode>3"
- [(set (match_operand:VIMAX_AVX2 0 "register_operand" "=x,v")
- (ashift:VIMAX_AVX2
- (match_operand:VIMAX_AVX2 1 "register_operand" "0,v")
- (match_operand:SI 2 "const_0_to_255_mul_8_operand" "n,n")))]
- "TARGET_SSE2"
+(define_insn "avx512bw_<shift_insn><mode>3"
+ [(set (match_operand:VIMAX_AVX512VL 0 "register_operand" "=v")
+ (any_lshift:VIMAX_AVX512VL
+ (match_operand:VIMAX_AVX512VL 1 "nonimmediate_operand" "vm")
+ (match_operand:SI 2 "const_0_to_255_mul_8_operand" "n")))]
+ "TARGET_AVX512BW"
{
operands[2] = GEN_INT (INTVAL (operands[2]) / 8);
-
- switch (which_alternative)
- {
- case 0:
- return "pslldq\t{%2, %0|%0, %2}";
- case 1:
- return "vpslldq\t{%2, %1, %0|%0, %1, %2}";
- default:
- gcc_unreachable ();
- }
+ return "vp<vshift>dq\t{%2, %1, %0|%0, %1, %2}";
}
- [(set_attr "isa" "noavx,avx")
- (set_attr "type" "sseishft")
+ [(set_attr "type" "sseishft")
(set_attr "length_immediate" "1")
- (set_attr "prefix_data16" "1,*")
- (set_attr "prefix" "orig,vex")
+ (set_attr "prefix" "maybe_evex")
(set_attr "mode" "<sseinsnmode>")])
-(define_expand "vec_shr_<mode>"
- [(set (match_dup 3)
- (lshiftrt:V1TI
- (match_operand:VI_128 1 "register_operand")
- (match_operand:SI 2 "const_0_to_255_mul_8_operand")))
- (set (match_operand:VI_128 0 "register_operand") (match_dup 4))]
- "TARGET_SSE2"
-{
- operands[1] = gen_lowpart (V1TImode, operands[1]);
- operands[3] = gen_reg_rtx (V1TImode);
- operands[4] = gen_lowpart (<MODE>mode, operands[3]);
-})
+(define_code_attr atom_shift_unit [(ashift "*") (lshiftrt "sishuf")])
-(define_insn "<sse2_avx2>_lshr<mode>3"
+(define_insn "<sse2_avx2>_<shift_insn><mode>3"
[(set (match_operand:VIMAX_AVX2 0 "register_operand" "=x,v")
- (lshiftrt:VIMAX_AVX2
+ (any_lshift:VIMAX_AVX2
(match_operand:VIMAX_AVX2 1 "register_operand" "0,v")
(match_operand:SI 2 "const_0_to_255_mul_8_operand" "n,n")))]
"TARGET_SSE2"
@@ -10842,9 +10827,9 @@ (define_insn "<sse2_avx2>_lshr<mode>3"
switch (which_alternative)
{
case 0:
- return "psrldq\t{%2, %0|%0, %2}";
+ return "p<vshift>dq\t{%2, %0|%0, %2}";
case 1:
- return "vpsrldq\t{%2, %1, %0|%0, %1, %2}";
+ return "vp<vshift>dq\t{%2, %1, %0|%0, %1, %2}";
default:
gcc_unreachable ();
}
@@ -10852,7 +10837,7 @@ (define_insn "<sse2_avx2>_lshr<mode>3"
[(set_attr "isa" "noavx,avx")
(set_attr "type" "sseishft")
(set_attr "length_immediate" "1")
- (set_attr "atom_unit" "sishuf")
+ (set_attr "atom_unit" "<atom_shift_unit>")
(set_attr "prefix_data16" "1,*")
(set_attr "prefix" "orig,vex")
(set_attr "mode" "<sseinsnmode>")])
--- gcc/testsuite/gcc.target/i386/pr82370.c.jj 2017-10-24 16:22:16.665464886
+0200
+++ gcc/testsuite/gcc.target/i386/pr82370.c 2017-10-24 16:22:16.665464886
+0200
@@ -0,0 +1,18 @@
+/* PR target/82370 */
+/* { dg-do compile } */
+/* { dg-options "-O2 -mavx512vl -mavx512bw -masm=att" } */
+/* { dg-final { scan-assembler-times "vpslldq\[ \t]\+\\\$5,
\\(%\[a-z0-9,]*\\), %xmm\[0-9]\+" 1 } } */
+/* { dg-final { scan-assembler-times "vpsrldq\[ \t]\+\\\$5,
\\(%\[a-z0-9,]*\\), %xmm\[0-9]\+" 1 } } */
+/* { dg-final { scan-assembler-times "vpslldq\[ \t]\+\\\$5,
\\(%\[a-z0-9,]*\\), %ymm\[0-9]\+" 1 } } */
+/* { dg-final { scan-assembler-times "vpsrldq\[ \t]\+\\\$5,
\\(%\[a-z0-9,]*\\), %ymm\[0-9]\+" 1 } } */
+/* { dg-final { scan-assembler-times "vpslldq\[ \t]\+\\\$5,
\\(%\[a-z0-9,]*\\), %zmm\[0-9]\+" 1 } } */
+/* { dg-final { scan-assembler-times "vpsrldq\[ \t]\+\\\$5,
\\(%\[a-z0-9,]*\\), %zmm\[0-9]\+" 1 } } */
+
+#include <x86intrin.h>
+
+__m512i f1 (__m512i *x) { return _mm512_bslli_epi128 (*x, 5); }
+__m512i f2 (__m512i *x) { return _mm512_bsrli_epi128 (*x, 5); }
+__m256i f3 (__m256i *x) { return _mm256_bslli_epi128 (*x, 5); }
+__m256i f4 (__m256i *x) { return _mm256_bsrli_epi128 (*x, 5); }
+__m128i f5 (__m128i *x) { return _mm_bslli_si128 (*x, 5); }
+__m128i f6 (__m128i *x) { return _mm_bsrli_si128 (*x, 5); }
Jakub
