Re: combine_conversions int->double->int

[Marc Glisse <marc dot glisse at inria dot fr>]

On Wed, 25 Apr 2012, Richard Guenther wrote:

> On Wed, Apr 25, 2012 at 10:12 AM, Marc Glisse <marc.glisse@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.glisse@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.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.


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 Glisse

Attachment: p3
Description: Text document