On Wed, 25 Apr 2012, Richard Guenther wrote:
On Wed, Apr 25, 2012 at 10:12 AM, Marc Glisse <marc.gli...@inria.fr> wrote:
Hello,
a conversion like int->double->int is just the identity, as long as double
is big enough to represent all ints exactly. The most natural way I found to
do this optimization is the attached:
2012-04-25 Marc Glisse <marc.gli...@inria.fr>
PR middle-end/27139
* tree-ssa-forwprop.c (combine_conversions): Handle INT->FP->INT.
Does the approach make sense? I don't know that code, and adding FLOAT_EXPR
/ FIX_TRUNC_EXPR was a bit of guesswork. The precision of double could be
multiplied by log2(base), but not doing it is safe. If the approach is ok, I
could extend it so int->double->long also skips the intermediate conversion.
Bootstrapped and regression tested on linux-x86_64.
Should I try and write a testcase for a specific target checking for
specific asm instructions there, or is there a better way?
Well, scanning the forwprop dump for example.
Btw, I think not munging this new case into the existing CONVERT_EXPR_P
code would be better - it makes the code (even) harder to understand and
I'm not convinced that adding FLOAT_EXPR/FIX_TRUNC_EXPR handling
does not wreck any assumptions in that code.
It also seems that for DECIMAL_FLOAT_TYPE_P the transform is always
valid?
Richard.
(note that I can't commit)
Here is take 2 on this patch, which seems cleaner. Bootstrapped and
regression tested.
gcc/ChangeLog
2012-04-25 Marc Glisse <marc.gli...@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.gli...@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.
While doing this, I noticed what I think is a mistake in a comment:
--- gcc/real.c (revision 186761)
+++ gcc/real.c (working copy)
@@ -2814,11 +2814,11 @@ significand_size (enum machine_mode mode
return 0;
if (fmt->b == 10)
{
/* Return the size in bits of the largest binary value that can be
- held by the decimal coefficient for this mode. This is one more
+ held by the decimal coefficient for this mode. This is one less
than the number of bits required to hold the largest coefficient
of this mode. */
double log2_10 = 3.3219281;
return fmt->p * log2_10;
}
--
Marc GlisseIndex: 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" } */
+
+short f1(short n)
+{
+ return (double)n;
+}
+long f2(short n)
+{
+ return (double)n;
+}
+
+long g1(long n)
+{
+ return (float)n;
+}
+short g2(long n)
+{
+ return (float)n;
+}
+
+/* { dg-final { scan-tree-dump-times "\\\(float\\\)" 2 "forwprop1" } } */
+/* { dg-final { scan-tree-dump-not "\\\(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 186761)
+++ 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))
{
tree defop0 = gimple_assign_rhs1 (def_stmt);
tree type = TREE_TYPE (lhs);
tree inside_type = TREE_TYPE (defop0);
tree inter_type = TREE_TYPE (op0);
@@ -2552,10 +2555,45 @@ combine_conversions (gimple_stmt_iterato
gimple_assign_set_rhs_from_tree (gsi, tem);
update_stmt (gsi_stmt (*gsi));
return 1;
}
}
+ else if (code == FIX_TRUNC_EXPR && 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);
+ int inside_int = INTEGRAL_TYPE_P (inside_type);
+ int inter_float = FLOAT_TYPE_P (inter_type);
+ int final_int = INTEGRAL_TYPE_P (type);
+ int inside_prec = TYPE_PRECISION (inside_type);
+ int inside_unsignedp = TYPE_UNSIGNED (inside_type);
+
+ /* 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 &&
+ 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;
}
/* Main entry point for the forward propagation and statement combine
