From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-pg1-x52b.google.com (mail-pg1-x52b.google.com [IPv6:2607:f8b0:4864:20::52b]) by sourceware.org (Postfix) with ESMTPS id 61CA83858C2F for ; Sun, 10 Jul 2022 03:43:22 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 61CA83858C2F Received: by mail-pg1-x52b.google.com with SMTP id 145so2112564pga.12 for ; Sat, 09 Jul 2022 20:43:22 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:message-id:date:mime-version:user-agent:subject :content-language:to:cc:references:from:in-reply-to :content-transfer-encoding; bh=aTICNF2qN5aFCndqRx5ATG5brO/SFazn8PjalrKnNco=; b=Eh/FylF8Sc0d46oFABE3SeuF7dEvc+xQP0WICO9S9jZdihANcNe+1tLXWef4hGlTUB zjK4gEkd2SdeOmM4u00G5bQBWcHAXiXfgL+wkfGgXuqjqnTpRU/KWinklMJYoEUUbVmo 63ZFIIbJyOytv74RkKP+UbZX9Rph40LZwQfzXYaxyi9W7QG5rnKsBnqBKdYVkfOrGA6T O2/bgl4vOsQzikQZ/7cgTdQd74L2sQUxPSbzzP8vIrFq5k61V+QtY1VQtf9M4UNpxp7F WXH4PJ5m8tkJuckJZgGo/dyHKHzjDmrb23QIiU3p5QZve1u3sroDvWSjH9CqCGSZ2Cm8 S6+w== X-Gm-Message-State: AJIora8+QudlvM4CmvIHYL6QCDhVAiyaQm91ctqi5t6mD4ljyy3wkyb0 RqI0IMBR43Lv9JhdVnFtncEAYSke754= X-Google-Smtp-Source: AGRyM1u2qWJT5KY9WpPXYt8mj2MHCBnw85MZsRUSE9Xkxp0Plhx/j96o0SXwlK9HbyFYTEYOWnDZKQ== X-Received: by 2002:a63:2486:0:b0:412:9de2:eb49 with SMTP id k128-20020a632486000000b004129de2eb49mr10266185pgk.224.1657424600907; Sat, 09 Jul 2022 20:43:20 -0700 (PDT) Received: from [172.31.0.204] (c-73-63-24-84.hsd1.ut.comcast.net. [73.63.24.84]) by smtp.gmail.com with ESMTPSA id x190-20020a6286c7000000b005252680aa30sm2207565pfd.3.2022.07.09.20.43.20 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Sat, 09 Jul 2022 20:43:20 -0700 (PDT) Message-ID: Date: Sat, 9 Jul 2022 21:43:19 -0600 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:91.0) Gecko/20100101 Thunderbird/91.10.0 Subject: Re: [PATCH] Implement global ranges for all vrange types (SSA_NAME_RANGE_INFO). Content-Language: en-US To: Aldy Hernandez , "MacLeod, Andrew" Cc: gcc-patches References: <20220706171019.738993-1-aldyh@redhat.com> From: Jeff Law In-Reply-To: Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit X-Spam-Status: No, score=-2.5 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, FREEMAIL_FROM, NICE_REPLY_A, RCVD_IN_DNSWL_NONE, SPF_HELO_NONE, SPF_PASS, TXREP, T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org X-BeenThere: gcc-patches@gcc.gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gcc-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sun, 10 Jul 2022 03:43:24 -0000 On 7/9/2022 1:31 PM, Aldy Hernandez wrote: > On Sat, Jul 9, 2022 at 6:16 PM Jeff Law via Gcc-patches > wrote: >> >> >> On 7/6/2022 11:10 AM, Aldy Hernandez via Gcc-patches wrote: >>> Currently SSA_NAME_RANGE_INFO only handles integer ranges, and loses >>> half the precision in the process because its use of legacy >>> value_range's. This patch rewrites all the SSA_NAME_RANGE_INFO >>> (nonzero bits included) to use the recently contributed >>> vrange_storage. With it, we'll be able to efficiently save any ranges >>> supported by ranger in GC memory. Presently this will only be >>> irange's, but shortly we'll add floating ranges and others to the mix. >>> >>> As per the discussion with the trailing_wide_ints adjustments and >>> vrange_storage, we'll be able to save integer ranges with a maximum of >>> 5 sub-ranges. This could be adjusted later if more sub-ranges are >>> needed (unlikely). >>> >>> Since this is a behavior changing patch, I would like to take a few >>> days for discussion, and commit early next week if all goes well. >>> >>> A few notes. >>> >>> First, we get rid of the SSA_NAME_ANTI_RANGE_P bit in the SSA_NAME >>> since we store full resolution ranges. Perhaps it could be re-used >>> for something else. >>> >>> The range_info_def struct is gone in favor of an opaque type handled >>> by vrange_storage. It currently supports irange, but will support >>> frange, prange, etc, in due time. >>> >>> From the looks of it, set_range_info was an update operation despite >>> its name, as we improved the nonzero bits with each call, even though >>> we clobbered the ranges. Presumably this was because doing a proper >>> intersect of ranges lost information with the anti-range hack. We no >>> longer have this limitation so now we formalize both set_range_info >>> and set_nonzero_bits to an update operation. After all, we should >>> never be losing information, but enhancing it whenever possible. This >>> means, that if folks' finger-memory is not offended, as a follow-up, >>> I'd like to rename set_nonzero_bits and set_range_info to update_*. >>> >>> I have kept the same global API we had in tree-ssanames.h, with the >>> caveat that all set operations are now update as discussed above. >>> >>> There is a 2% performance penalty for evrp and a 3% penalty for VRP >>> that is coincidentally in line with a previous improvement of the same >>> amount in the vrange abstraction patchset. Interestingly, this >>> penalty is mostly due to the wide int to tree dance we keep doing with >>> irange and legacy. In a first draft of this patch where I was >>> streaming trees directly, there was actually a small improvement >>> instead. I hope to get some of the gain back when we move irange's to >>> wide-ints, though I'm not in a hurry ;-). >>> >>> Tested and benchmarked on x86-64 Linux. I will also test on ppc64le >>> before the final commit. >>> >>> Comments welcome. >>> >>> gcc/ChangeLog: >>> >>> * gimple-range.cc (gimple_ranger::export_global_ranges): Remove >>> verification against legacy value_range. >>> * tree-core.h (struct range_info_def): Remove. >>> (struct irange_storage_slot): New. >>> (struct tree_base): Remove SSA_NAME_ANTI_RANGE_P documentation. >>> (struct tree_ssa_name): Add vrange_storage support. >>> * tree-ssanames.cc (range_info_p): New. >>> (range_info_fits_p): New. >>> (range_info_alloc): New. >>> (range_info_free): New. >>> (range_info_get_range): New. >>> (range_info_set_range): New. >>> (set_range_info_raw): Remove. >>> (set_range_info): Adjust to use vrange_storage. >>> (set_nonzero_bits): Same. >>> (get_nonzero_bits): Same. >>> (duplicate_ssa_name_range_info): Remove overload taking >>> value_range_kind. >>> Rewrite tree overload to use vrange_storage. >>> (duplicate_ssa_name_fn): Adjust to use vrange_storage. >>> * tree-ssanames.h (struct range_info_def): Remove. >>> (set_range_info): Adjust prototype to take vrange. >>> * tree-vrp.cc (vrp_asserts::remove_range_assertions): Call >>> duplicate_ssa_name_range_info. >>> * tree.h (SSA_NAME_ANTI_RANGE_P): Remove. >>> (SSA_NAME_RANGE_TYPE): Remove. >>> * value-query.cc (get_ssa_name_range_info): Adjust to use >>> vrange_storage. >>> (update_global_range): Use int_range_max. >>> (get_range_global): Remove as_a. >> I'll be so happy once we don't have to keep doing the conversions >> between the types. >> >> Anti-ranges no more! > Yeah, it took a little longer than the 6 weeks Andrew had estimated > originally :-P. > > Note that anti range kinda sorta still exist in two forms: > > a) If you use value_range, as it still uses the legacy stuff > underneath. But any new consumers (evrp, DOM, etc), all pass an > int_range or an int_range_max, so anyone who cares about ranges > should never see an anti range. Later this cycle value_range will be > typedefed to what is now Value_Range, which is an infinite precision > range that works for all types the ranger supports. So anti-ranges > here will die a quick death. > > b) There are some passes which still use the deprecated > irange::kind(). This method will return VR_ANTI_RANGE if the range > looks like this [-MIN, 123][567,+MAX]. But kind() is just a > convenience function so that passes that have yet to be converted can > still pretend they see anti-ranges. Underneath a non-legacy irange > has no concept of an anti-range. > > Currently, I see the following passes still using the anti-range nonsense: > > gimple-array-bounds.cc > gimple-ssa-warn-restrict.cc > ipa-fnsummary.cc > ipa-prop.cc > pointer-query.cc > tree-ssa-strlen.cc > > I don't understand the ipa-* stuff, so I never touched it. OTOH, the > middle end warnings always break when you improve ranges so I left > them alone. I wouldn't worry much about the IPA stuff.  But we should make an effort to kill the ANTI ranges in other places (then we'll bug the IPA guys to fix their code :-). I'm not sure what's best to tackle first.  array-bounds and warn-restrict are probably smaller, though you have to work through the problems that pop up when they're given better range data. pointer-query.cc is probably the easiest place to start.  It looks fairly well documented in terms of what it's doing and why. tree-ssa-strlen is probably a good second choice, mostly because I think we're less likely to run into "we gave it better range data and got more/new diagnostics" problem. jeff