From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 52071 invoked by alias); 4 Sep 2018 11:58:48 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Received: (qmail 52059 invoked by uid 89); 4 Sep 2018 11:58:47 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-2.9 required=5.0 tests=AWL,BAYES_00,FREEMAIL_FROM,RCVD_IN_DNSWL_NONE,SPF_PASS autolearn=ham version=3.3.2 spammy=wi X-HELO: mail-lf1-f49.google.com Received: from mail-lf1-f49.google.com (HELO mail-lf1-f49.google.com) (209.85.167.49) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Tue, 04 Sep 2018 11:58:46 +0000 Received: by mail-lf1-f49.google.com with SMTP id c29-v6so2733857lfj.1 for ; Tue, 04 Sep 2018 04:58:46 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=W1BnhKdb7pszEylZlGHZdyrHwqDjrj9p8P+tH9X3yoM=; b=j8NfWzrjZA0KCVPYMmkuP1900uKTpsLXtKXYGeDswZWtgteLDlEDM4aUcHAGfdLCBr YzInS2StOkfPwHpPjtzwKitE6QWZ7b62gNc7hP6vi8FVGmzwAEHIZzZyVfaDTQkKEiyH hv51nri1vOnawSDnocpW78vTmmg70kFJLeBsL++Pb9GXI0cODkoJlbrCIX6F5HGReiso MeKwcQodU3a7lwkJ2R4X8z7WSThR4UkAWKqb5W3d053IrM2za6LOnMXd03dUp6ct3Atq U8DQslKIwLTiIlkrk9PA4bpp2Ixh8ZPL6VkgFSBmVHnHnS3/5YacMAAQ+8spEw/mYoyH FhIA== MIME-Version: 1.0 References: In-Reply-To: From: Richard Biener Date: Tue, 04 Sep 2018 11:58:00 -0000 Message-ID: Subject: Re: VRP: abstract out wide int CONVERT_EXPR_P code To: Aldy Hernandez Cc: GCC Patches Content-Type: text/plain; charset="UTF-8" X-IsSubscribed: yes X-SW-Source: 2018-09/txt/msg00173.txt.bz2 On Mon, Sep 3, 2018 at 1:32 PM Aldy Hernandez wrote: > > > > On 08/28/2018 05:27 AM, Richard Biener wrote: > > On Mon, Aug 27, 2018 at 2:24 PM Aldy Hernandez 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?