Re: [PATCH] Fix MMX/SSE/AVX* shifts by non-immediate scalar (PR target/80286)

[Jakub Jelinek <jakub at redhat dot com>]

On Thu, Apr 06, 2017 at 10:47:03AM +0200, Uros Bizjak wrote:
> > +(define_insn_and_split "*<shift_insn><mode>3<mask_name>_1"
> > +  [(set (match_operand:VI2_AVX2_AVX512BW 0 "register_operand")
> > +       (any_lshift:VI2_AVX2_AVX512BW
> > +         (match_operand:VI2_AVX2_AVX512BW 1 "register_operand")
> > +         (sign_extend:DI (match_operand:SI 2 "nonmemory_operand"))))]
> > +  "TARGET_SSE2 && <mask_mode512bit_condition> && <mask_avx512bw_condition>
> > +   && can_create_pseudo_p ()"
> > +  "#"
> > +  "&& 1"
> > +  [(set (match_dup 3) (zero_extend:DI (match_dup 2)))
> > +   (set (match_dup 0) (any_lshift:VI2_AVX2_AVX512BW
> > +                       (match_dup 1) (match_dup 3)))]
> > +{
> > +  operands[3] = gen_reg_rtx (DImode);
> > +})
> >
> Yes, something like this. You ca use any_extend instead of
> sign_extend, so the pattern will also remove possible zero_extend of
> count operand.

The pattern splits it immediately (during split1) into a zext + shift,
so unless we let the pattern survive in this form (but then we need
constraints and it is unclear which ones) after reload, I don't see
advantage in matching it for zext, it is split exactly to what there
used to be before.

> >           (match_operand:<avx512fmaskmode> 3 "register_operand" "Yk")))])
> > that is a transformation we want to do on the define_insn part of
> > define_insn_and_split, but not exactly what we want to do on the split
> > part of the insn - there we want literaly match_dup 0, match_dup 1,
> > and instead of the 2 other match_operand match_dup 2 and match_dup 3.
> 
> Hm, I'm not that versed in define_subst, but that looks quite a
> drawback of define_subst to me.

Perhaps, but we'd need to define what it means to subst a
define_insn_and_split.

	Jakub