VRP: allow unsigned truncating conversions that will fit
diff mbox series

Message ID db121669-f55d-749d-2337-7827d1e82338@redhat.com
State New
Headers show
Series
  • VRP: allow unsigned truncating conversions that will fit
Related show

Commit Message

Aldy Hernandez Sept. 14, 2018, 10:42 a.m. UTC
I'm still thinking about this one...

Is there a subtle reason why we're avoiding unsigned truncating 
conversions of the form:

	[X, +INF]

If the X fits in the new type, why can't we just build [X, +INF] in the 
new type?  See attached patch.

If there isn't, OK for trunk?

Comments

Aldy Hernandez Sept. 14, 2018, 11:05 a.m. UTC | #1
On 09/14/2018 06:42 AM, Aldy Hernandez wrote:
> I'm still thinking about this one...
> 
> Is there a subtle reason why we're avoiding unsigned truncating 
> conversions of the form:
> 
>      [X, +INF]
> 
> If the X fits in the new type, why can't we just build [X, +INF] in the 
> new type?  See attached patch.

For that matter, the new type doesn't even have to be unsigned, as long 
as the old type is unsigned.

[25, +INF] can happily convert from long long unsigned to signed char.

> 
> If there isn't, OK for trunk?
Michael Matz Sept. 14, 2018, 12:33 p.m. UTC | #2
Hi,

On Fri, 14 Sep 2018, Aldy Hernandez wrote:

> Is there a subtle reason why we're avoiding unsigned truncating conversions of
> the form:
> 
> 	[X, +INF]
> 
> If the X fits in the new type, why can't we just build [X, +INF] in the new
> type?

But (uin8_t)((unsigned)[255,+INF]) aka ([255,+INF] & 255) isn't [255,+INF] 
but rather [0,+INF].  What am I missing?  Even considering that the caller 
must canonicalize, I don't see how that would transform [255,+INF] (which 
is a normal range) into [0,+INF].


Ciao,
Michael.
Aldy Hernandez Sept. 17, 2018, 9:54 a.m. UTC | #3
On 09/14/2018 08:33 AM, Michael Matz wrote:
> Hi,
> 
> On Fri, 14 Sep 2018, Aldy Hernandez wrote:
> 
>> Is there a subtle reason why we're avoiding unsigned truncating conversions of
>> the form:
>>
>> 	[X, +INF]
>>
>> If the X fits in the new type, why can't we just build [X, +INF] in the new
>> type?
> 
> But (uin8_t)((unsigned)[255,+INF]) aka ([255,+INF] & 255) isn't [255,+INF]
> but rather [0,+INF].  What am I missing?  Even considering that the caller
> must canonicalize, I don't see how that would transform [255,+INF] (which
> is a normal range) into [0,+INF].

Ughh, yes.  You're absolutely right.  I was getting confused with how we 
special handled anti-ranges of pointers which make no sense when you 
split them up into it's pieces and try to do the math.  And it made no 
sense because I missed that pointers are actually special and we only 
care about null / non-nullness.

Anyways...ignore the noise.  I have a much cleaner patch to the 
tree-vrp.c conversion code that I'm testing now.

Thanks for your input.
Aldy

Patch
diff mbox series

commit b2e8c9baeeca3e321957d205b2c4c8b83b9bc754
Author: Aldy Hernandez <aldyh@redhat.com>
Date:   Fri Sep 14 12:02:42 2018 +0200

            * wide-int-range.cc (wide_int_range_convert): Allow unsigned
            truncating conversions that will fit in the new type.

diff --git a/gcc/wide-int-range.cc b/gcc/wide-int-range.cc
index a85fe9f9ad7..4e323a63206 100644
--- a/gcc/wide-int-range.cc
+++ b/gcc/wide-int-range.cc
@@ -768,6 +768,24 @@  wide_int_range_convert (wide_int &min, wide_int &max,
       return (!wi::eq_p (min, wi::min_value (outer_prec, outer_sign))
 	      || !wi::eq_p (max, wi::max_value (outer_prec, outer_sign)));
     }
+  /* Unsigned truncating conversions whose lower bound fits, but whose
+     upper bound is +INF can be handled straightforwardly.  This will
+     handle things like:
+
+     [1,+INF] (i.e. ~[0,0]) from long unsigned to unsigned short
+     [1000, +INF] from long unsigned to unsigned short.  */
+  if (inner_sign == UNSIGNED
+      && inner_sign == outer_sign
+      && inner_prec > outer_prec
+      && wi::le_p (vr0_min,
+		   wi::mask (outer_prec, false, inner_prec),
+		   UNSIGNED)
+      && vr0_max == wi::max_value (inner_prec, UNSIGNED))
+    {
+      min = wide_int::from (vr0_min, outer_prec, inner_sign);
+      max = wi::mask (outer_prec, false, outer_prec);
+      return true;
+    }
   return false;
 }