This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH i386 5/8] [AVX-512] Extend vectorizer hooks.
- From: Eric Botcazou <ebotcazou at adacore dot com>
- To: Kirill Yukhin <kirill dot yukhin at gmail dot com>
- Cc: gcc-patches at gcc dot gnu dot org, Uros Bizjak <ubizjak at gmail dot com>, Richard Henderson <rth at redhat dot com>, Jakub Jelinek <jakub at redhat dot com>
- Date: Thu, 2 Jan 2014 16:12:14 +0100
- Subject: Re: [PATCH i386 5/8] [AVX-512] Extend vectorizer hooks.
- Authentication-results: sourceware.org; auth=none
- References: <20131112123633 dot GC34333 at msticlxl57 dot ims dot intel dot com> <201401020007 dot 52562 dot ebotcazou at adacore dot com> <20140102105326 dot GA57767 at msticlxl57 dot ims dot intel dot com>
> Frankly speaking, I do not understand, what's wrong here.
> IMHO, this change is pretty mechanical: we just extend maximal aligment
> available. Because of 512-bit data types we now extend maximal aligment to
> 512 bits.
Nothing wrong per se, but...
> I suspect that an issue is here:
> if (opt
> && AGGREGATE_TYPE_P (type)
> && TYPE_SIZE (type)
> && TREE_CODE (TYPE_SIZE (type)) == INTEGER_CST
> && (TREE_INT_CST_LOW (TYPE_SIZE (type)) >= (unsigned) max_align
>
> || TREE_INT_CST_HIGH (TYPE_SIZE (type)))
>
> && align < max_align)
> align = max_align;
...yes, bumping max_align has the unexpected side effect of changing the
behavior for sizes between the old value and the new value because of this
code. I'm no x86 specialist, but I think that this should be fixed.
> Maybe we can split it and handle 256-bit aggregates separately?
Probably, and we should also add a warning just before the declaration of
max_align, as well as investigate whether this didn't already happen when
max_align was bumped from 128 to 256.
--
Eric Botcazou