On October 4, 2019 5:38:09 PM GMT+02:00, Jeff Law <l...@redhat.com> wrote: >On 10/4/19 6:59 AM, Aldy Hernandez wrote: >> When I did the value_range canonicalization work, I noticed that an >> unsigned [1,MAX] and an ~[0,0] could be two different representations >> for the same thing. I didn't address the problem then because >callers >> to ranges_from_anti_range() would go into an infinite loop trying to >> extract ~[0,0] into [1,MAX] and []. We had a lot of callers to >> >ranges_from_anti_range, and it smelled like a rat's nest, so I bailed. >> >> Now that we have one main caller (from the symbolic PLUS/MINUS >> handling), it's a lot easier to contain. Well, singleton_p also >calls >> >it, but it's already handling nonzero specially, so it wouldn't be affected. >> >> >> With some upcoming cleanups I'm about to post, the fact that [1,MAX] >and >> ~[0,0] are equal_p(), but not nonzero_p(), matters. Plus, it's just >> good form to have one representation, giving us the ability to pick >at >> nonzero_p ranges with ease. >> >> The code in extract_range_from_plus_minus_expr() continues to be a >mess >> (as it has always been), but at least it's contained, and with this >> patch, it's slightly smaller. >> >> Note, I'm avoiding adding a comment header for functions with highly >> descriptive obvious names. >> >> OK? >> >> Aldy >> >> canonicalize-nonzero-ranges.patch >> >> commit 1c333730deeb4ddadc46ad6d12d5344f92c0352c >> Author: Aldy Hernandez <al...@redhat.com> >> Date: Fri Oct 4 08:51:25 2019 +0200 >> >> Canonicalize UNSIGNED [1,MAX] into ~[0,0]. >> >> Adapt PLUS/MINUS symbolic handling, so it doesn't call >> ranges_from_anti_range with a VR_ANTI_RANGE containing one >sub-range. >> >> diff --git a/gcc/ChangeLog b/gcc/ChangeLog >> index 6e4f145af46..3934b41fdf9 100644 >> --- a/gcc/ChangeLog >> +++ b/gcc/ChangeLog >> @@ -1,3 +1,18 @@ >> +2019-10-04 Aldy Hernandez <al...@redhat.com> >> + >> + * tree-vrp.c (value_range_base::singleton_p): Use num_pairs >> + instead of calling vrp_val_is_*. >> + (value_range_base::set): Canonicalize unsigned [1,MAX] into >> + non-zero. >> + (range_has_numeric_bounds_p): New. >> + (range_int_cst_p): Use range_has_numeric_bounds_p. >> + (ranges_from_anti_range): Assert that we won't recurse >> + indefinitely. >> + (extract_extremes_from_range): New. >> + (extract_range_from_plus_minus_expr): Adapt so we don't call >> + ranges_from_anti_range with an anti-range containing only one >> + sub-range. >So no problem with the implementation, but I do have a higher level >question. > >One of the goals of the representation side of the Ranger project is to >drop anti-ranges. Canonicalizing [1, MAX] to ~[0,0] seems to be going >in the opposite direction. So do we really want to canonicalize to >~[0,0]?
No, we don't. Richard. >jeff