[PATCH] Fix PR49715, (float)unsigned -> (float)signed

[PATCH] Fix PR49715, (float)unsigned -> (float)signed

[Richard Guenther]

These two patches fix PR49715, the inability to optimize unsigned -> float
compares when the unsigned operand also fits into an SImode signed
integer.

The first patch makes VRP handle SSA name creation during 
substitute_and_fold nicely and the second implements the above transform.

Does anyone know if this transformation is not beneficial on
any architecture?

Bootstrapped on x86_64-unknown-linux-gnu, testing in progress.

Thanks,
Richard.

2011-07-21  Richard Guenther  <rguenther@suse.de>

	* tree-vrp.c (num_vr_values, values_propagated): New global vars.
	(get_value_range): For out-of-range SSA names or names created
	after propagation return a read-only varying range.
	(dump_all_value_ranges): Adjust.
	(vrp_initialize): Likewise.
	(vrp_finalize): Likewise.

Index: trunk/gcc/tree-vrp.c
===================================================================
*** trunk.orig/gcc/tree-vrp.c	2011-07-21 16:23:30.000000000 +0200
--- trunk/gcc/tree-vrp.c	2011-07-21 16:46:39.000000000 +0200
*************** static assert_locus_t *asserts_for;
*** 138,144 ****
--- 138,146 ----
  
  /* Value range array.  After propagation, VR_VALUE[I] holds the range
     of values that SSA name N_I may take.  */
+ static unsigned num_vr_values;
  static value_range_t **vr_value;
+ static bool values_propagated;
  
  /* For a PHI node which sets SSA name N_I, VR_COUNTS[I] holds the
     number of executable edges we saw the last time we visited the
*************** abs_extent_range (value_range_t *vr, tre
*** 658,663 ****
--- 660,667 ----
  static value_range_t *
  get_value_range (const_tree var)
  {
+   static const struct value_range_d vr_const_varying
+     = { VR_VARYING, NULL_TREE, NULL_TREE, NULL };
    value_range_t *vr;
    tree sym;
    unsigned ver = SSA_NAME_VERSION (var);
*************** get_value_range (const_tree var)
*** 666,675 ****
--- 670,689 ----
    if (! vr_value)
      return NULL;
  
+   /* If we query the range for a new SSA name return an unmodifiable VARYING.
+      We should get here at most from the substitute-and-fold stage which
+      will never try to change values.  */
+   if (ver >= num_vr_values)
+     return CONST_CAST (value_range_t *, &vr_const_varying);
+ 
    vr = vr_value[ver];
    if (vr)
      return vr;
  
+   /* After propagation finished do not allocate new value-ranges.  */
+   if (values_propagated)
+     return CONST_CAST (value_range_t *, &vr_const_varying);
+ 
    /* Create a default value range.  */
    vr_value[ver] = vr = XCNEW (value_range_t);
  
