From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <gcc-patches-return-358374-listarch-gcc-patches=gcc.gnu.org@gcc.gnu.org>
Received: (qmail 12376 invoked by alias); 18 Dec 2013 16:08:27 -0000
Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm
Precedence: bulk
List-Id: <gcc-patches.gcc.gnu.org>
List-Archive: <http://gcc.gnu.org/ml/gcc-patches/>
List-Post: <mailto:gcc-patches@gcc.gnu.org>
List-Help: <mailto:gcc-patches-help@gcc.gnu.org>
Sender: gcc-patches-owner@gcc.gnu.org
Received: (qmail 12367 invoked by uid 89); 18 Dec 2013 16:08:27 -0000
Authentication-Results: sourceware.org; auth=none
X-Virus-Found: No
X-Spam-SWARE-Status: No, score=-2.4 required=5.0 tests=AWL,BAYES_00,FREEMAIL_FROM,RCVD_IN_DNSWL_LOW,SPF_PASS autolearn=ham version=3.3.2
X-HELO: mail-ob0-f174.google.com
Received: from mail-ob0-f174.google.com (HELO mail-ob0-f174.google.com) (209.85.214.174) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES128-SHA encrypted) ESMTPS; Wed, 18 Dec 2013 16:08:25 +0000
Received: by mail-ob0-f174.google.com with SMTP id wn1so8040283obc.19        for <gcc-patches@gcc.gnu.org>; Wed, 18 Dec 2013 08:08:23 -0800 (PST)
MIME-Version: 1.0
X-Received: by 10.60.50.202 with SMTP id e10mr21478390oeo.39.1387382903397; Wed, 18 Dec 2013 08:08:23 -0800 (PST)
Received: by 10.182.137.136 with HTTP; Wed, 18 Dec 2013 08:08:23 -0800 (PST)
In-Reply-To: <20131218132342.GI57860@msticlxl57.ims.intel.com>
References: <20131119103513.GI25998@msticlxl57.ims.intel.com>	<CAFULd4afgDMO9+0r9n6E3x907skxFvzqWt+HeHa6A5p4Jp3P5Q@mail.gmail.com>	<20131120113044.GA41629@msticlxl57.ims.intel.com>	<20131202131838.GH2453@msticlxl57.ims.intel.com>	<20131218132342.GI57860@msticlxl57.ims.intel.com>
Date: Wed, 18 Dec 2013 16:08:00 -0000
Message-ID: <CAFULd4ZYMbwpqwkdhgwu+8U8XDDTKxNkVePvHHDZVv8G6VTqWA@mail.gmail.com>
Subject: Re: [PATCH i386 7/8] [AVX-512] Add tests.
From: Uros Bizjak <ubizjak@gmail.com>
To: Kirill Yukhin <kirill.yukhin@gmail.com>
Cc: Richard Henderson <rth@redhat.com>, Jakub Jelinek <jakub@redhat.com>, Jeff Law <law@redhat.com>, 	GCC Patches <gcc-patches@gcc.gnu.org>
Content-Type: text/plain; charset=ISO-8859-1
X-SW-Source: 2013-12/txt/msg01612.txt.bz2

On Wed, Dec 18, 2013 at 2:23 PM, Kirill Yukhin <kirill.yukhin@gmail.com> wrote:
> Hello,
> On 02 Dec 16:18, Kirill Yukhin wrote:
>> Hello,
>> > Is it ok now?
>> Ping?
> Ping.
>
> Rebased patch attached.

Whoa.

--- a/gcc/config/i386/sse.md
+++ b/gcc/config/i386/sse.md

No, not in this patch.

+/* { dg-do run } */
+/* { dg-options "-O2 -mavx512cd -DHAVE_512 -DAVX512CD" } */
+/* { dg-require-effective-target avx512cd } */
+
+#include "avx512f-helper.h"

Please put the definitions in the source itself, not in command line.

+  if ((ecx & bit_OSXSAVE) == (bit_OSXSAVE))
+    {
+      if (__get_cpuid_max (0, NULL) < 7)
+    return 0;
+
+      __cpuid_count (7, 0, eax, ebx, ecx, edx);
+
+      if ((avx512f_os_support ()) && ((ebx & (bit_AVX512ER)) ==
(bit_AVX512ER)))

No need for parenthesis around bit_* constants. They alredy have
needed parenthesis in cpuid.h.

+++ b/gcc/testsuite/gcc.target/i386/avx512f-vgetmantps-2.c
@@ -0,0 +1,110 @@
+/* { dg-do run } */
+/* { dg-options "-O2 -mavx512f -DAVX512F" } */
+/* { dg-require-effective-target avx512f } */
+
+#include "avx512f-helper.h"
+
+#define SIZE (AVX512F_LEN / 32)
+#include "avx512f-mask-type.h"
+#include <math.h>
+
+#ifndef GET_NORM_MANT
+#define GET_NORM_MANT
+float
+get_norm_mant (float source, int signctrl, int interv)
+{
+  int dest, src, sign, exp, fraction;
+  src = *(int *) &source;

Strict aliasing violation.

+  dest = (sign << 31) | (exp << 23) | fraction;
+  return *(float *) &dest;

also here. I wonder, why you didn't get:

warning: dereferencing type-punned pointer will break strict-aliasing
rules [-Wstrict-aliasing]
   src = *(int *) &source;
warning: dereferencing type-punned pointer will break strict-aliasing
rules [-Wstrict-aliasing]
   return *(float *) &dest;

Can you please review testcases for aliasing violations (you can check
the new tests with -Wstrict-aliasing warning enabled)

The patch is otherwise OK for mainline, on the (obvious) condition
that other patches get approved and committed first.

Thanks,
Uros.