diff mbox

combine_conversions int->double->int

Message ID alpine.DEB.2.02.1204262031001.2542@laptop-mg.saclay.inria.fr
State New
Headers show

Commit Message

Marc Glisse April 26, 2012, 6:43 p.m. UTC 
On Thu, 26 Apr 2012, Richard Guenther wrote:

> On Wed, Apr 25, 2012 at 3:58 PM, Marc Glisse <marc.glisse@inria.fr> wrote:
>> Here is take 2 on this patch, which seems cleaner. Bootstrapped and
>> regression tested.
>>
>> gcc/ChangeLog
>>
>> 2012-04-25  Marc Glisse  <marc.glisse@inria.fr>
>>
>>        PR middle-end/27139
>>        * tree-ssa-forwprop.c (combine_conversions): Handle INT->FP->INT.
>>
>> gcc/testsuite/ChangeLog
>>
>> 2012-04-25  Marc Glisse  <marc.glisse@inria.fr>
>>
>>        PR middle-end/27139
>>        * gcc.dg/tree-ssa/forwprop-18.c: New test.
>>
>>
>> In my patch, the lines with gimple_assign_* are vaguely guessed from what is
>> around, I don't pretend to understand them.
>
> ;)
>
> The patch looks good to me - on a 2nd thought folding the case back in
> to the if (CONVERT_EXPR_CODE_P (gimple_assign_rhs_code (def_stmt)))
> block to benefit from the local vars therein makes sense (but as separate
> sub-if () as it is now).

Like the attached? (c,c++ bootstrapped, make -k check, no regression)

I changed the types in the testcase to make it really unlikely that a 
platform needs to skip it (it would require a long double that can't 
represent all signed char, or a float that can represent all unsigned long 
long).

Comments

Richard Biener April 27, 2012, 8:23 a.m. UTC | #1 
On Thu, Apr 26, 2012 at 8:43 PM, Marc Glisse <marc.glisse@inria.fr> wrote:
> On Thu, 26 Apr 2012, Richard Guenther wrote:
>
>> On Wed, Apr 25, 2012 at 3:58 PM, Marc Glisse <marc.glisse@inria.fr> wrote:
>>>
>>> Here is take 2 on this patch, which seems cleaner. Bootstrapped and
>>> regression tested.
>>>
>>> gcc/ChangeLog
>>>
>>> 2012-04-25  Marc Glisse  <marc.glisse@inria.fr>
>>>
>>>        PR middle-end/27139
>>>        * tree-ssa-forwprop.c (combine_conversions): Handle INT->FP->INT.
>>>
>>> gcc/testsuite/ChangeLog
>>>
>>> 2012-04-25  Marc Glisse  <marc.glisse@inria.fr>
>>>
>>>        PR middle-end/27139
>>>        * gcc.dg/tree-ssa/forwprop-18.c: New test.
>>>
>>>
>>> In my patch, the lines with gimple_assign_* are vaguely guessed from what
>>> is
>>> around, I don't pretend to understand them.
>>
>>
>> ;)
>>
>> The patch looks good to me - on a 2nd thought folding the case back in
>> to the if (CONVERT_EXPR_CODE_P (gimple_assign_rhs_code (def_stmt)))
>> block to benefit from the local vars therein makes sense (but as separate
>> sub-if () as it is now).
>
>
> Like the attached? (c,c++ bootstrapped, make -k check, no regression)
>
> I changed the types in the testcase to make it really unlikely that a
> platform needs to skip it (it would require a long double that can't
> represent all signed char, or a float that can represent all unsigned long
> long).

Yes.  Do you have a copyright assignment on file?

Thanks,
Richard.

> --
> Marc Glisse
Marc Glisse April 27, 2012, 8:32 a.m. UTC | #2 
On Fri, 27 Apr 2012, Richard Guenther wrote:

> Do you have a copyright assignment on file?

Yes.
diff mbox

Patch

