From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 2356 invoked by alias); 2 Sep 2015 11:36:36 -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 2343 invoked by uid 89); 2 Sep 2015 11:36:35 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=0.9 required=5.0 tests=AWL,BAYES_50,FREEMAIL_FROM,KAM_ASCII_DIVIDERS,RCVD_IN_DNSWL_LOW,SPF_PASS autolearn=no version=3.3.2 X-HELO: mail-la0-f53.google.com Received: from mail-la0-f53.google.com (HELO mail-la0-f53.google.com) (209.85.215.53) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES128-GCM-SHA256 encrypted) ESMTPS; Wed, 02 Sep 2015 11:36:34 +0000 Received: by lamp12 with SMTP id p12so4707394lam.0 for ; Wed, 02 Sep 2015 04:36:30 -0700 (PDT) MIME-Version: 1.0 X-Received: by 10.152.181.5 with SMTP id ds5mr15539186lac.60.1441193790554; Wed, 02 Sep 2015 04:36:30 -0700 (PDT) Received: by 10.25.24.71 with HTTP; Wed, 2 Sep 2015 04:36:30 -0700 (PDT) In-Reply-To: References: Date: Wed, 02 Sep 2015 11:36:00 -0000 Message-ID: Subject: Re: Fix 61441 From: Sujoy Saraswati To: Richard Biener Cc: GCC Patches Content-Type: text/plain; charset=UTF-8 X-IsSubscribed: yes X-SW-Source: 2015-09/txt/msg00131.txt.bz2 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. 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; >> +}