On Mon, Sep 3, 2018 at 1:32 PM Aldy Hernandez <al...@redhat.com> wrote: > > > > On 08/28/2018 05:27 AM, Richard Biener wrote: > > On Mon, Aug 27, 2018 at 2:24 PM Aldy Hernandez <al...@redhat.com> wrote: > >> > >> Howdy! > >> > >> Phew, I think this is the last abstraction. This handles the unary > >> CONVERT_EXPR_P code. > >> > >> It's the usual story-- normalize the symbolics to [-MIN,+MAX] and handle > >> everything generically. > >> > >> Normalizing the symbolics brought about some nice surprises. We now > >> handle a few things we were punting on before, which I've documented in > >> the patch, but can remove if so desired. I wrote them mainly for myself: > >> > >> /* NOTES: Previously we were returning VARYING for all symbolics, but > >> we can do better by treating them as [-MIN, +MAX]. For > >> example, converting [SYM, SYM] from INT to LONG UNSIGNED, > >> we can return: ~[0x8000000, 0xffffffff7fffffff]. > >> > >> We were also failing to convert ~[0,0] from char* to unsigned, > >> instead choosing to return VR_VARYING. Now we return ~[0,0]. */ > >> > >> Tested on x86-64 by the usual bootstrap and regtest gymnastics, > >> including --enable-languages=all, because my past sins are still > >> haunting me. > >> > >> OK? > > > > The new wide_int_range_convert_tree looks odd given it returns > > tree's. I'd have expected an API that does the conversion resulting > > in a wide_int range and the VRP code adapting to that by converting > > the result to trees. > > Rewritten as suggested. > > A few notes. > > First. I am not using widest_int as was done previously. We were > passing 0/false to the overflow args in force_fit_type so the call was > just degrading into wide_int_to_tree() anyhow. Am I missing some > subtlety here, and must we use widest_int and then force_fit_type on the > caller?
The issue is that the wide-ints get "converted", so it's indeed subtle. + || wi::rshift (wi::sub (vr0_max, vr0_min), + wi::uhwi (outer_prec, inner_prec), + inner_sign) == 0) this was previously allowed only for VR_RANGEs - now it's also for VR_ANTI_RANGE. That looks incorrect. Testcase welcome ;) + return (!wi::eq_p (min, wi::min_value (outer_prec, outer_sign)) + || !wi::eq_p (max, wi::max_value (outer_prec, outer_sign))); so you return false for VARYING? Why? I'd simply return true and return VARYING in the new type in the path you return false. Richard. > > Second. I need extract_range_into_wide_ints to work with anti ranges of > constants. It seems like only the unary handling of CONVERT_EXPR here > uses it that way, so I believe it should go into one patch. If you > believe strongly otherwise, I could split out the 4 lines into a > separate patch, but I'd prefer not to. > > Finally, I could use vr0_min.get_precision() instead of inner_precision, > but I think it's more regular and readable the way I have it. > > Tested on all languages on x86-64 Linux. > > OK for trunk?
