From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-pl1-x62c.google.com (mail-pl1-x62c.google.com [IPv6:2607:f8b0:4864:20::62c]) by sourceware.org (Postfix) with ESMTPS id 2569238485B9 for ; Sat, 9 Jul 2022 16:15:49 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 2569238485B9 Received: by mail-pl1-x62c.google.com with SMTP id c13so1183058pla.6 for ; Sat, 09 Jul 2022 09:15:49 -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:references:from:in-reply-to :content-transfer-encoding; bh=GgkDpM9sMsFOxT3htMVuliuSz5b+qvsWiZQFSFbJxe8=; b=M1rai2yH79vk24tNYF21FFnXzjen9c7oZu+dyreFrjNUrVk9YM3nYI/uD0o3XaeAaB 6nNo4Y7wADntm1S5ISekWELC7cA8pYDh/921x2izRFHKoj0P99afwhVFxRSPHIQgNbYD TPKKvju3rd3xxbc0OjSDYASnZDk5Aj/5a1/ckuq8Ro4hNpTQFhGd24SI6H9bf0PLgwNk XHBFxcU51WmSVUjZe9y8ZsIQa6ahwdPNd/7h6hrq4r7e8rwS7o2pV7hlnGycgRoI0/Tz uDab3YylztkHtuMnOeb0m8GmWpxPRRafo7tw4cg8eNy6cJj97Ra4f2L+D5zkIZ7lZexr Hwtw== X-Gm-Message-State: AJIora+4qA18N28/whOqSex4ZauGsSs3UP5c9DIHnzEi3J+yW1+jZC8R ciujBkyYhday41tZ5zpgc1eyeR0xjA8= X-Google-Smtp-Source: AGRyM1u1w9Ur39Q/gRioJNbh+eHXiQI9wFrqpvuJWJ/BzDOr1nNtWk5ZXFW+h+YGArRbTbEjd8ivCw== X-Received: by 2002:a17:90a:a796:b0:1ef:85de:6d9c with SMTP id f22-20020a17090aa79600b001ef85de6d9cmr6636618pjq.201.1657383346656; Sat, 09 Jul 2022 09:15:46 -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 k145-20020a628497000000b005289798ecc3sm1616748pfd.183.2022.07.09.09.15.46 for (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Sat, 09 Jul 2022 09:15:46 -0700 (PDT) Message-ID: Date: Sat, 9 Jul 2022 10:15:45 -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: gcc-patches@gcc.gnu.org References: <20220706171019.738993-1-aldyh@redhat.com> From: Jeff Law In-Reply-To: <20220706171019.738993-1-aldyh@redhat.com> 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: Sat, 09 Jul 2022 16:15:51 -0000 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! I've got no real concerns here.  So unless someone objects, your plan is OK. jeff