Index: testsuite/gcc.dg/tree-ssa/forwprop-18.c
===================================================================
--- testsuite/gcc.dg/tree-ssa/forwprop-18.c	(revision 0)
+++ testsuite/gcc.dg/tree-ssa/forwprop-18.c	(revision 0)
@@ -0,0 +1,24 @@ 
+/* { dg-do compile } */
+/* { dg-options "-O -fdump-tree-forwprop1" } */
+
+signed char f1(signed char n)
+{
+  return (long double)n;
+}
+unsigned long long f2(signed char n)
+{
+  return (long double)n;
+}
+
+unsigned long long g1(unsigned long long n)
+{
+  return (float)n;
+}
+signed char g2(unsigned long long n)
+{
+  return (float)n;
+}
+
+/* { dg-final { scan-tree-dump-times "\\\(float\\\)" 2 "forwprop1" } } */
+/* { dg-final { scan-tree-dump-not "\\\(long double\\\)" "forwprop1" } } */
+/* { dg-final { cleanup-tree-dump "forwprop1" } } */

Property changes on: testsuite/gcc.dg/tree-ssa/forwprop-18.c
___________________________________________________________________
Added: svn:keywords
   + Author Date Id Revision URL
Added: svn:eol-style
   + native

Index: tree-ssa-forwprop.c
===================================================================
--- tree-ssa-forwprop.c	(revision 186876)
+++ tree-ssa-forwprop.c	(working copy)
@@ -2403,10 +2403,11 @@  combine_conversions (gimple_stmt_iterato
 {
   gimple stmt = gsi_stmt (*gsi);
   gimple def_stmt;
   tree op0, lhs;
   enum tree_code code = gimple_assign_rhs_code (stmt);
+  enum tree_code code2;
 
   gcc_checking_assert (CONVERT_EXPR_CODE_P (code)
 		       || code == FLOAT_EXPR
 		       || code == FIX_TRUNC_EXPR);
 
@@ -2423,11 +2424,13 @@  combine_conversions (gimple_stmt_iterato
 
   def_stmt = SSA_NAME_DEF_STMT (op0);
   if (!is_gimple_assign (def_stmt))
     return 0;
 
-  if (CONVERT_EXPR_CODE_P (gimple_assign_rhs_code (def_stmt)))
+  code2 = gimple_assign_rhs_code (def_stmt);
+
+  if (CONVERT_EXPR_CODE_P (code2) || code2 == FLOAT_EXPR)
     {
       tree defop0 = gimple_assign_rhs1 (def_stmt);
       tree type = TREE_TYPE (lhs);
       tree inside_type = TREE_TYPE (defop0);
       tree inter_type = TREE_TYPE (op0);
@@ -2551,10 +2554,33 @@  combine_conversions (gimple_stmt_iterato
 	  else
 	    gimple_assign_set_rhs_from_tree (gsi, tem);
 	  update_stmt (gsi_stmt (*gsi));
 	  return 1;
 	}
+
+      /* If we are converting an integer to a floating-point that can
+	 represent it exactly and back to an integer, we can skip the
+	 floating-point conversion.  */
+      if (inside_int && inter_float && final_int &&
+          (unsigned) significand_size (TYPE_MODE (inter_type))
+          >= inside_prec - !inside_unsignedp)
+        {
+	  if (useless_type_conversion_p (type, inside_type))
+	    {
+	      gimple_assign_set_rhs1 (stmt, unshare_expr (defop0));
+	      gimple_assign_set_rhs_code (stmt, TREE_CODE (defop0));
+	      update_stmt (stmt);
+	      return remove_prop_source_from_use (op0) ? 2 : 1;
+	    }
+	  else
+	    {
+	      gimple_assign_set_rhs1 (stmt, defop0);
+	      gimple_assign_set_rhs_code (stmt, CONVERT_EXPR);
+	      update_stmt (stmt);
+	      return remove_prop_source_from_use (op0) ? 2 : 1;
+	    }
+	}
     }
 
   return 0;
 }