Re: [PATCH i386 6/8] [AVX-512] Add builtins/intrinsics.

[Uros Bizjak <ubizjak at gmail dot com>]

On Wed, Dec 18, 2013 at 2:21 PM, Kirill Yukhin <kirill.yukhin@gmail.com> wrote:
> Hello,
>
> On 02 Dec 16:15, Kirill Yukhin wrote:
>> Hello
>> > Ok for trunk?
>> Ping?
> Ping.
>
> Rebased patch attached.

+      error ("third argument must be comparison constant.");

"the third ...", without dot at the end. Please review many other
instances of either missing "the" and/or unneded dot.
+          error ("the immediate argument must be 4-bit immediate.");

"... an 4-bit immediate"

+      error ("last argument must be scale 1, 2, 4, 8");

"the last ..."

+      error ("forth argument must be scale 1, 2, 4, 8");

"the fourth ..."

(disclaimer: English is not my native language).

+/* Walk through insns sequence or pattern and erase rounding mentions.
+   Main transformation is performed in ix86_erase_embedded_rounding_1.  */
+static rtx
+ix86_erase_embedded_rounding (rtx pat)

All calls to this function are made with insn pattern, so we can
remove this function and use ix86_erase_embedded_rounding_1 directly
instead. The function to handle sequences can be re-introduced when
needed, probably in a later patch.

@@ -34092,6 +35818,16 @@ ix86_builtin_vectorized_function (tree
fndecl, tree type_out,

Changes to ix86_builtin_vectorized_function belong to "[PATCH i386
5/8] [AVX-512] Extend vectorizer hooks.". This one is already huge...

So, based on following findings:
- the patch uses standard builtin expansion infrastructure and
expansion approaches
- headers and builtins survive gcc.target/i386/sse-{12,13,14,22,23}.c
and g++.dg/other/i386-{2,3}.C regression smoketests
- the functionality is covered by extensive testsuite

the patch is OK (with above mentioned changes) for mainline.

Uros.