From patchwork Thu Sep 10 07:30:08 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Sujoy Saraswati X-Patchwork-Id: 516149 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from sourceware.org (server1.sourceware.org [209.132.180.131]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 72284140157 for ; Thu, 10 Sep 2015 17:30:23 +1000 (AEST) Authentication-Results: ozlabs.org; dkim=pass (1024-bit key; unprotected) header.d=gcc.gnu.org header.i=@gcc.gnu.org header.b=m/uaf7pV; dkim-atps=neutral DomainKey-Signature: a=rsa-sha1; c=nofws; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender :mime-version:in-reply-to:references:date:message-id:subject :from:to:cc:content-type; q=dns; s=default; b=J2z99ww3lMkG6B9UXY Iq9rafUPVNWLap6Izv6nU3Cnj3hIdShpG1zlgBDuFyHVA+VgtxXmEB5EfhoNKjoX 8jY2Jy6PcBqTZ0eZAqTdUMYbk6EH1yddzGXka/4+CQgcal/+iOhkHDiSl4BIMg8o JgRQ2csytW3MiNppSYZ961HpE= DKIM-Signature: v=1; a=rsa-sha1; c=relaxed; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender :mime-version:in-reply-to:references:date:message-id:subject :from:to:cc:content-type; s=default; bh=ihg8QqRBpwKg4RLUuQ/zC1gy MjE=; b=m/uaf7pV7kvSD+cbZVbJnfmDSus22OwpmHuStZbkSgCFrlWZAw61xvI2 evN06hReYSB26eAf5ko5gBlVGLDZiJ9RGVwGgy4z3cHJdxeTnUxHo1i3dWykWxVo H2rv8nBVp81wO7dOalClodoRgDaVzquF+FzQagv2vsFRVTzGkNA= Received: (qmail 22497 invoked by alias); 10 Sep 2015 07:30:15 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Delivered-To: mailing list gcc-patches@gcc.gnu.org Received: (qmail 22480 invoked by uid 89); 10 Sep 2015 07:30:14 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-0.4 required=5.0 tests=AWL, BAYES_00, FREEMAIL_FROM, KAM_ASCII_DIVIDERS, RCVD_IN_DNSWL_LOW, SPF_PASS autolearn=no version=3.3.2 X-HELO: mail-lb0-f173.google.com Received: from mail-lb0-f173.google.com (HELO mail-lb0-f173.google.com) (209.85.217.173) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES128-GCM-SHA256 encrypted) ESMTPS; Thu, 10 Sep 2015 07:30:12 +0000 Received: by lbcjc2 with SMTP id jc2so18263749lbc.0 for ; Thu, 10 Sep 2015 00:30:09 -0700 (PDT) MIME-Version: 1.0 X-Received: by 10.152.23.4 with SMTP id i4mr24468838laf.60.1441870208924; Thu, 10 Sep 2015 00:30:08 -0700 (PDT) Received: by 10.25.24.71 with HTTP; Thu, 10 Sep 2015 00:30:08 -0700 (PDT) In-Reply-To: References: Date: Thu, 10 Sep 2015 13:00:08 +0530 Message-ID: Subject: Re: Fix 61441 From: Sujoy Saraswati To: Richard Biener Cc: GCC Patches X-IsSubscribed: yes Hi, Here is a modified patch for this. The change this time is in fold-const.c and real.c. Bootstrap and regression tests on x86_64-linux-gnu and aarch64-unknown-linux-gnu passed with changes done on trunk. Is this fine ? Regards, Sujoy 2015-09-10 Sujoy Saraswati PR tree-optimization/61441 * fold-const.c (const_binop, fold_abs_const): Convert sNaN to qNaN when flag_signaling_nans is off. * real.c (do_add, do_multiply, do_divide, do_fix_trunc): Same. (real_arithmetic, real_ldexp, real_convert): Same PR tree-optimization/61441 * gcc.dg/pr61441.c: New testcase. On Wed, Sep 2, 2015 at 5:26 PM, Richard Biener wrote: > On Wed, Sep 2, 2015 at 1:36 PM, Sujoy Saraswati wrote: >> Hi Richard, >> >>> Note that I'm curious what >>> the actual bug is - is it that (double) sNaN creates a sNaN? Then the fix >>> should be elsewhere, in constant folding itself >>> (fold_convert_const_real_from_real >>> or real_convert). >>> >>> If that isn't the bug you have very many other passes to fix for the >>> same problem. >>> >>> So - can you please explain? >> >> In this test case, the floating point operation for converting the >> float to double is what should convert the sNaN to qNaN. I tried to >> cover more floating point operations than just the conversion >> operations exposed by this test case. For example, if we consider the >> following program - >> >> #define _GNU_SOURCE >> #include >> #include >> >> int main (void) >> { >> float x; >> float sNaN = __builtin_nansf (""); >> x = sNaN + .0; >> return issignaling(x); >> } >> >> The operation (sNaN + .0) should also result in qNaN after folding. >> Hence, I thought of doing the sNaN to qNaN conversion in various >> places under tree-ssa-ccp, where the result upon a folding is >> available. I do agree that this approach may mean many more such >> places should also be covered in other passes, but thought of sending >> the fix for the ccp pass to start with. >> >> Let me know if you suggest alternate approach. > > CCP and other passes ultimatively end up using fold-const.c:const_{unop,binop} > for constant folding so that is where the fix should go to (or to real.c). That > will automatically handle other passes doing similar transforms. > > Richard. > >> Regards, >> Sujoy >> >>> Thanks, >>> Richard. >>> >>>> Regards, >>>> Sujoy >>>> >>>> 2015-09-01 Sujoy Saraswati >>>> >>>> PR tree-optimization/61441 >>>> * tree-ssa-ccp.c (convert_snan_to_qnan): Convert sNaN to qNaN when >>>> flag_signaling_nans is off. >>>> (ccp_fold_stmt, visit_assignment, visit_cond_stmt): call >>>> convert_snan_to_qnan to convert sNaN to qNaN on constant folding. >>>> >>>> PR tree-optimization/61441 >>>> * gcc.dg/pr61441.c: New testcase. >>>> >>>> Index: gcc/tree-ssa-ccp.c >>>> =================================================================== >>>> --- gcc/tree-ssa-ccp.c (revision 226965) >>>> +++ gcc/tree-ssa-ccp.c (working copy) >>>> @@ -560,6 +560,24 @@ value_to_wide_int (ccp_prop_value_t val) >>>> return 0; >>>> } >>>> >>>> +/* Convert sNaN to qNaN when flag_signaling_nans is off */ >>>> + >>>> +static void >>>> +convert_snan_to_qnan (tree expr) >>>> +{ >>>> + if (expr >>>> + && (TREE_CODE (expr) == REAL_CST) >>>> + && !flag_signaling_nans) >>>> + { >>>> + REAL_VALUE_TYPE *d = TREE_REAL_CST_PTR (expr); >>>> + >>>> + if (HONOR_NANS (TYPE_MODE (TREE_TYPE (expr))) >>>> + && REAL_VALUE_ISNAN (*d) >>>> + && d->signalling) >>>> + d->signalling = 0; >>>> + } >>>> +} >>>> + >>>> /* Return the value for the address expression EXPR based on alignment >>>> information. */ >>>> >>>> @@ -2156,6 +2174,7 @@ ccp_fold_stmt (gimple_stmt_iterator *gsi) >>>> if (val.lattice_val != CONSTANT >>>> || val.mask != 0) >>>> return false; >>>> + convert_snan_to_qnan (val.value); >>>> >>>> if (dump_file) >>>> { >>>> @@ -2197,7 +2216,10 @@ ccp_fold_stmt (gimple_stmt_iterator *gsi) >>>> bool res; >>>> if (!useless_type_conversion_p (TREE_TYPE (lhs), >>>> TREE_TYPE (new_rhs))) >>>> + { >>>> new_rhs = fold_convert (TREE_TYPE (lhs), new_rhs); >>>> + convert_snan_to_qnan (new_rhs); >>>> + } >>>> res = update_call_from_tree (gsi, new_rhs); >>>> gcc_assert (res); >>>> return true; >>>> @@ -2216,6 +2238,7 @@ ccp_fold_stmt (gimple_stmt_iterator *gsi) >>>> tree new_rhs = fold_builtin_alloca_with_align (stmt); >>>> if (new_rhs) >>>> { >>>> + convert_snan_to_qnan (new_rhs); >>>> bool res = update_call_from_tree (gsi, new_rhs); >>>> tree var = TREE_OPERAND (TREE_OPERAND (new_rhs, 0),0); >>>> gcc_assert (res); >>>> @@ -2260,7 +2283,10 @@ ccp_fold_stmt (gimple_stmt_iterator *gsi) >>>> { >>>> tree rhs = unshare_expr (val); >>>> if (!useless_type_conversion_p (TREE_TYPE (lhs), TREE_TYPE (rhs))) >>>> + { >>>> rhs = fold_build1 (VIEW_CONVERT_EXPR, TREE_TYPE (lhs), rhs); >>>> + convert_snan_to_qnan (rhs); >>>> + } >>>> gimple_assign_set_rhs_from_tree (gsi, rhs); >>>> return true; >>>> } >>>> @@ -2292,6 +2318,7 @@ visit_assignment (gimple stmt, tree *output_p) >>>> /* Evaluate the statement, which could be >>>> either a GIMPLE_ASSIGN or a GIMPLE_CALL. */ >>>> val = evaluate_stmt (stmt); >>>> + convert_snan_to_qnan (val.value); >>>> >>>> /* If STMT is an assignment to an SSA_NAME, we only have one >>>> value to set. */ >>>> @@ -2324,6 +2351,7 @@ visit_cond_stmt (gimple stmt, edge *taken_edge_p) >>>> if (val.lattice_val != CONSTANT >>>> || val.mask != 0) >>>> return SSA_PROP_VARYING; >>>> + convert_snan_to_qnan (val.value); >>>> >>>> /* Find which edge out of the conditional block will be taken and add it >>>> to the worklist. If no single edge can be determined statically, >>>> >>>> Index: gcc/testsuite/gcc.dg/pr61441.c >>>> =================================================================== >>>> --- gcc/testsuite/gcc.dg/pr61441.c (revision 0) >>>> +++ gcc/testsuite/gcc.dg/pr61441.c (working copy) >>>> @@ -0,0 +1,17 @@ >>>> +/* { dg-do run } */ >>>> +/* { dg-options "-O1 -lm" } */ >>>> + >>>> +#define _GNU_SOURCE >>>> +#include >>>> +#include >>>> + >>>> +int main (void) >>>> +{ >>>> + float sNaN = __builtin_nansf (""); >>>> + double x = (double) sNaN; >>>> + if (issignaling(x)) >>>> + { >>>> + __builtin_abort(); >>>> + } >>>> + return 0; >>>> +} Index: gcc/fold-const.c =================================================================== --- gcc/fold-const.c (revision 227584) +++ gcc/fold-const.c (working copy) @@ -1183,9 +1183,19 @@ const_binop (enum tree_code code, tree arg1, tree /* If either operand is a NaN, just return it. Otherwise, set up for floating-point trap; we return an overflow. */ if (REAL_VALUE_ISNAN (d1)) + { + /* Convert sNaN to qNaN when flag_signaling_nans is off */ + if (!flag_signaling_nans) + (TREE_REAL_CST_PTR (arg1))->signalling = 0; return arg1; + } else if (REAL_VALUE_ISNAN (d2)) + { + /* Convert sNaN to qNaN when flag_signaling_nans is off */ + if (!flag_signaling_nans) + (TREE_REAL_CST_PTR (arg1))->signalling = 0; return arg2; + } inexact = real_arithmetic (&value, code, &d1, &d2); real_convert (&result, mode, &value); @@ -13644,6 +13654,9 @@ fold_abs_const (tree arg0, tree type) t = build_real (type, real_value_negate (&TREE_REAL_CST (arg0))); else t = arg0; + /* Convert sNaN to qNaN when flag_signaling_nans is off */ + if (!flag_signaling_nans) + (TREE_REAL_CST_PTR (t))->signalling = 0; break; default: Index: gcc/real.c =================================================================== --- gcc/real.c (revision 227584) +++ gcc/real.c (working copy) @@ -545,6 +545,9 @@ do_add (REAL_VALUE_TYPE *r, const REAL_VALUE_TYPE case CLASS2 (rvc_normal, rvc_inf): /* R + Inf = Inf. */ *r = *b; + /* Convert sNaN to qNaN when flag_signaling_nans is off */ + if (!flag_signaling_nans) + r->signalling = 0; r->sign = sign ^ subtract_p; return false; @@ -558,6 +561,9 @@ do_add (REAL_VALUE_TYPE *r, const REAL_VALUE_TYPE case CLASS2 (rvc_inf, rvc_normal): /* Inf + R = Inf. */ *r = *a; + /* Convert sNaN to qNaN when flag_signaling_nans is off */ + if (!flag_signaling_nans) + r->signalling = 0; return false; case CLASS2 (rvc_inf, rvc_inf): @@ -680,6 +686,9 @@ do_multiply (REAL_VALUE_TYPE *r, const REAL_VALUE_ case CLASS2 (rvc_nan, rvc_nan): /* ANY * NaN = NaN. */ *r = *b; + /* Convert sNaN to qNaN when flag_signaling_nans is off */ + if (!flag_signaling_nans) + r->signalling = 0; r->sign = sign; return false; @@ -688,6 +697,9 @@ do_multiply (REAL_VALUE_TYPE *r, const REAL_VALUE_ case CLASS2 (rvc_nan, rvc_inf): /* NaN * ANY = NaN. */ *r = *a; + /* Convert sNaN to qNaN when flag_signaling_nans is off */ + if (!flag_signaling_nans) + r->signalling = 0; r->sign = sign; return false; @@ -830,6 +842,9 @@ do_divide (REAL_VALUE_TYPE *r, const REAL_VALUE_TY case CLASS2 (rvc_nan, rvc_nan): /* ANY / NaN = NaN. */ *r = *b; + /* Convert sNaN to qNaN when flag_signaling_nans is off */ + if (!flag_signaling_nans) + r->signalling = 0; r->sign = sign; return false; @@ -838,6 +853,9 @@ do_divide (REAL_VALUE_TYPE *r, const REAL_VALUE_TY case CLASS2 (rvc_nan, rvc_inf): /* NaN / ANY = NaN. */ *r = *a; + /* Convert sNaN to qNaN when flag_signaling_nans is off */ + if (!flag_signaling_nans) + r->signalling = 0; r->sign = sign; return false; @@ -968,6 +986,9 @@ do_fix_trunc (REAL_VALUE_TYPE *r, const REAL_VALUE case rvc_zero: case rvc_inf: case rvc_nan: + /* Convert sNaN to qNaN when flag_signaling_nans is off */ + if (!flag_signaling_nans) + r->signalling = 0; break; case rvc_normal: @@ -1059,6 +1080,11 @@ real_arithmetic (REAL_VALUE_TYPE *r, int icode, co default: gcc_unreachable (); } + + /* Convert sNaN to qNaN when flag_signaling_nans is off */ + if (!flag_signaling_nans) + r->signalling = 0; + return false; } @@ -1150,6 +1176,9 @@ real_ldexp (REAL_VALUE_TYPE *r, const REAL_VALUE_T case rvc_zero: case rvc_inf: case rvc_nan: + /* Convert sNaN to qNaN when flag_signaling_nans is off */ + if (!flag_signaling_nans) + r->signalling = 0; break; case rvc_normal: @@ -2718,6 +2747,10 @@ real_convert (REAL_VALUE_TYPE *r, machine_mode mod round_for_format (fmt, r); + /* Convert sNaN to qNaN when flag_signaling_nans is off */ + if (HONOR_NANS (mode) && !flag_signaling_nans) + r->signalling = 0; + /* round_for_format de-normalizes denormals. Undo just that part. */ if (r->cl == rvc_normal) normalize (r); Index: gcc/testsuite/gcc.dg/pr61441.c =================================================================== --- gcc/testsuite/gcc.dg/pr61441.c (revision 0) +++ gcc/testsuite/gcc.dg/pr61441.c (working copy) @@ -0,0 +1,34 @@ +/* { dg-do run } */ +/* { dg-options "-O1 -lm" } */ + +#define _GNU_SOURCE +#include +#include + +void conversion() +{ + float sNaN = __builtin_nansf (""); + double x = (double) sNaN; + if (issignaling(x)) + { + __builtin_abort(); + } +} + +void addition() +{ + float x; + float sNaN = __builtin_nansf (""); + x = sNaN + .0; + if (issignaling(x)) + { + __builtin_abort(); + } +} + +int main (void) +{ + conversion(); + addition(); + return 0; +}