From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 91782 invoked by alias); 17 Jul 2018 13:27:41 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Received: (qmail 91760 invoked by uid 89); 17 Jul 2018 13:27:41 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-2.5 required=5.0 tests=AWL,BAYES_00,FREEMAIL_FROM,RCVD_IN_DNSWL_NONE,SPF_PASS autolearn=ham version=3.3.2 spammy= X-HELO: mail-lf0-f68.google.com Received: from mail-lf0-f68.google.com (HELO mail-lf0-f68.google.com) (209.85.215.68) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Tue, 17 Jul 2018 13:27:39 +0000 Received: by mail-lf0-f68.google.com with SMTP id u14-v6so846848lfu.0; Tue, 17 Jul 2018 06:27:39 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=TalzB/Uzv/l3+EJO8QkuV/3zeRFEkp4+T25AHxFdr/s=; b=FVyJ9UnYkAtePQCp3v8rGfnTaxUiLt/N/+DwHB4fSY/EyiFYuBIiHsbv6+KukXeLhZ OtwowOraJ/+XjLWF1qg8iNsrpKIDEPCmdmh4D7zQ7ISdrqIEhe64rYeOt3pIfv4OvHK+ 7AlPzfWqyL+JJwTCq+WDEn+ZsQvtF9S+ZTY9K8jXWul4hKYJx6af5hJIHUhLv3cEz3Op Bbggd6r+EC97Qtpm02XEJJV6J672HP3m3gIMeRAnHvuwJM2tkUGYl3wAlxZD1tr7MXMz sMJje9JyQuDZ2drq2UGeuJ019ZS02+68bcMLlkDcC/JA6ZzN6okLFWHfHkk4gm9IqKRf LEpA== MIME-Version: 1.0 References: <5B4DE283.9060100@foss.arm.com> In-Reply-To: <5B4DE283.9060100@foss.arm.com> From: Richard Biener Date: Tue, 17 Jul 2018 13:27:00 -0000 Message-ID: Subject: Re: [PATCH][Fortran] Use MIN/MAX_EXPR for intrinsics or __builtin_fmin/max when appropriate To: kyrylo.tkachov@foss.arm.com Cc: "fortran@gcc.gnu.org" , GCC Patches Content-Type: text/plain; charset="UTF-8" X-IsSubscribed: yes X-SW-Source: 2018-07/txt/msg00924.txt.bz2 On Tue, Jul 17, 2018 at 2:35 PM Kyrill Tkachov wrote: > > Hi all, > > This is my first Fortran patch, so apologies if I'm missing something. > The current expansion of the min and max intrinsics explicitly expands > the comparisons between each argument to calculate the global min/max. > Some targets, like aarch64, have instructions that can calculate the min/max > of two real (floating-point) numbers with the proper NaN-handling semantics > (if both inputs are NaN, return Nan. If one is NaN, return the other) and those > are the semantics provided by the __builtin_fmin/max family of functions that expand > to these instructions. > > This patch makes the frontend emit __builtin_fmin/max directly to compare each > pair of numbers when the numbers are floating-point, and use MIN_EXPR/MAX_EXPR otherwise > (integral types and -ffast-math) which should hopefully be easier to recognise in the What is Fortrans requirement on min/max intrinsics? Doesn't it only require things that are guaranteed by MIN/MAX_EXPR anyways? The only restriction here is /* Minimum and maximum values. When used with floating point, if both operands are zeros, or if either operand is NaN, then it is unspecified which of the two operands is returned as the result. */ which means MIN/MAX_EXPR are not strictly IEEE compliant with signed zeros or NaNs. Thus the correct test would be !HONOR_SIGNED_ZEROS && !HONOR_NANS if singed zeros are significant. I'm not sure if using fmin/max calls when we cannot use MIN/MAX_EXPR is a good idea, this may both generate bigger code and be slower. Richard. > midend and optimise. The previous approach of generating the open-coded version of that > is used when we don't have an appropriate __builtin_fmin/max available. > For example, for a configuration of x86_64-unknown-linux-gnu that I tested there was no > 128-bit __built_fminl available. > > With this patch I'm seeing more than 7000 FMINNM/FMAXNM instructions being generated at -O3 > on aarch64 for 521.wrf from fprate SPEC2017 where none before were generated > (we were generating explicit comparisons and NaN checks). This gave a 2.4% improvement > in performance on a Cortex-A72. > > Bootstrapped and tested on aarch64-none-linux-gnu and x86_64-unknown-linux-gnu. > > Ok for trunk? > Thanks, > Kyrill > > 2018-07-17 Kyrylo Tkachov > > * f95-lang.c (gfc_init_builtin_functions): Define __builtin_fmin, > __builtin_fminf, __builtin_fminl, __builtin_fmax, __builtin_fmaxf, > __builtin_fmaxl. > * trans-intrinsic.c: Include builtins.h. > (gfc_conv_intrinsic_minmax): Emit __builtin_fmin/max or MIN/MAX_EXPR > functions to calculate the min/max. > > 2018-07-17 Kyrylo Tkachov > > * gfortran.dg/max_fmaxf.f90: New test. > * gfortran.dg/min_fminf.f90: Likewise. > * gfortran.dg/minmax_integer.f90: Likewise. > * gfortran.dg/max_fmaxl_aarch64.f90: Likewise. > * gfortran.dg/min_fminl_aarch64.f90: Likewise.