Patchwork combine_conversions int->double->int

login
register
mail settings
Submitter Marc Glisse
Date April 26, 2012, 6:43 p.m.
Message ID <alpine.DEB.2.02.1204262031001.2542@laptop-mg.saclay.inria.fr>
Download mbox | patch
Permalink /patch/155316/
State New
Headers show

Comments

Marc Glisse - April 26, 2012, 6:43 p.m.
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).
Richard Guenther - April 27, 2012, 8:23 a.m.
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.
On Fri, 27 Apr 2012, Richard Guenther wrote:

> Do you have a copyright assignment on file?

Yes.

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;
 }