This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH i386 AVX512] [56/n] Add plus/minus/abs/neg/andnot insn patterns.
- From: Uros Bizjak <ubizjak at gmail dot com>
- To: Jakub Jelinek <jakub at redhat dot com>
- Cc: Kirill Yukhin <kirill dot yukhin at gmail dot com>, Richard Henderson <rth at redhat dot com>, GCC Patches <gcc-patches at gcc dot gnu dot org>
- Date: Mon, 20 Oct 2014 16:58:11 +0200
- Subject: Re: [PATCH i386 AVX512] [56/n] Add plus/minus/abs/neg/andnot insn patterns.
- Authentication-results: sourceware.org; auth=none
- References: <20140925141206 dot GB27825 at msticlxl57 dot ims dot intel dot com> <CAFULd4aRs34jXbkcJuHU=9zofSwtvEWisH87Rdaexpf1ivBQuA at mail dot gmail dot com> <20141014071820 dot GA59591 at msticlxl57 dot ims dot intel dot com> <20141020123610 dot GN10376 at tucnak dot redhat dot com> <20141020133026 dot GA12661 at msticlxl57 dot ims dot intel dot com> <20141020134109 dot GO10376 at tucnak dot redhat dot com>
On Mon, Oct 20, 2014 at 3:41 PM, Jakub Jelinek <jakub@redhat.com> wrote:
> On Mon, Oct 20, 2014 at 05:30:36PM +0400, Kirill Yukhin wrote:
>> > Unfortunately this caused PR63600. The problem is that VI_AVX2
>> > mode iterator includes V2DI and for AVX2 also V4DI, but for pre-ssse3
>> > ix86_expand_sse2_abs doesn't handle V2DI (and can't easily, we don't have
>> > PSRAQ instruction), for ssse3 there is no vpabsq instruction, and for
>> > avx2 neither.
>> > We can handle V2DI/V4DI only for TARGET_AVX512VL, and V8DI for
>> > TARGET_AVX512F.
>> > Thus, IMHO the mode iterator on at least
>> > (define_insn "*abs<mode>2"
>> > and on
>> > (define_expand "abs<mode>2"
>> > is wrong, should not include V2DI/V4DI unless TARGET_AVX512VL
>> > (so new (or ressurrected, was that VI124_AVX2_48_AVX512F?)
>> > specialized mode iterator?).
>>
>>
>> This patch removes absq insn patterns for non-AVX-512 targets.
>>
>>
>> gcc/
>> * config/i386/sse.md (define_mode_iterator VI_AVX2): Restore to 128-,
>> 256- bit integer modes only.
>> (define_mode_iterator VI_AVX2_AVX512): New.
>> (define_expand "neg<mode>2"): Use VI_AVX2_AVX512 mode iterator.
>> (define_expand "<plusminus_insn><mode>3"): Ditto.
>> (define_insn "*<plusminus_insn><mode>3"): Ditto.
>> (define_expand "<sse2_avx2>_andnot<mode>3"): Ditto.
>> (define_mode_iterator VI1248_AVX512VL_AVX512BW): New.
>> (define_insn "abs<VI1248_AVX512VL_AVX512BW:mode>2"): Ditto.
>>
>> Bootstrap in progress. AVX-512 tests pass.
>>
>> Is it ok for trunk?
>
> I'll certainly leave the review to Uros, whatever he prefers.
> That said, I was expecting you'd keep VI_AVX2 as is (because from the patch
> clearly that is what is used most commonly, the V?DI modes are for most
> insns normal integral vector modes, VI* uses the same modes and VI_AVX2
> used to be just like VI, just with TARGET_AVX conditions replaced with
> TARGET_AVX2), and just add a new mode iterator for the two abs patterns
> (*abs<mode>2 and abs<mode>2), it can be specialized mode iterator just
> for the abs with ABS in names or something.
Yes, I like this idea, too.
Just add IV1248_AVX512VL_AVX512BW and use it in abs patterns.
The changed patch is pre-approved, but please still make full
bootstrap and regtest cycle.
Thanks,
Uros.