*************** dump_all_value_ranges (FILE *file)
*** 3931,3937 ****
  {
    size_t i;
  
!   for (i = 0; i < num_ssa_names; i++)
      {
        if (vr_value[i])
  	{
--- 3945,3951 ----
  {
    size_t i;
  
!   for (i = 0; i < num_vr_values; i++)
      {
        if (vr_value[i])
  	{
*************** vrp_initialize (void)
*** 5593,5599 ****
  {
    basic_block bb;
  
!   vr_value = XCNEWVEC (value_range_t *, num_ssa_names);
    vr_phi_edge_counts = XCNEWVEC (int, num_ssa_names);
  
    FOR_EACH_BB (bb)
--- 5607,5615 ----
  {
    basic_block bb;
  
!   values_propagated = false;
!   num_vr_values = num_ssa_names;
!   vr_value = XCNEWVEC (value_range_t *, num_vr_values);
    vr_phi_edge_counts = XCNEWVEC (int, num_ssa_names);
  
    FOR_EACH_BB (bb)
*************** vrp_visit_assignment_or_call (gimple stm
*** 5720,5726 ****
  static inline value_range_t
  get_vr_for_comparison (int i)
  {
!   value_range_t vr = *(vr_value[i]);
  
    /* If name N_i does not have a valid range, use N_i as its own
       range.  This allows us to compare against names that may
--- 5736,5742 ----
  static inline value_range_t
  get_vr_for_comparison (int i)
  {
!   value_range_t vr = *get_value_range (ssa_name (i));
  
    /* If name N_i does not have a valid range, use N_i as its own
       range.  This allows us to compare against names that may
*************** static void
*** 7733,7739 ****
  vrp_finalize (void)
  {
    size_t i;
!   unsigned num = num_ssa_names;
  
    if (dump_file)
      {
--- 7749,7756 ----
  vrp_finalize (void)
  {
    size_t i;
! 
!   values_propagated = true;
  
    if (dump_file)
      {
*************** vrp_finalize (void)
*** 7753,7759 ****
    identify_jump_threads ();
  
    /* Free allocated memory.  */
!   for (i = 0; i < num; i++)
      if (vr_value[i])
        {
  	BITMAP_FREE (vr_value[i]->equiv);
--- 7770,7776 ----
    identify_jump_threads ();
  
    /* Free allocated memory.  */
!   for (i = 0; i < num_vr_values; i++)
      if (vr_value[i])
        {
  	BITMAP_FREE (vr_value[i]->equiv);


2011-07-21  Richard Guenther  <rguenther@suse.de>

	PR tree-optimization/49715
	* tree-vrp.c (simplify_float_conversion_using_ranges): New
	function.
	(simplify_stmt_using_ranges): Call it.

	* gcc.target/i386/pr49715.c: New testcase.

Index: gcc/tree-vrp.c
===================================================================
*** gcc/tree-vrp.c.orig	2011-07-21 16:46:39.000000000 +0200
--- gcc/tree-vrp.c	2011-07-21 16:56:08.000000000 +0200
*************** simplify_conversion_using_ranges (gimple
*** 7448,7453 ****
--- 7448,7496 ----
    return true;
  }
  
+ /* Simplify a conversion from integral SSA name to float in STMT.  */
+ 
+ static bool
+ simplify_float_conversion_using_ranges (gimple_stmt_iterator *gsi, gimple stmt)
+ {
+   tree rhs1 = gimple_assign_rhs1 (stmt);
+   value_range_t *vr = get_value_range (rhs1);
+   tree tem;
+   gimple conv;
+ 
+   /* It's not interesting to widen anything smaller than SImode.  */
+   if (TYPE_PRECISION (TREE_TYPE (rhs1)) < GET_MODE_PRECISION (SImode)
+       || (!TYPE_UNSIGNED (TREE_TYPE (rhs1))
+ 	  && TYPE_PRECISION (TREE_TYPE (rhs1)) == GET_MODE_PRECISION (SImode)))
+     return false;
+ 
+   /* We can only handle constant ranges.  */
+   if (vr->type != VR_RANGE
+       || TREE_CODE (vr->min) != INTEGER_CST
+       || TREE_CODE (vr->max) != INTEGER_CST)
+     return false;
+ 
+   /* Try if the value fits in a signed SImode integer, that's the only
+      interesting case.  */
+   if (!double_int_fits_to_tree_p (intSI_type_node,
+ 				  tree_to_double_int (vr->min))
+       || !double_int_fits_to_tree_p (intSI_type_node,
+ 				     tree_to_double_int (vr->max)))
+     return false;
+ 
+   /* It works, insert a truncation or sign-change before the
+      float conversion.  */
+   tem = create_tmp_var (intSI_type_node, NULL);
+   conv = gimple_build_assign_with_ops (NOP_EXPR, tem, rhs1, NULL_TREE);
+   tem = make_ssa_name (tem, conv);
+   gimple_assign_set_lhs (conv, tem);
+   gsi_insert_before (gsi, conv, GSI_SAME_STMT);
+   gimple_assign_set_rhs1 (stmt, tem);
+   update_stmt (stmt);
+ 
+   return true;
+ }
+ 
  /* Simplify STMT using ranges if possible.  */
  
  static bool
*************** simplify_stmt_using_ranges (gimple_stmt_
*** 7507,7512 ****
--- 7550,7561 ----
  	    return simplify_conversion_using_ranges (stmt);
  	  break;
  
+ 	case FLOAT_EXPR:
+ 	  if (TREE_CODE (rhs1) == SSA_NAME
+ 	      && INTEGRAL_TYPE_P (TREE_TYPE (rhs1)))
+ 	    return simplify_float_conversion_using_ranges (gsi, stmt);
+ 	  break;
+ 
  	default:
  	  break;
  	}
Index: gcc/testsuite/gcc.target/i386/pr49715.c
===================================================================
*** /dev/null	1970-01-01 00:00:00.000000000 +0000
--- gcc/testsuite/gcc.target/i386/pr49715.c	2011-07-21 16:55:57.000000000 +0200
***************
*** 0 ****
--- 1,9 ----
+ /* { dg-do compile } */
+ /* { dg-options "-O2 -msse -mfpmath=sse" } */
+ 
+ float func(unsigned x)
+ {
+   return (x & 0xfffff) * 0.01f;
+ }
+ 
+ /* { dg-final { scan-assembler-times "cvtsi2ss" 1 } } */

Re: [PATCH] Fix PR49715, (float)unsigned -> (float)signed

[Richard Henderson]

On 07/21/2011 08:09 AM, Richard Guenther wrote:
> +   /* It's not interesting to widen anything smaller than SImode.  */
> +   if (TYPE_PRECISION (TREE_TYPE (rhs1)) < GET_MODE_PRECISION (SImode)
> +       || (!TYPE_UNSIGNED (TREE_TYPE (rhs1))
> + 	  && TYPE_PRECISION (TREE_TYPE (rhs1)) == GET_MODE_PRECISION (SImode)))
> +     return false;

Hard-coding SImode?  Really?  You might as well hard-code 32.  At least
that's "more correct" when someone does have BITS_PER_UNIT != 8.

> +   /* Try if the value fits in a signed SImode integer, that's the only
> +      interesting case.  */
> +   if (!double_int_fits_to_tree_p (intSI_type_node,
> + 				  tree_to_double_int (vr->min))
> +       || !double_int_fits_to_tree_p (intSI_type_node,
> + 				     tree_to_double_int (vr->max)))
> +     return false;

unsigned long long -> long long is also a very interesting transform.

Indeed, that's the one will help 64-bit targets most.  Unsigned SImode
normally just gets zero-extended to signed DImode.  Whereas unsigned
DImode generally has to do the bias/convert/unbias thing.


r~

Re: [PATCH] Fix PR49715, (float)unsigned -> (float)signed

[Richard Guenther]

On Thu, 21 Jul 2011, Richard Henderson wrote:

> On 07/21/2011 08:09 AM, Richard Guenther wrote:
> > +   /* It's not interesting to widen anything smaller than SImode.  */
> > +   if (TYPE_PRECISION (TREE_TYPE (rhs1)) < GET_MODE_PRECISION (SImode)
> > +       || (!TYPE_UNSIGNED (TREE_TYPE (rhs1))
> > + 	  && TYPE_PRECISION (TREE_TYPE (rhs1)) == GET_MODE_PRECISION (SImode)))
> > +     return false;
> 
> Hard-coding SImode?  Really?  You might as well hard-code 32.  At least
> that's "more correct" when someone does have BITS_PER_UNIT != 8.

I grepped for named expanders and saw mostly floatsi (thus, SImode).

> 
> > +   /* Try if the value fits in a signed SImode integer, that's the only
> > +      interesting case.  */
> > +   if (!double_int_fits_to_tree_p (intSI_type_node,
> > + 				  tree_to_double_int (vr->min))
> > +       || !double_int_fits_to_tree_p (intSI_type_node,
> > + 				     tree_to_double_int (vr->max)))
> > +     return false;
> 
> unsigned long long -> long long is also a very interesting transform.

Yeah, floatdi would also be interesting, but it appears that RTL
optimizers can at least figure out a testcase like

double func(unsigned long long x)
{
  return (x & 0x7fffffffffffffffULL) * 0.01;
}

themselves.

> Indeed, that's the one will help 64-bit targets most.  Unsigned SImode
> normally just gets zero-extended to signed DImode.  Whereas unsigned
> DImode generally has to do the bias/convert/unbias thing.

Ok, I'll think of some more interesting testcases and handling
DImode.

As of hard-coding SImode I want to avoid truncation to something
smaller than int, so should I rather use ints precision and type
(and then iterate over wider integer modes than the mode of int)?

Thanks,
Richard.

Re: [PATCH] Fix PR49715, (float)unsigned -> (float)signed

[Joseph S. Myers]

On Thu, 21 Jul 2011, Richard Henderson wrote:

> On 07/21/2011 08:09 AM, Richard Guenther wrote:
> > +   /* It's not interesting to widen anything smaller than SImode.  */
> > +   if (TYPE_PRECISION (TREE_TYPE (rhs1)) < GET_MODE_PRECISION (SImode)
> > +       || (!TYPE_UNSIGNED (TREE_TYPE (rhs1))
> > + 	  && TYPE_PRECISION (TREE_TYPE (rhs1)) == GET_MODE_PRECISION (SImode)))
> > +     return false;
> 
> Hard-coding SImode?  Really?  You might as well hard-code 32.  At least
> that's "more correct" when someone does have BITS_PER_UNIT != 8.

I'd guess this is intended to match the hardcoding of SImode in 
optabs.c:expand_float - both of which should maybe use the same target 
hook (that defaults to using SImode) rather than hardcoding SImode.  
(Strictly, in the model that the very existence of SImode should be 
target-dependent, the default hook definition would go in a file in 
config/ that's shared by all targets with BITS_PER_UNIT == 8 and the 
classic set of machine modes, rather than in targhooks.c, but I don't 
think we need to worry about that for now.)

-- 
Joseph S. Myers
joseph@codesourcery.com

Re: [PATCH] Fix PR49715, (float)unsigned -> (float)signed

[Richard Guenther]

On Thu, 21 Jul 2011, Joseph S. Myers wrote:

> On Thu, 21 Jul 2011, Richard Henderson wrote:
> 
> > On 07/21/2011 08:09 AM, Richard Guenther wrote:
> > > +   /* It's not interesting to widen anything smaller than SImode.  */
> > > +   if (TYPE_PRECISION (TREE_TYPE (rhs1)) < GET_MODE_PRECISION (SImode)
> > > +       || (!TYPE_UNSIGNED (TREE_TYPE (rhs1))
> > > + 	  && TYPE_PRECISION (TREE_TYPE (rhs1)) == GET_MODE_PRECISION (SImode)))
> > > +     return false;
> > 
> > Hard-coding SImode?  Really?  You might as well hard-code 32.  At least
> > that's "more correct" when someone does have BITS_PER_UNIT != 8.
> 
> I'd guess this is intended to match the hardcoding of SImode in 
> optabs.c:expand_float - both of which should maybe use the same target 
> hook (that defaults to using SImode) rather than hardcoding SImode.  
> (Strictly, in the model that the very existence of SImode should be 
> target-dependent, the default hook definition would go in a file in 
> config/ that's shared by all targets with BITS_PER_UNIT == 8 and the 
> classic set of machine modes, rather than in targhooks.c, but I don't 
> think we need to worry about that for now.)

Hm, of course some targets define unsigned float expanders just do
do the complicated stuff manually (which is why I didn't look
at the conversion optabs).  I assume that if a target can do
an unsigned -> float conversion it can also to a signed -> float
conversion of the same source mode (probably a safe assumption?).

Patch also handling wider modes and not starting with SImode but
the mode of int:

2011-07-21  Richard Guenther  <rguenther@suse.de>

	PR tree-optimization/49715
	* tree-vrp.c (simplify_float_conversion_using_ranges): New
	function.
	(simplify_stmt_using_ranges): Call it.

	* gcc.target/i386/pr49715-1.c: New testcase.
	* gcc.target/i386/pr49715-2.c: Likewise.

Index: gcc/tree-vrp.c
===================================================================
*** gcc/tree-vrp.c.orig	2011-07-21 16:46:39.000000000 +0200
--- gcc/tree-vrp.c	2011-07-21 17:37:09.000000000 +0200
*************** simplify_conversion_using_ranges (gimple
*** 7448,7453 ****
--- 7448,7516 ----
    return true;
  }
  
+ /* Simplify a conversion from integral SSA name to float in STMT.  */
+ 
+ static bool
+ simplify_float_conversion_using_ranges (gimple_stmt_iterator *gsi, gimple stmt)
+ {
+   tree rhs1 = gimple_assign_rhs1 (stmt);
+   value_range_t *vr = get_value_range (rhs1);
+   enum machine_mode mode;
+   tree tem;
+   gimple conv;
+ 
+   /* It's not interesting to widen anything.  */
+   if (TYPE_PRECISION (TREE_TYPE (rhs1)) < TYPE_PRECISION (integer_type_node)
+       || (!TYPE_UNSIGNED (TREE_TYPE (rhs1))
+ 	  && (TYPE_PRECISION (TREE_TYPE (rhs1))
+ 	      == TYPE_PRECISION (integer_type_node))))
+     return false;
+ 
+   /* We can only handle constant ranges.  */
+   if (vr->type != VR_RANGE
+       || TREE_CODE (vr->min) != INTEGER_CST
+       || TREE_CODE (vr->max) != INTEGER_CST)
+     return false;
+ 
+   /* Search for a mode we can use.  Start with that of int.  */
+   mode = TYPE_MODE (integer_type_node);
+   do
+     {
+       double_int tem;
+ 
+       tem = double_int_ext (tree_to_double_int (vr->min),
+ 			    GET_MODE_PRECISION (mode), 0);
+       if (double_int_equal_p (tree_to_double_int (vr->min), tem))
+ 	{
+ 	  tem = double_int_ext (tree_to_double_int (vr->max),
+ 				GET_MODE_PRECISION (mode), 0);
+ 	  if (double_int_equal_p (tree_to_double_int (vr->max), tem))
+ 	    break;
+ 	}
+ 
+       mode = GET_MODE_WIDER_MODE (mode);
+       if (GET_MODE_PRECISION (mode) > TYPE_PRECISION (TREE_TYPE (rhs1)))
+ 	return false;
+     }
+   while (mode != VOIDmode);
+ 
+   if (mode == VOIDmode)
+     return false;
+ 
+   /* It works, insert a truncation or sign-change before the
+      float conversion.  */
+   tem = create_tmp_var (build_nonstandard_integer_type
+ 			  (GET_MODE_PRECISION (mode), 0), NULL);
+   conv = gimple_build_assign_with_ops (NOP_EXPR, tem, rhs1, NULL_TREE);
+   tem = make_ssa_name (tem, conv);
+   gimple_assign_set_lhs (conv, tem);
+   gsi_insert_before (gsi, conv, GSI_SAME_STMT);
+   gimple_assign_set_rhs1 (stmt, tem);
+   update_stmt (stmt);
+ 
+   return true;
+ }
+ 
  /* Simplify STMT using ranges if possible.  */
  
  static bool
*************** simplify_stmt_using_ranges (gimple_stmt_
*** 7507,7512 ****
--- 7570,7581 ----
  	    return simplify_conversion_using_ranges (stmt);
  	  break;
  
+ 	case FLOAT_EXPR:
+ 	  if (TREE_CODE (rhs1) == SSA_NAME
+ 	      && INTEGRAL_TYPE_P (TREE_TYPE (rhs1)))
+ 	    return simplify_float_conversion_using_ranges (gsi, stmt);
+ 	  break;
+ 
  	default:
  	  break;
  	}
Index: gcc/testsuite/gcc.target/i386/pr49715-1.c
===================================================================
*** /dev/null	1970-01-01 00:00:00.000000000 +0000
--- gcc/testsuite/gcc.target/i386/pr49715-1.c	2011-07-21 16:55:57.000000000 +0200
***************
*** 0 ****
--- 1,9 ----
+ /* { dg-do compile } */
+ /* { dg-options "-O2 -msse -mfpmath=sse" } */
+ 
+ float func(unsigned x)
+ {
+   return (x & 0xfffff) * 0.01f;
+ }
+ 
+ /* { dg-final { scan-assembler-times "cvtsi2ss" 1 } } */
Index: gcc/testsuite/gcc.target/i386/pr49715-2.c
===================================================================
*** /dev/null	1970-01-01 00:00:00.000000000 +0000
--- gcc/testsuite/gcc.target/i386/pr49715-2.c	2011-07-21 17:46:05.000000000 +0200
***************
*** 0 ****
--- 1,12 ----
+ /* { dg-do compile } */
+ /* { dg-require-effective-target lp64 } */
+ /* { dg-options "-O2" } */
+ 
+ double func(unsigned long long x)
+ {
+   if (x <= 0x7ffffffffffffffeULL)
+     return (x + 1) * 0.01;
+   return 0.0;
+ }
+ 
+ /* { dg-final { scan-assembler-times "cvtsi2sdq" 1 } } */

Re: [PATCH] Fix PR49715, (float)unsigned -> (float)signed

[Richard Henderson]

On 07/21/2011 08:47 AM, Richard Guenther wrote:
> Hm, of course some targets define unsigned float expanders just do
> do the complicated stuff manually (which is why I didn't look
> at the conversion optabs).

Unfortunately, there's no way to tell optabs.c not to widen
conversions all the way into software-only fp modes.  This
has forced backends to replicate the fallback logic.

> I assume that if a target can do
> an unsigned -> float conversion it can also to a signed -> float
> conversion of the same source mode (probably a safe assumption?).

That's an assumption that's borne out in the md files.


r~

Re: [PATCH] Fix PR49715, (float)unsigned -> (float)signed

[Joseph S. Myers]

On Thu, 21 Jul 2011, Richard Guenther wrote:

> Patch also handling wider modes and not starting with SImode but
> the mode of int:

Use of target int for anything not about C ABIs is certainly wrong.  This 
might be about what operations the target does efficiently, or what 
functions are present in libgcc (both of which would be functions of 
machine modes), but it's not about the choice of C int.

-- 
Joseph S. Myers
joseph@codesourcery.com

Re: [PATCH] Fix PR49715, (float)unsigned -> (float)signed

[Richard Henderson]

On 07/21/2011 01:16 PM, Joseph S. Myers wrote:
> On Thu, 21 Jul 2011, Richard Guenther wrote:
> 
>> Patch also handling wider modes and not starting with SImode but
>> the mode of int:
> 
> Use of target int for anything not about C ABIs is certainly wrong.  This 
> might be about what operations the target does efficiently, or what 
> functions are present in libgcc (both of which would be functions of 
> machine modes), but it's not about the choice of C int.
> 

Given that we've leaked other uses of the contents of optabs into the
gimple optimizers, would it be too gross to simply look at all of the
integer modes and ignore all those for which can_float_p is false?


r~

Re: [PATCH] Fix PR49715, (float)unsigned -> (float)signed

[Richard Guenther]

On Thu, 21 Jul 2011, Joseph S. Myers wrote:

> On Thu, 21 Jul 2011, Richard Guenther wrote:
> 
> > Patch also handling wider modes and not starting with SImode but
> > the mode of int:
> 
> Use of target int for anything not about C ABIs is certainly wrong.  This 
> might be about what operations the target does efficiently, or what 
> functions are present in libgcc (both of which would be functions of 
> machine modes), but it's not about the choice of C int.

Ok.  Given rths last suggestion I'm testing the following which
checks all integer modes (but never will widen - optabs.c will do
that if it turns out to be profitable).

Richard.

2011-07-22  Richard Guenther  <rguenther@suse.de>

	PR tree-optimization/49715
	* tree-vrp.c: Include expr.h and optabs.h.
	(simplify_float_conversion_using_ranges): New function.
	(simplify_stmt_using_ranges): Call it.
	* Makefile.in (tree-vrp.o): Add $(EXPR_H) and $(OPTABS_H) dependencies.
	* optabs.c (can_float_p): Export.
	* optabs.h (can_float_p): Declare.

	* gcc.target/i386/pr49715-1.c: New testcase.
	* gcc.target/i386/pr49715-2.c: Likewise.

Index: gcc/tree-vrp.c
===================================================================
*** gcc/tree-vrp.c.orig	2011-07-21 16:46:39.000000000 +0200
--- gcc/tree-vrp.c	2011-07-21 17:37:09.000000000 +0200
*************** simplify_conversion_using_ranges (gimple
*** 7448,7453 ****
--- 7448,7516 ----
    return true;
  }
  
+ /* Simplify a conversion from integral SSA name to float in STMT.  */
+ 
+ static bool
+ simplify_float_conversion_using_ranges (gimple_stmt_iterator *gsi, gimple stmt)
+ {
+   tree rhs1 = gimple_assign_rhs1 (stmt);
+   value_range_t *vr = get_value_range (rhs1);
+   enum machine_mode mode;
+   tree tem;
+   gimple conv;
+ 
+   /* It's not interesting to widen anything.  */
+   if (TYPE_PRECISION (TREE_TYPE (rhs1)) < TYPE_PRECISION (integer_type_node)
+       || (!TYPE_UNSIGNED (TREE_TYPE (rhs1))
+ 	  && (TYPE_PRECISION (TREE_TYPE (rhs1))
+ 	      == TYPE_PRECISION (integer_type_node))))
+     return false;
+ 
+   /* We can only handle constant ranges.  */
+   if (vr->type != VR_RANGE
+       || TREE_CODE (vr->min) != INTEGER_CST
+       || TREE_CODE (vr->max) != INTEGER_CST)
+     return false;
+ 
+   /* Search for a mode we can use.  Start with that of int.  */
+   mode = TYPE_MODE (integer_type_node);
+   do
+     {
+       double_int tem;
+ 
+       tem = double_int_ext (tree_to_double_int (vr->min),
+ 			    GET_MODE_PRECISION (mode), 0);
+       if (double_int_equal_p (tree_to_double_int (vr->min), tem))
+ 	{
+ 	  tem = double_int_ext (tree_to_double_int (vr->max),
+ 				GET_MODE_PRECISION (mode), 0);
+ 	  if (double_int_equal_p (tree_to_double_int (vr->max), tem))
+ 	    break;
+ 	}
+ 
+       mode = GET_MODE_WIDER_MODE (mode);
+       if (GET_MODE_PRECISION (mode) > TYPE_PRECISION (TREE_TYPE (rhs1)))
+ 	return false;
+     }
+   while (mode != VOIDmode);
+ 
+   if (mode == VOIDmode)
+     return false;
+ 
+   /* It works, insert a truncation or sign-change before the
+      float conversion.  */
+   tem = create_tmp_var (build_nonstandard_integer_type
+ 			  (GET_MODE_PRECISION (mode), 0), NULL);
+   conv = gimple_build_assign_with_ops (NOP_EXPR, tem, rhs1, NULL_TREE);
+   tem = make_ssa_name (tem, conv);
+   gimple_assign_set_lhs (conv, tem);
+   gsi_insert_before (gsi, conv, GSI_SAME_STMT);
+   gimple_assign_set_rhs1 (stmt, tem);
+   update_stmt (stmt);
+ 
+   return true;
+ }
+ 
  /* Simplify STMT using ranges if possible.  */
  
  static bool
*************** simplify_stmt_using_ranges (gimple_stmt_
*** 7507,7512 ****
--- 7570,7581 ----
  	    return simplify_conversion_using_ranges (stmt);
  	  break;
  
+ 	case FLOAT_EXPR:
+ 	  if (TREE_CODE (rhs1) == SSA_NAME
+ 	      && INTEGRAL_TYPE_P (TREE_TYPE (rhs1)))
+ 	    return simplify_float_conversion_using_ranges (gsi, stmt);
+ 	  break;
+ 
  	default:
  	  break;
  	}
Index: gcc/testsuite/gcc.target/i386/pr49715-1.c
===================================================================
*** /dev/null	1970-01-01 00:00:00.000000000 +0000
--- gcc/testsuite/gcc.target/i386/pr49715-1.c	2011-07-21 16:55:57.000000000 +0200
***************
*** 0 ****
--- 1,9 ----
+ /* { dg-do compile } */
+ /* { dg-options "-O2 -msse -mfpmath=sse" } */
+ 
+ float func(unsigned x)
+ {
+   return (x & 0xfffff) * 0.01f;
+ }
+ 
+ /* { dg-final { scan-assembler-times "cvtsi2ss" 1 } } */
Index: gcc/testsuite/gcc.target/i386/pr49715-2.c
===================================================================
*** /dev/null	1970-01-01 00:00:00.000000000 +0000
--- gcc/testsuite/gcc.target/i386/pr49715-2.c	2011-07-21 17:46:05.000000000 +0200
***************
*** 0 ****
--- 1,12 ----
+ /* { dg-do compile } */
+ /* { dg-require-effective-target lp64 } */
+ /* { dg-options "-O2" } */
+ 
+ double func(unsigned long long x)
+ {
+   if (x <= 0x7ffffffffffffffeULL)
+     return (x + 1) * 0.01;
+   return 0.0;
+ }
+ 
+ /* { dg-final { scan-assembler-times "cvtsi2sdq" 1 } } */

Re: [PATCH] Fix PR49715, (float)unsigned -> (float)signed

[Richard Guenther]

On Fri, 22 Jul 2011, Richard Guenther wrote:

> On Thu, 21 Jul 2011, Joseph S. Myers wrote:
> 
> > On Thu, 21 Jul 2011, Richard Guenther wrote:
> > 
> > > Patch also handling wider modes and not starting with SImode but
> > > the mode of int:
> > 
> > Use of target int for anything not about C ABIs is certainly wrong.  This 
> > might be about what operations the target does efficiently, or what 
> > functions are present in libgcc (both of which would be functions of 
> > machine modes), but it's not about the choice of C int.
> 
> Ok.  Given rths last suggestion I'm testing the following which
> checks all integer modes (but never will widen - optabs.c will do
> that if it turns out to be profitable).

Err, I should refresh the patch before sending it ... here it goes.

Richard.

2011-07-22  Richard Guenther  <rguenther@suse.de>

	PR tree-optimization/49715
	* tree-vrp.c: Include expr.h and optabs.h.
	(simplify_float_conversion_using_ranges): New function.
	(simplify_stmt_using_ranges): Call it.
	* Makefile.in (tree-vrp.o): Add $(EXPR_H) and $(OPTABS_H) dependencies.
	* optabs.c (can_float_p): Export.
	* optabs.h (can_float_p): Declare.

	* gcc.target/i386/pr49715-1.c: New testcase.
	* gcc.target/i386/pr49715-2.c: Likewise.

Index: gcc/tree-vrp.c
===================================================================
*** gcc/tree-vrp.c.orig	2011-07-21 17:46:16.000000000 +0200
--- gcc/tree-vrp.c	2011-07-22 11:09:03.000000000 +0200
*************** along with GCC; see the file COPYING3.
*** 40,45 ****
--- 40,47 ----
  #include "tree-ssa-propagate.h"
  #include "tree-chrec.h"
  #include "gimple-fold.h"
+ #include "expr.h"
+ #include "optabs.h"
  
  
  /* Type of value ranges.  See value_range_d for a description of these
*************** simplify_conversion_using_ranges (gimple
*** 7448,7453 ****
--- 7450,7520 ----
    return true;
  }
  
+ /* Simplify a conversion from integral SSA name to float in STMT.  */
+ 
+ static bool
+ simplify_float_conversion_using_ranges (gimple_stmt_iterator *gsi, gimple stmt)
+ {
+   tree rhs1 = gimple_assign_rhs1 (stmt);
+   value_range_t *vr = get_value_range (rhs1);
+   enum machine_mode fltmode = TYPE_MODE (TREE_TYPE (gimple_assign_lhs (stmt)));
+   enum machine_mode mode;
+   tree tem;
+   gimple conv;
+ 
+   /* We can only handle constant ranges.  */
+   if (vr->type != VR_RANGE
+       || TREE_CODE (vr->min) != INTEGER_CST
+       || TREE_CODE (vr->max) != INTEGER_CST)
+     return false;
+ 
+   /* Search for a mode we can use.  */
+   mode = GET_CLASS_NARROWEST_MODE (MODE_INT);
+   do
+     {
+       double_int tem;
+ 
+       /* If we cannot do a signed conversion to float from mode
+ 	 or if the value-range does not fit in the signed type
+ 	 try with a wider mode.  */
+       if (can_float_p (fltmode, mode, 0) != CODE_FOR_nothing)
+ 	{
+ 	  tem = double_int_ext (tree_to_double_int (vr->min),
+ 				GET_MODE_PRECISION (mode), 0);
+ 	  if (double_int_equal_p (tree_to_double_int (vr->min), tem))
+ 	    {
+ 	      tem = double_int_ext (tree_to_double_int (vr->max),
+ 				    GET_MODE_PRECISION (mode), 0);
+ 	      if (double_int_equal_p (tree_to_double_int (vr->max), tem))
+ 		break;
+ 	    }
+ 	}
+ 
+       mode = GET_MODE_WIDER_MODE (mode);
+       /* But do not widen the input.  Instead leave that to the
+          optabs expansion code.  */
+       if (GET_MODE_PRECISION (mode) > TYPE_PRECISION (TREE_TYPE (rhs1)))
+ 	return false;
+     }
+   while (mode != VOIDmode);
+ 
+   if (mode == VOIDmode)
+     return false;
+ 
+   /* It works, insert a truncation or sign-change before the
+      float conversion.  */
+   tem = create_tmp_var (build_nonstandard_integer_type
+ 			  (GET_MODE_PRECISION (mode), 0), NULL);
+   conv = gimple_build_assign_with_ops (NOP_EXPR, tem, rhs1, NULL_TREE);
+   tem = make_ssa_name (tem, conv);
+   gimple_assign_set_lhs (conv, tem);
+   gsi_insert_before (gsi, conv, GSI_SAME_STMT);
+   gimple_assign_set_rhs1 (stmt, tem);
+   update_stmt (stmt);
+ 
+   return true;
+ }
+ 
  /* Simplify STMT using ranges if possible.  */
  
  static bool
*************** simplify_stmt_using_ranges (gimple_stmt_
*** 7507,7512 ****
--- 7574,7585 ----
  	    return simplify_conversion_using_ranges (stmt);
  	  break;
  
+ 	case FLOAT_EXPR:
+ 	  if (TREE_CODE (rhs1) == SSA_NAME
+ 	      && INTEGRAL_TYPE_P (TREE_TYPE (rhs1)))
+ 	    return simplify_float_conversion_using_ranges (gsi, stmt);
+ 	  break;
+ 
  	default:
  	  break;
  	}
Index: gcc/testsuite/gcc.target/i386/pr49715-1.c
===================================================================
*** /dev/null	1970-01-01 00:00:00.000000000 +0000
--- gcc/testsuite/gcc.target/i386/pr49715-1.c	2011-07-21 17:46:39.000000000 +0200
***************
*** 0 ****
--- 1,9 ----
+ /* { dg-do compile } */
+ /* { dg-options "-O2 -msse -mfpmath=sse" } */
+ 
+ float func(unsigned x)
+ {
+   return (x & 0xfffff) * 0.01f;
+ }
+ 
+ /* { dg-final { scan-assembler-times "cvtsi2ss" 1 } } */
Index: gcc/testsuite/gcc.target/i386/pr49715-2.c
===================================================================
*** /dev/null	1970-01-01 00:00:00.000000000 +0000
--- gcc/testsuite/gcc.target/i386/pr49715-2.c	2011-07-21 17:46:39.000000000 +0200
***************
*** 0 ****
--- 1,12 ----
+ /* { dg-do compile } */
+ /* { dg-require-effective-target lp64 } */
+ /* { dg-options "-O2" } */
+ 
+ double func(unsigned long long x)
+ {
+   if (x <= 0x7ffffffffffffffeULL)
+     return (x + 1) * 0.01;
+   return 0.0;
+ }
+ 
+ /* { dg-final { scan-assembler-times "cvtsi2sdq" 1 } } */
Index: gcc/Makefile.in
===================================================================
*** gcc/Makefile.in.orig	2011-07-19 12:59:09.000000000 +0200
--- gcc/Makefile.in	2011-07-22 11:09:16.000000000 +0200
*************** tree-vrp.o : tree-vrp.c $(CONFIG_H) $(SY
*** 2504,2510 ****
     $(TREE_FLOW_H) $(TREE_PASS_H) $(TREE_DUMP_H) $(DIAGNOSTIC_H) $(GGC_H) \
     $(BASIC_BLOCK_H) tree-ssa-propagate.h $(FLAGS_H) $(TREE_DUMP_H) \
     $(CFGLOOP_H) $(SCEV_H) $(TIMEVAR_H) intl.h tree-pretty-print.h \
!    gimple-pretty-print.h gimple-fold.h
  tree-cfg.o : tree-cfg.c $(TREE_FLOW_H) $(CONFIG_H) $(SYSTEM_H) \
     $(TREE_H) $(TM_P_H) $(EXPR_H) $(GGC_H) $(FLAGS_H) output.h \
     $(DIAGNOSTIC_H) $(FUNCTION_H) $(TIMEVAR_H) $(TM_H) coretypes.h \
--- 2504,2510 ----
     $(TREE_FLOW_H) $(TREE_PASS_H) $(TREE_DUMP_H) $(DIAGNOSTIC_H) $(GGC_H) \
     $(BASIC_BLOCK_H) tree-ssa-propagate.h $(FLAGS_H) $(TREE_DUMP_H) \
     $(CFGLOOP_H) $(SCEV_H) $(TIMEVAR_H) intl.h tree-pretty-print.h \
!    gimple-pretty-print.h gimple-fold.h $(OPTABS_H) $(EXPR_H)
  tree-cfg.o : tree-cfg.c $(TREE_FLOW_H) $(CONFIG_H) $(SYSTEM_H) \
     $(TREE_H) $(TM_P_H) $(EXPR_H) $(GGC_H) $(FLAGS_H) output.h \
     $(DIAGNOSTIC_H) $(FUNCTION_H) $(TIMEVAR_H) $(TM_H) coretypes.h \
Index: gcc/optabs.c
===================================================================
*** gcc/optabs.c.orig	2011-07-11 17:02:51.000000000 +0200
--- gcc/optabs.c	2011-07-22 11:04:42.000000000 +0200
*************** can_fix_p (enum machine_mode fixmode, en
*** 4626,4632 ****
    return CODE_FOR_nothing;
  }
  
! static enum insn_code
  can_float_p (enum machine_mode fltmode, enum machine_mode fixmode,
  	     int unsignedp)
  {
--- 4626,4632 ----
    return CODE_FOR_nothing;
  }
  
! enum insn_code
  can_float_p (enum machine_mode fltmode, enum machine_mode fixmode,
  	     int unsignedp)
  {
Index: gcc/optabs.h
===================================================================
*** gcc/optabs.h.orig	2011-06-22 16:01:28.000000000 +0200
--- gcc/optabs.h	2011-07-22 11:06:01.000000000 +0200
*************** extern void expand_fixed_convert (rtx, r
*** 849,854 ****
--- 849,857 ----
  /* Generate code for a FLOAT_EXPR.  */
  extern void expand_float (rtx, rtx, int);
  
+ /* Return the insn_code for a FLOAT_EXPR.  */
+ enum insn_code can_float_p (enum machine_mode, enum machine_mode, int);
+ 
  /* Generate code for a FIX_EXPR.  */
  extern void expand_fix (rtx, rtx, int);
  

Re: [PATCH] Fix PR49715, (float)unsigned -> (float)signed

[Richard Guenther]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 9503 bytes --]

On Fri, 22 Jul 2011, Richard Guenther wrote:

> On Fri, 22 Jul 2011, Richard Guenther wrote:
> 
> > On Thu, 21 Jul 2011, Joseph S. Myers wrote:
> > 
> > > On Thu, 21 Jul 2011, Richard Guenther wrote:
> > > 
> > > > Patch also handling wider modes and not starting with SImode but
> > > > the mode of int:
> > > 
> > > Use of target int for anything not about C ABIs is certainly wrong.  This 
> > > might be about what operations the target does efficiently, or what 
> > > functions are present in libgcc (both of which would be functions of 
> > > machine modes), but it's not about the choice of C int.
> > 
> > Ok.  Given rths last suggestion I'm testing the following which
> > checks all integer modes (but never will widen - optabs.c will do
> > that if it turns out to be profitable).
> 
> Err, I should refresh the patch before sending it ... here it goes.

Regresses vectorization on i?86 because that defines floathi expanders
but the vectorizer does not recognize a short -> float conversion
as that requires different sized vectors (the int -> short truncation
is also a complication for it).  One of the cases that fail to vectorize 
is

#define N 40
float image[N][N] __attribute__ ((__aligned__(__BIGGEST_ALIGNMENT__)));
float out[N];

/* Outer-loop vectorization.  */

__attribute__ ((noinline)) void
foo (){
  int i,j;

  for (i = 0; i < N; i++) {
    for (j = 0; j < N; j++) {
      image[j][i] = j+i;
    }
  }
}

where VRP figures out that j+i can be represented as a short.

Now, I can dumb down the VRP code to only consider doing the
unsigned -> signed demotion and never truncate (unless maybe
the target does not support floatMODE of the current input mode).

Other suggestions?

Thanks,
Richard.

> Richard.
> 
> 2011-07-22  Richard Guenther  <rguenther@suse.de>
> 
> 	PR tree-optimization/49715
> 	* tree-vrp.c: Include expr.h and optabs.h.
> 	(simplify_float_conversion_using_ranges): New function.
> 	(simplify_stmt_using_ranges): Call it.
> 	* Makefile.in (tree-vrp.o): Add $(EXPR_H) and $(OPTABS_H) dependencies.
> 	* optabs.c (can_float_p): Export.
> 	* optabs.h (can_float_p): Declare.
> 
> 	* gcc.target/i386/pr49715-1.c: New testcase.
> 	* gcc.target/i386/pr49715-2.c: Likewise.
> 
> Index: gcc/tree-vrp.c
> ===================================================================
> *** gcc/tree-vrp.c.orig	2011-07-21 17:46:16.000000000 +0200
> --- gcc/tree-vrp.c	2011-07-22 11:09:03.000000000 +0200
> *************** along with GCC; see the file COPYING3.
> *** 40,45 ****
> --- 40,47 ----
>   #include "tree-ssa-propagate.h"
>   #include "tree-chrec.h"
>   #include "gimple-fold.h"
> + #include "expr.h"
> + #include "optabs.h"
>   
>   
>   /* Type of value ranges.  See value_range_d for a description of these
> *************** simplify_conversion_using_ranges (gimple
> *** 7448,7453 ****
> --- 7450,7520 ----
>     return true;
>   }
>   
> + /* Simplify a conversion from integral SSA name to float in STMT.  */
> + 
> + static bool
> + simplify_float_conversion_using_ranges (gimple_stmt_iterator *gsi, gimple stmt)
> + {
> +   tree rhs1 = gimple_assign_rhs1 (stmt);
> +   value_range_t *vr = get_value_range (rhs1);
> +   enum machine_mode fltmode = TYPE_MODE (TREE_TYPE (gimple_assign_lhs (stmt)));
> +   enum machine_mode mode;
> +   tree tem;
> +   gimple conv;
> + 
> +   /* We can only handle constant ranges.  */
> +   if (vr->type != VR_RANGE
> +       || TREE_CODE (vr->min) != INTEGER_CST
> +       || TREE_CODE (vr->max) != INTEGER_CST)
> +     return false;
> + 
> +   /* Search for a mode we can use.  */
> +   mode = GET_CLASS_NARROWEST_MODE (MODE_INT);
> +   do
> +     {
> +       double_int tem;
> + 
> +       /* If we cannot do a signed conversion to float from mode
> + 	 or if the value-range does not fit in the signed type
> + 	 try with a wider mode.  */
> +       if (can_float_p (fltmode, mode, 0) != CODE_FOR_nothing)
> + 	{
> + 	  tem = double_int_ext (tree_to_double_int (vr->min),
> + 				GET_MODE_PRECISION (mode), 0);
> + 	  if (double_int_equal_p (tree_to_double_int (vr->min), tem))
> + 	    {
> + 	      tem = double_int_ext (tree_to_double_int (vr->max),
> + 				    GET_MODE_PRECISION (mode), 0);
> + 	      if (double_int_equal_p (tree_to_double_int (vr->max), tem))
> + 		break;
> + 	    }
> + 	}
> + 
> +       mode = GET_MODE_WIDER_MODE (mode);
> +       /* But do not widen the input.  Instead leave that to the
> +          optabs expansion code.  */
> +       if (GET_MODE_PRECISION (mode) > TYPE_PRECISION (TREE_TYPE (rhs1)))
> + 	return false;
> +     }
> +   while (mode != VOIDmode);
> + 
> +   if (mode == VOIDmode)
> +     return false;
> + 
> +   /* It works, insert a truncation or sign-change before the
> +      float conversion.  */
> +   tem = create_tmp_var (build_nonstandard_integer_type
> + 			  (GET_MODE_PRECISION (mode), 0), NULL);
> +   conv = gimple_build_assign_with_ops (NOP_EXPR, tem, rhs1, NULL_TREE);
> +   tem = make_ssa_name (tem, conv);
> +   gimple_assign_set_lhs (conv, tem);
> +   gsi_insert_before (gsi, conv, GSI_SAME_STMT);
> +   gimple_assign_set_rhs1 (stmt, tem);
> +   update_stmt (stmt);
> + 
> +   return true;
> + }
> + 
>   /* Simplify STMT using ranges if possible.  */
>   
>   static bool
> *************** simplify_stmt_using_ranges (gimple_stmt_
> *** 7507,7512 ****
> --- 7574,7585 ----
>   	    return simplify_conversion_using_ranges (stmt);
>   	  break;
>   
> + 	case FLOAT_EXPR:
> + 	  if (TREE_CODE (rhs1) == SSA_NAME
> + 	      && INTEGRAL_TYPE_P (TREE_TYPE (rhs1)))
> + 	    return simplify_float_conversion_using_ranges (gsi, stmt);
> + 	  break;
> + 
>   	default:
>   	  break;
>   	}
> Index: gcc/testsuite/gcc.target/i386/pr49715-1.c
> ===================================================================
> *** /dev/null	1970-01-01 00:00:00.000000000 +0000
> --- gcc/testsuite/gcc.target/i386/pr49715-1.c	2011-07-21 17:46:39.000000000 +0200
> ***************
> *** 0 ****
> --- 1,9 ----
> + /* { dg-do compile } */
> + /* { dg-options "-O2 -msse -mfpmath=sse" } */
> + 
> + float func(unsigned x)
> + {
> +   return (x & 0xfffff) * 0.01f;
> + }
> + 
> + /* { dg-final { scan-assembler-times "cvtsi2ss" 1 } } */
> Index: gcc/testsuite/gcc.target/i386/pr49715-2.c
> ===================================================================
> *** /dev/null	1970-01-01 00:00:00.000000000 +0000
> --- gcc/testsuite/gcc.target/i386/pr49715-2.c	2011-07-21 17:46:39.000000000 +0200
> ***************
> *** 0 ****
> --- 1,12 ----
> + /* { dg-do compile } */
> + /* { dg-require-effective-target lp64 } */
> + /* { dg-options "-O2" } */
> + 
> + double func(unsigned long long x)
> + {
> +   if (x <= 0x7ffffffffffffffeULL)
> +     return (x + 1) * 0.01;
> +   return 0.0;
> + }
> + 
> + /* { dg-final { scan-assembler-times "cvtsi2sdq" 1 } } */
> Index: gcc/Makefile.in
> ===================================================================
> *** gcc/Makefile.in.orig	2011-07-19 12:59:09.000000000 +0200
> --- gcc/Makefile.in	2011-07-22 11:09:16.000000000 +0200
> *************** tree-vrp.o : tree-vrp.c $(CONFIG_H) $(SY
> *** 2504,2510 ****
>      $(TREE_FLOW_H) $(TREE_PASS_H) $(TREE_DUMP_H) $(DIAGNOSTIC_H) $(GGC_H) \
>      $(BASIC_BLOCK_H) tree-ssa-propagate.h $(FLAGS_H) $(TREE_DUMP_H) \
>      $(CFGLOOP_H) $(SCEV_H) $(TIMEVAR_H) intl.h tree-pretty-print.h \
> !    gimple-pretty-print.h gimple-fold.h
>   tree-cfg.o : tree-cfg.c $(TREE_FLOW_H) $(CONFIG_H) $(SYSTEM_H) \
>      $(TREE_H) $(TM_P_H) $(EXPR_H) $(GGC_H) $(FLAGS_H) output.h \
>      $(DIAGNOSTIC_H) $(FUNCTION_H) $(TIMEVAR_H) $(TM_H) coretypes.h \
> --- 2504,2510 ----
>      $(TREE_FLOW_H) $(TREE_PASS_H) $(TREE_DUMP_H) $(DIAGNOSTIC_H) $(GGC_H) \
>      $(BASIC_BLOCK_H) tree-ssa-propagate.h $(FLAGS_H) $(TREE_DUMP_H) \
>      $(CFGLOOP_H) $(SCEV_H) $(TIMEVAR_H) intl.h tree-pretty-print.h \
> !    gimple-pretty-print.h gimple-fold.h $(OPTABS_H) $(EXPR_H)
>   tree-cfg.o : tree-cfg.c $(TREE_FLOW_H) $(CONFIG_H) $(SYSTEM_H) \
>      $(TREE_H) $(TM_P_H) $(EXPR_H) $(GGC_H) $(FLAGS_H) output.h \
>      $(DIAGNOSTIC_H) $(FUNCTION_H) $(TIMEVAR_H) $(TM_H) coretypes.h \
> Index: gcc/optabs.c
> ===================================================================
> *** gcc/optabs.c.orig	2011-07-11 17:02:51.000000000 +0200
> --- gcc/optabs.c	2011-07-22 11:04:42.000000000 +0200
> *************** can_fix_p (enum machine_mode fixmode, en
> *** 4626,4632 ****
>     return CODE_FOR_nothing;
>   }
>   
> ! static enum insn_code
>   can_float_p (enum machine_mode fltmode, enum machine_mode fixmode,
>   	     int unsignedp)
>   {
> --- 4626,4632 ----
>     return CODE_FOR_nothing;
>   }
>   
> ! enum insn_code
>   can_float_p (enum machine_mode fltmode, enum machine_mode fixmode,
>   	     int unsignedp)
>   {
> Index: gcc/optabs.h
> ===================================================================
> *** gcc/optabs.h.orig	2011-06-22 16:01:28.000000000 +0200
> --- gcc/optabs.h	2011-07-22 11:06:01.000000000 +0200
> *************** extern void expand_fixed_convert (rtx, r
> *** 849,854 ****
> --- 849,857 ----
>   /* Generate code for a FLOAT_EXPR.  */
>   extern void expand_float (rtx, rtx, int);
>   
> + /* Return the insn_code for a FLOAT_EXPR.  */
> + enum insn_code can_float_p (enum machine_mode, enum machine_mode, int);
> + 
>   /* Generate code for a FIX_EXPR.  */
>   extern void expand_fix (rtx, rtx, int);
>   
> 

-- 
Richard Guenther <rguenther@suse.de>
Novell / SUSE Labs
SUSE LINUX Products GmbH - Nuernberg - AG Nuernberg - HRB 16746
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer

Re: [PATCH] Fix PR49715, (float)unsigned -> (float)signed

[Michael Matz]

Hi,

On Fri, 22 Jul 2011, Richard Guenther wrote:

> Regresses vectorization on i?86 because that defines floathi expanders
> but the vectorizer does not recognize a short -> float conversion
> as that requires different sized vectors (the int -> short truncation
> is also a complication for it).

But the vectorizer should be capable of handling this.  Certainly 
vectorizable_conversion tries to handle this.  Perhaps it's only outer 
loop vectorization?

> #define N 40
> 
> Now, I can dumb down the VRP code to only consider doing the
> unsigned -> signed demotion and never truncate (unless maybe
> the target does not support floatMODE of the current input mode).
> 
> Other suggestions?

Adjust testcase to N=300.
Adjust testcase to work with SSE-only math (it seems the floathi expander 
then is disabled).
Fix the vectorizer.

I think artificially dumbing down VRP would be wrong.


Ciao,
Michael.

Re: [PATCH] Fix PR49715, (float)unsigned -> (float)signed

[Richard Guenther]

On Fri, 22 Jul 2011, Richard Guenther wrote:

> On Fri, 22 Jul 2011, Richard Guenther wrote:
> 
> > On Fri, 22 Jul 2011, Richard Guenther wrote:
> > 
> > > On Thu, 21 Jul 2011, Joseph S. Myers wrote:
> > > 
> > > > On Thu, 21 Jul 2011, Richard Guenther wrote:
> > > > 
> > > > > Patch also handling wider modes and not starting with SImode but
> > > > > the mode of int:
> > > > 
> > > > Use of target int for anything not about C ABIs is certainly wrong.  This 
> > > > might be about what operations the target does efficiently, or what 
> > > > functions are present in libgcc (both of which would be functions of 
> > > > machine modes), but it's not about the choice of C int.
> > > 
> > > Ok.  Given rths last suggestion I'm testing the following which
> > > checks all integer modes (but never will widen - optabs.c will do
> > > that if it turns out to be profitable).
> > 
> > Err, I should refresh the patch before sending it ... here it goes.
> 
> Regresses vectorization on i?86 because that defines floathi expanders
> but the vectorizer does not recognize a short -> float conversion
> as that requires different sized vectors (the int -> short truncation
> is also a complication for it).  One of the cases that fail to vectorize 
> is
> 
> #define N 40
> float image[N][N] __attribute__ ((__aligned__(__BIGGEST_ALIGNMENT__)));
> float out[N];
> 
> /* Outer-loop vectorization.  */
> 
> __attribute__ ((noinline)) void
> foo (){
>   int i,j;
> 
>   for (i = 0; i < N; i++) {
>     for (j = 0; j < N; j++) {
>       image[j][i] = j+i;
>     }
>   }
> }
> 
> where VRP figures out that j+i can be represented as a short.
> 
> Now, I can dumb down the VRP code to only consider doing the
> unsigned -> signed demotion and never truncate (unless maybe
> the target does not support floatMODE of the current input mode).
> 
> Other suggestions?

The following implements the above - do not truncate if the target
claims to be able to do the unsigned -> float conversion in the
original modes, but unconditionally try to change it to signed -> float.

Bootstrapped and tested on x86_64-unknown-linux-gnu.

Does this look sensible as a start?  We can always improve things
incrementally when we discover a case that is worthwhile.

Thanks,
Richard.

2011-07-22  Richard Guenther  <rguenther@suse.de>

	PR tree-optimization/49715
	* tree-vrp.c: Include expr.h and optabs.h.
	(range_fits_type_): New function.
	(simplify_float_conversion_using_ranges): Likewise.
	(simplify_stmt_using_ranges): Call it.
	* Makefile.in (tree-vrp.o): Add $(EXPR_H) and $(OPTABS_H) dependencies.
	* optabs.c (can_float_p): Export.
	* optabs.h (can_float_p): Declare.

	* gcc.target/i386/pr49715-1.c: New testcase.
	* gcc.target/i386/pr49715-2.c: Likewise.

Index: gcc/tree-vrp.c
===================================================================
*** gcc/tree-vrp.c.orig	2011-07-21 17:46:16.000000000 +0200
--- gcc/tree-vrp.c	2011-07-22 14:41:46.000000000 +0200
*************** along with GCC; see the file COPYING3.
*** 40,45 ****
--- 40,47 ----
  #include "tree-ssa-propagate.h"
  #include "tree-chrec.h"
  #include "gimple-fold.h"
+ #include "expr.h"
+ #include "optabs.h"
  
  
  /* Type of value ranges.  See value_range_d for a description of these
*************** simplify_conversion_using_ranges (gimple
*** 7448,7453 ****
--- 7450,7548 ----
    return true;
  }
  
+ /* Return whether the value range *VR fits in an integer type specified
+    by PRECISION and UNSIGNED_P.  */
+ 
+ static bool
+ range_fits_type_p (value_range_t *vr, unsigned precision, bool unsigned_p)
+ {
+   double_int tem;
+ 
+   /* We can only handle constant ranges.  */
+   if (vr->type != VR_RANGE
+       || TREE_CODE (vr->min) != INTEGER_CST
+       || TREE_CODE (vr->max) != INTEGER_CST)
+     return false;
+ 
+   tem = double_int_ext (tree_to_double_int (vr->min), precision, unsigned_p);
+   if (!double_int_equal_p (tree_to_double_int (vr->min), tem))
+     return false;
+ 
+   tem = double_int_ext (tree_to_double_int (vr->max), precision, unsigned_p);
+   if (!double_int_equal_p (tree_to_double_int (vr->max), tem))
+     return false;
+ 
+   return true;
+ }
+ 
+ /* Simplify a conversion from integral SSA name to float in STMT.  */
+ 
+ static bool
+ simplify_float_conversion_using_ranges (gimple_stmt_iterator *gsi, gimple stmt)
+ {
+   tree rhs1 = gimple_assign_rhs1 (stmt);
+   value_range_t *vr = get_value_range (rhs1);
+   enum machine_mode fltmode = TYPE_MODE (TREE_TYPE (gimple_assign_lhs (stmt)));
+   enum machine_mode mode;
+   tree tem;
+   gimple conv;
+ 
+   /* We can only handle constant ranges.  */
+   if (vr->type != VR_RANGE
+       || TREE_CODE (vr->min) != INTEGER_CST
+       || TREE_CODE (vr->max) != INTEGER_CST)
+     return false;
+ 
+   /* First check if we can use a signed type in place of an unsigned.  */
+   if (TYPE_UNSIGNED (TREE_TYPE (rhs1))
+       && (can_float_p (fltmode, TYPE_MODE (TREE_TYPE (rhs1)), 0)
+ 	  != CODE_FOR_nothing)
+       && range_fits_type_p (vr, GET_MODE_PRECISION
+ 			          (TYPE_MODE (TREE_TYPE (rhs1))), 0))
+     mode = TYPE_MODE (TREE_TYPE (rhs1));
+   /* If we can do the conversion in the current input mode do nothing.  */
+   else if (can_float_p (fltmode, TYPE_MODE (TREE_TYPE (rhs1)),
+ 			TYPE_UNSIGNED (TREE_TYPE (rhs1))))
+     return false;
+   /* Otherwise search for a mode we can use, starting from the narrowest
+      integer mode available.  */
+   else
+     {
+       mode = GET_CLASS_NARROWEST_MODE (MODE_INT);
+       do
+ 	{
+ 	  /* If we cannot do a signed conversion to float from mode
+ 	     or if the value-range does not fit in the signed type
+ 	     try with a wider mode.  */
+ 	  if (can_float_p (fltmode, mode, 0) != CODE_FOR_nothing
+ 	      && range_fits_type_p (vr, GET_MODE_PRECISION (mode), 0))
+ 	    break;
+ 
+ 	  mode = GET_MODE_WIDER_MODE (mode);
+ 	  /* But do not widen the input.  Instead leave that to the
+ 	     optabs expansion code.  */
+ 	  if (GET_MODE_PRECISION (mode) > TYPE_PRECISION (TREE_TYPE (rhs1)))
+ 	    return false;
+ 	}
+       while (mode != VOIDmode);
+       if (mode == VOIDmode)
+ 	return false;
+     }
+ 
+   /* It works, insert a truncation or sign-change before the
+      float conversion.  */
+   tem = create_tmp_var (build_nonstandard_integer_type
+ 			  (GET_MODE_PRECISION (mode), 0), NULL);
+   conv = gimple_build_assign_with_ops (NOP_EXPR, tem, rhs1, NULL_TREE);
+   tem = make_ssa_name (tem, conv);
+   gimple_assign_set_lhs (conv, tem);
+   gsi_insert_before (gsi, conv, GSI_SAME_STMT);
+   gimple_assign_set_rhs1 (stmt, tem);
+   update_stmt (stmt);
+ 
+   return true;
+ }
+ 
  /* Simplify STMT using ranges if possible.  */
  
  static bool
*************** simplify_stmt_using_ranges (gimple_stmt_
*** 7507,7512 ****
--- 7602,7613 ----
  	    return simplify_conversion_using_ranges (stmt);
  	  break;
  
+ 	case FLOAT_EXPR:
+ 	  if (TREE_CODE (rhs1) == SSA_NAME
+ 	      && INTEGRAL_TYPE_P (TREE_TYPE (rhs1)))
+ 	    return simplify_float_conversion_using_ranges (gsi, stmt);
+ 	  break;
+ 
  	default:
  	  break;
  	}
Index: gcc/testsuite/gcc.target/i386/pr49715-1.c
===================================================================
*** /dev/null	1970-01-01 00:00:00.000000000 +0000
--- gcc/testsuite/gcc.target/i386/pr49715-1.c	2011-07-21 17:46:39.000000000 +0200
***************
*** 0 ****
--- 1,9 ----
+ /* { dg-do compile } */
+ /* { dg-options "-O2 -msse -mfpmath=sse" } */
+ 
+ float func(unsigned x)
+ {
+   return (x & 0xfffff) * 0.01f;
+ }
+ 
+ /* { dg-final { scan-assembler-times "cvtsi2ss" 1 } } */
Index: gcc/testsuite/gcc.target/i386/pr49715-2.c
===================================================================
*** /dev/null	1970-01-01 00:00:00.000000000 +0000
--- gcc/testsuite/gcc.target/i386/pr49715-2.c	2011-07-21 17:46:39.000000000 +0200
***************
*** 0 ****
--- 1,12 ----
+ /* { dg-do compile } */
+ /* { dg-require-effective-target lp64 } */
+ /* { dg-options "-O2" } */
+ 
+ double func(unsigned long long x)
+ {
+   if (x <= 0x7ffffffffffffffeULL)
+     return (x + 1) * 0.01;
+   return 0.0;
+ }
+ 
+ /* { dg-final { scan-assembler-times "cvtsi2sdq" 1 } } */
Index: gcc/Makefile.in
===================================================================
*** gcc/Makefile.in.orig	2011-07-19 12:59:09.000000000 +0200
--- gcc/Makefile.in	2011-07-22 11:09:16.000000000 +0200
*************** tree-vrp.o : tree-vrp.c $(CONFIG_H) $(SY
*** 2504,2510 ****
     $(TREE_FLOW_H) $(TREE_PASS_H) $(TREE_DUMP_H) $(DIAGNOSTIC_H) $(GGC_H) \
     $(BASIC_BLOCK_H) tree-ssa-propagate.h $(FLAGS_H) $(TREE_DUMP_H) \
     $(CFGLOOP_H) $(SCEV_H) $(TIMEVAR_H) intl.h tree-pretty-print.h \
!    gimple-pretty-print.h gimple-fold.h
  tree-cfg.o : tree-cfg.c $(TREE_FLOW_H) $(CONFIG_H) $(SYSTEM_H) \
     $(TREE_H) $(TM_P_H) $(EXPR_H) $(GGC_H) $(FLAGS_H) output.h \
     $(DIAGNOSTIC_H) $(FUNCTION_H) $(TIMEVAR_H) $(TM_H) coretypes.h \
--- 2504,2510 ----
     $(TREE_FLOW_H) $(TREE_PASS_H) $(TREE_DUMP_H) $(DIAGNOSTIC_H) $(GGC_H) \
     $(BASIC_BLOCK_H) tree-ssa-propagate.h $(FLAGS_H) $(TREE_DUMP_H) \
     $(CFGLOOP_H) $(SCEV_H) $(TIMEVAR_H) intl.h tree-pretty-print.h \
!    gimple-pretty-print.h gimple-fold.h $(OPTABS_H) $(EXPR_H)
  tree-cfg.o : tree-cfg.c $(TREE_FLOW_H) $(CONFIG_H) $(SYSTEM_H) \
     $(TREE_H) $(TM_P_H) $(EXPR_H) $(GGC_H) $(FLAGS_H) output.h \
     $(DIAGNOSTIC_H) $(FUNCTION_H) $(TIMEVAR_H) $(TM_H) coretypes.h \
Index: gcc/optabs.c
===================================================================
*** gcc/optabs.c.orig	2011-07-11 17:02:51.000000000 +0200
--- gcc/optabs.c	2011-07-22 11:04:42.000000000 +0200
*************** can_fix_p (enum machine_mode fixmode, en
*** 4626,4632 ****
    return CODE_FOR_nothing;
  }
  
! static enum insn_code
  can_float_p (enum machine_mode fltmode, enum machine_mode fixmode,
  	     int unsignedp)
  {
--- 4626,4632 ----
    return CODE_FOR_nothing;
  }
  
! enum insn_code
  can_float_p (enum machine_mode fltmode, enum machine_mode fixmode,
  	     int unsignedp)
  {
Index: gcc/optabs.h
===================================================================
*** gcc/optabs.h.orig	2011-06-22 16:01:28.000000000 +0200
--- gcc/optabs.h	2011-07-22 11:06:01.000000000 +0200
*************** extern void expand_fixed_convert (rtx, r
*** 849,854 ****
--- 849,857 ----
  /* Generate code for a FLOAT_EXPR.  */
  extern void expand_float (rtx, rtx, int);
  
+ /* Return the insn_code for a FLOAT_EXPR.  */
+ enum insn_code can_float_p (enum machine_mode, enum machine_mode, int);
+ 
  /* Generate code for a FIX_EXPR.  */
  extern void expand_fix (rtx, rtx, int);
  

Re: [PATCH] Fix PR49715, (float)unsigned -> (float)signed

[Richard Henderson]

On 07/22/2011 07:11 AM, Richard Guenther wrote:
> Does this look sensible as a start?  We can always improve things
> incrementally when we discover a case that is worthwhile.

Looks good to me.


r~

Re: [PATCH] Fix PR49715, (float)unsigned -> (float)signed

[H.J. Lu]

On Fri, Jul 22, 2011 at 2:13 AM, Richard Guenther <rguenther@suse.de> wrote:
> On Fri, 22 Jul 2011, Richard Guenther wrote:
>
>> On Thu, 21 Jul 2011, Joseph S. Myers wrote:
>>
>> > On Thu, 21 Jul 2011, Richard Guenther wrote:
>> >
>> > > Patch also handling wider modes and not starting with SImode but
>> > > the mode of int:
>> >
>> > Use of target int for anything not about C ABIs is certainly wrong.  This
>> > might be about what operations the target does efficiently, or what
>> > functions are present in libgcc (both of which would be functions of
>> > machine modes), but it's not about the choice of C int.
>>
>> Ok.  Given rths last suggestion I'm testing the following which
>> checks all integer modes (but never will widen - optabs.c will do
>> that if it turns out to be profitable).
>
> Err, I should refresh the patch before sending it ... here it goes.
>
> Richard.
>
> 2011-07-22  Richard Guenther  <rguenther@suse.de>
>
>        PR tree-optimization/49715
>        * tree-vrp.c: Include expr.h and optabs.h.
>        (simplify_float_conversion_using_ranges): New function.
>        (simplify_stmt_using_ranges): Call it.
>        * Makefile.in (tree-vrp.o): Add $(EXPR_H) and $(OPTABS_H) dependencies.
>        * optabs.c (can_float_p): Export.
>        * optabs.h (can_float_p): Declare.
>
>        * gcc.target/i386/pr49715-1.c: New testcase.
>        * gcc.target/i386/pr49715-2.c: Likewise.
>

I think this caused:

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=49840

-- 
H.J.

Re: [PATCH] Fix PR49715, (float)unsigned -> (float)signed

[Richard Guenther]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 1762 bytes --]

On Mon, 25 Jul 2011, H.J. Lu wrote:

> On Fri, Jul 22, 2011 at 2:13 AM, Richard Guenther <rguenther@suse.de> wrote:
> > On Fri, 22 Jul 2011, Richard Guenther wrote:
> >
> >> On Thu, 21 Jul 2011, Joseph S. Myers wrote:
> >>
> >> > On Thu, 21 Jul 2011, Richard Guenther wrote:
> >> >
> >> > > Patch also handling wider modes and not starting with SImode but
> >> > > the mode of int:
> >> >
> >> > Use of target int for anything not about C ABIs is certainly wrong.  This
> >> > might be about what operations the target does efficiently, or what
> >> > functions are present in libgcc (both of which would be functions of
> >> > machine modes), but it's not about the choice of C int.
> >>
> >> Ok.  Given rths last suggestion I'm testing the following which
> >> checks all integer modes (but never will widen - optabs.c will do
> >> that if it turns out to be profitable).
> >
> > Err, I should refresh the patch before sending it ... here it goes.
> >
> > Richard.
> >
> > 2011-07-22  Richard Guenther  <rguenther@suse.de>
> >
> >        PR tree-optimization/49715
> >        * tree-vrp.c: Include expr.h and optabs.h.
> >        (simplify_float_conversion_using_ranges): New function.
> >        (simplify_stmt_using_ranges): Call it.
> >        * Makefile.in (tree-vrp.o): Add $(EXPR_H) and $(OPTABS_H) dependencies.
> >        * optabs.c (can_float_p): Export.
> >        * optabs.h (can_float_p): Declare.
> >
> >        * gcc.target/i386/pr49715-1.c: New testcase.
> >        * gcc.target/i386/pr49715-2.c: Likewise.
> >
> 
> I think this caused:
> 
> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=49840

I didn't see those with -m32 on x86_64.  Would we expect these to
only show up on a host i?86 machine?

Richard.