On Mon, Aug 18, 2014 at 8:30 PM, Kirill Yukhin <kirill.yuk...@gmail.com> wrote:
>> > (define_insn "<avx2_avx512bw>_ashrv<mode><mask_name>"): New. >> >> It looks to me that the macroization is somehow wrong for ashrv. I'd >> split the mode iterator to: >> >> > +(define_mode_iterator VI248_AVX512 >> > + [(V16SI "TARGET_AVX512F") (V8SI "TARGET_AVX2") (V4SI "TARGET_AVX2") >> > + (V32HI "TARGET_AVX512BW") >> > + (V16HI "TARGET_AVX512BW && TARGET_AVX512VL") >> > + (V8HI "TARGET_AVX512BW && TARGET_AVX512VL") >> > + (V8DI "TARGET_AVX512F") (V4DI "TARGET_AVX512VL") (V2DI >> > "TARGET_AVX512VL")]) >> > + >> >> V4SI, V8SI, V16SI (AVX512F), V8DI (AVX512F), V4DI (AVX512VL), V2DI >> (AVX512VL), with AVX2 as the baseline >> >> and >> >> V32HI, V16HI (AVX512VL), V8HI (AVX512VL), with AVX512BW as the baseline > Agreed. After doing `make mddump' I can see that I've enabled > vpsavw for AVX2. > > I (hopefully) implemented the proposal. Thanks for review! > > Is it ok now? Yes, this looks much better! I hope that some simple logic behind AVX512VL and AVX512BW (etc) combinations can be found. At least by using simple insn constraint and simple mode constraints, one can find which modes are supported by the pattern in a much easier way. Please also use the same approach in your future patches (using simple insn and mode constraints). We can always merge patterns later, if any additional macroization possibilities would be found. The patch is OK. BTW: Taking these new findings into account, is it possible to split V_AVX512VL mode iterator in the same way to avoid compound conditions in the mode iterator? Thanks, Uros.
