This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH i386 AVX512] [75/n] Update vec_init.
- 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: Fri, 17 Oct 2014 15:00:08 +0200
- Subject: Re: [PATCH i386 AVX512] [75/n] Update vec_init.
- Authentication-results: sourceware.org; auth=none
- References: <20141009121324 dot GL25028 at msticlxl57 dot ims dot intel dot com> <20141015162354 dot GT10376 at tucnak dot redhat dot com> <20141017122800 dot GA49545 at msticlxl57 dot ims dot intel dot com> <20141017125708 dot GV10376 at tucnak dot redhat dot com>
On Fri, Oct 17, 2014 at 2:57 PM, Jakub Jelinek <jakub@redhat.com> wrote:
> On Fri, Oct 17, 2014 at 04:28:12PM +0400, Kirill Yukhin wrote:
>> > I wonder whether for these modes it can ever be beneficial to build them
>> > through interleaves/concatenations etc., if it wouldn't be better to build
>> > them by storing all values into memory and just reading it back.
>> I've tried this example:
>> #include <immintrin.h>
>>
>> unsigned char a0, a1, a2, a3, a4, a5, a6, a7, a8, a9, a10, a11, a12, a13, a14,
>> a15, a16, a17, a18, a19, a20, a21, a22, a23, a24, a25, a26, a27, a28, a29,
>> a30, a31, a32, a33, a34, a35, a36, a37, a38, a39, a40, a41, a42, a43, a44,
>> a45, a46, a47, a48, a49, a50, a51, a52, a53, a54, a55, a56, a57, a58, a59,
>> a60, a61, a62, a63;
>>
>> __m512i foo ()
>> {
>> return __extension__ (__m512i)(__v64qi){
>> a0, a1, a2, a3, a4, a5, a6, a7, a8, a9, a10, a11, a12, a13, a14,
>> a15, a16, a17, a18, a19, a20, a21, a22, a23, a24, a25, a26, a27, a28, a29,
>> a30, a31, a32, a33, a34, a35, a36, a37, a38, a39, a40, a41, a42, a43, a44,
>> a45, a46, a47, a48, a49, a50, a51, a52, a53, a54, a55, a56, a57, a58, a59,
>> a60, a61, a62, a63 };
>> }
>>
>> w/ and w/o -mavx512bw (and always -mavx512f).
>>
>> When, this code works, we've got 127 lines of assembly to do this init.
>> W/o AVX-512BW we've got > 300 lines of code (mostly on GPRs, using sal, and etc.)
>>
>> Then I've looked into actual assembly w/ -mavx512bw and it turns out that no
>> AVX-512BW insn were generated, only AVX-512F (and below). Fixed iterator.
>
> Ok, if it is shorter than copying all those into memory and reading from
> memory, so be it.
>
>> > > -(define_mode_iterator VI48F_512 [V16SI V16SF V8DI V8DF])
>> > > +(define_mode_iterator VI48F_I12_AVX512BW
>> > > + [V16SI V16SF V8DI V8DF
>> > > + (V32HI "TARGET_AVX512BW") (V64QI "TARGET_AVX512BW")])
>> >
>> > What does the I12 stand for? Wasn't it meant to be VI48F_512_AVX512BW
>> > or I512?
>> Actually, I am not awere of any name convention for iterators.
>> As far as I understand, name [more or less] for vector mode
>> should reflect:
>> - Type family of the unit: float or int
>> - Size of the unit: 1, 2, 4 etc. bytes
>> - If possible, target predicates to enable certain modes in
>> given iterator.
>>
>> The name is:
>> - Vector (V)
>> - I48F - contains both ints and floats of size 4 and 8
>> - I12 - contains ints of size 1 and 2
>> - AVX512BW - affected by the target (according to previous note - to be removed)
>>
>> Maybe it'll be better to name it: VF48_I1248?
>
> I'll leave that to Uros, the patch is ok by me.
Don't want to bikeshed, but VF48_I1248 looks somehow better to me.
Anyway, the patch is OK even without this change.
Thanks,
Uros.