From patchwork Wed Apr 26 19:26:00 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Andrew MacLeod X-Patchwork-Id: 1774241 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@legolas.ozlabs.org Authentication-Results: legolas.ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=gcc.gnu.org (client-ip=2620:52:3:1:0:246e:9693:128c; helo=sourceware.org; envelope-from=gcc-patches-bounces+incoming=patchwork.ozlabs.org@gcc.gnu.org; receiver=) Authentication-Results: legolas.ozlabs.org; dkim=pass (1024-bit key; unprotected) header.d=gcc.gnu.org header.i=@gcc.gnu.org header.a=rsa-sha256 header.s=default header.b=AO74ickK; dkim-atps=neutral Received: from sourceware.org (server2.sourceware.org [IPv6:2620:52:3:1:0:246e:9693:128c]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (P-384) server-digest SHA384) (No client certificate requested) by legolas.ozlabs.org (Postfix) with ESMTPS id 4Q684f4fW1z23vF for ; Thu, 27 Apr 2023 05:26:42 +1000 (AEST) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 16EFE38555BF for ; Wed, 26 Apr 2023 19:26:39 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 16EFE38555BF DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gcc.gnu.org; s=default; t=1682537199; bh=4EouVqwk98Mj0TBp06rMh8JPtLSs3a4y7BQ+rAlIa80=; h=Date:Subject:To:Cc:References:In-Reply-To:List-Id: List-Unsubscribe:List-Archive:List-Post:List-Help:List-Subscribe: From:Reply-To:From; b=AO74ickKh072olhV3N9tpt/uuivdA5MUy+zZ+Eh8fJamR6K9ud7omoCc5FWyrwzoU T9UpD/UhU+3war4lvmzD0H6PCP+18EpveL9nUdSuUq9wxisOGRAiJtLAJPD+kTuDrU R/CgtXfuJcCyE3koJZflBH96aPtkiIlJpAPzigtc= X-Original-To: gcc-patches@gcc.gnu.org Delivered-To: gcc-patches@gcc.gnu.org Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by sourceware.org (Postfix) with ESMTPS id D49FD3857713 for ; Wed, 26 Apr 2023 19:26:04 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org D49FD3857713 Received: from mail-qk1-f197.google.com (mail-qk1-f197.google.com [209.85.222.197]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-468-dhbwCJI0M4abHOC1ESWtpw-1; Wed, 26 Apr 2023 15:26:02 -0400 X-MC-Unique: dhbwCJI0M4abHOC1ESWtpw-1 Received: by mail-qk1-f197.google.com with SMTP id af79cd13be357-74e1cdf9cbeso79850185a.0 for ; Wed, 26 Apr 2023 12:26:02 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1682537162; x=1685129162; h=in-reply-to:from:references:cc:to:content-language:subject :user-agent:mime-version:date:message-id:x-gm-message-state:from:to :cc:subject:date:message-id:reply-to; bh=GGjrow1PCm7fRjMpnPpUyrcLfEnDUXFdIawVQxy15Tc=; b=NXGe41ZDzqTxU9qGkvAbrjaoSEZfaGW0xa+596PUuFH3dgye8lDInYp+EooBmyMX46 IWWQNzmkIcgkG2mG/yCxNTx63fnxd6VbBEv1LgXnV8CZEt+24YARgrdHIlGNDZoj8dey igo9B8Y483gZjsUIyxLR/ByWpxCbXtZKRxumpXxq8Afzg5S3s7YPkB999oYe0DEmDj8A +8lAIlotNUMRvQPOQVv7U1zjsrdLsZLdtxzHRH0LLcb4sWTrO2ySvIEQrWlTZVjFZPal oKDAsr7F5udjNyUn5UBEutMKr8WLw6ykHWJtwL5xvakbWTPKmlGzyJlbIE+hSMUYaDkt EhyA== X-Gm-Message-State: AC+VfDwDwckz1hd/v+4fWEY4dM5bfyv7S0CkWXhJbfs36Un3JoUjWGbg rJ/MJArf5z+3I9ycfEKy1eQjAw/h7wq1QBzQ2LIUzFikaB+8oqhTdK2nYjp9QDDE5cyEAfRKiPK bn/YReyYKfO5+0zVFXQ== X-Received: by 2002:ac8:5b95:0:b0:3f0:a108:8642 with SMTP id a21-20020ac85b95000000b003f0a1088642mr5676333qta.20.1682537162279; Wed, 26 Apr 2023 12:26:02 -0700 (PDT) X-Google-Smtp-Source: ACHHUZ73cogplJZXrIu1mMxJhQHsrcfTnFdZPRxJSCc9H5JVxYg4XB/Qy+7uvdBOveGPf50W7ccQ4A== X-Received: by 2002:ac8:5b95:0:b0:3f0:a108:8642 with SMTP id a21-20020ac85b95000000b003f0a1088642mr5676310qta.20.1682537162012; Wed, 26 Apr 2023 12:26:02 -0700 (PDT) Received: from ?IPV6:2607:fea8:51df:4200::1dcd? ([2607:fea8:51df:4200::1dcd]) by smtp.gmail.com with ESMTPSA id r16-20020ac85e90000000b003ef189ffa82sm5541208qtx.90.2023.04.26.12.26.01 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Wed, 26 Apr 2023 12:26:01 -0700 (PDT) Message-ID: Date: Wed, 26 Apr 2023 15:26:00 -0400 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.8.0 Subject: [COMMITTED 1/5] PR tree-optimization/109417 - Don't save ssa-name pointer in dependency cache. To: Jeff Law , Richard Biener Cc: gcc-patches@gcc.gnu.org, "hernandez, aldy" References: <428a4619-9653-ff0b-8092-25efc933ba80@gmail.com> <9c61bac0-b3ae-98e6-8abf-5a092db98f64@redhat.com> <6116962e-f273-2d5a-93ff-2408dc4a4cea@gmail.com> In-Reply-To: <6116962e-f273-2d5a-93ff-2408dc4a4cea@gmail.com> X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Language: en-US X-Spam-Status: No, score=-11.7 required=5.0 tests=BAYES_00, DKIMWL_WL_HIGH, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, RCVD_IN_DNSWL_NONE, RCVD_IN_MSPIKE_H2, SPF_HELO_NONE, SPF_NONE, 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-Patchwork-Original-From: Andrew MacLeod via Gcc-patches From: Andrew MacLeod Reply-To: Andrew MacLeod Errors-To: gcc-patches-bounces+incoming=patchwork.ozlabs.org@gcc.gnu.org Sender: "Gcc-patches" On 4/25/23 22:34, Jeff Law wrote: > > > On 4/24/23 07:51, Andrew MacLeod wrote: > >> >> Its not a real cache..  its merely a statement shortcut in dependency >> analysis to avoid re-parsing statements every time we look at them >> for dependency analysis >> >> It is not suppose to be used for anything other than dependency >> checking.   ie, if an SSA_NAME changes, we can check if it matches >> either of the 2 "cached" names on this DEF, and if so, we know this >> name is stale.  we are never actually suppose to use the dependency >> cached values to drive anything, merely respond to the question if >> either matches a given name.   So it doesnt matter if the name here >> has been freed > OK.  I'll take your word for it.  Note that a free'd SSA_NAME may have > an empty TREE_TYPE or an unexpected TREE_CHAIN field IIRC. So you have > to be a bit careful if you're going to allow them. > >> >> >>> We never re-use SSA names from within the pass releasing it.  But if >>> the ranger cache >>> persists across passes this could be a problem.  See >> >> >> This particular valueswould never persist beyond a current pass.. its >> just the dependency chains and they would get rebuilt every time >> because the IL has changed. > Good.  THat would limit the concerns significantly.  I don't think we > recycle names within a pass anymore (we used to within DOM due to the > way threading worked eons ago, but we no longer take things out of SSA > form to handle the CFG/SSA graph updates.  One could even argue we > don't need to maintain the freelist and recycle names anymore. > > Jeff > well, no worries.  taken care of thusly for the future. Its a hair slower, but nothing outrageous Bootstrapped on x86_64-pc-linux-gnu with no regressions.  Pushed. Andrew From a530eb642032da7ad4d30de51131421631055f72 Mon Sep 17 00:00:00 2001 From: Andrew MacLeod Date: Tue, 25 Apr 2023 15:33:52 -0400 Subject: [PATCH 1/5] Don't save ssa-name pointer in dependency cache. If the direct dependence fields point directly to an ssa-name, its possible that an optimization frees an ssa-name, and the value pointed to may now be in the free list. Simply maintain the ssa version number instead. PR tree-optimization/109417 * gimple-range-gori.cc (range_def_chain::register_dependency): Save the ssa version number, not the pointer. (gori_compute::may_recompute_p): No need to check if a dependency is in the free list. * gimple-range-gori.h (class range_def_chain): Change ssa1 and ssa2 fields to be unsigned int instead of trees. (ange_def_chain::depend1): Adjust. (ange_def_chain::depend2): Adjust. * gimple-range.h: Include "ssa.h" to inline ssa_name(). --- gcc/gimple-range-gori.cc | 8 ++++---- gcc/gimple-range-gori.h | 14 ++++++++++---- gcc/gimple-range.h | 1 + 3 files changed, 15 insertions(+), 8 deletions(-) diff --git a/gcc/gimple-range-gori.cc b/gcc/gimple-range-gori.cc index d77e1f51ac2..5bba77c7b7b 100644 --- a/gcc/gimple-range-gori.cc +++ b/gcc/gimple-range-gori.cc @@ -182,9 +182,9 @@ range_def_chain::register_dependency (tree name, tree dep, basic_block bb) // Set the direct dependency cache entries. if (!src.ssa1) - src.ssa1 = dep; - else if (!src.ssa2 && src.ssa1 != dep) - src.ssa2 = dep; + src.ssa1 = SSA_NAME_VERSION (dep); + else if (!src.ssa2 && src.ssa1 != SSA_NAME_VERSION (dep)) + src.ssa2 = SSA_NAME_VERSION (dep); // Don't calculate imports or export/dep chains if BB is not provided. // This is usually the case for when the temporal cache wants the direct @@ -1316,7 +1316,7 @@ gori_compute::may_recompute_p (tree name, basic_block bb, int depth) // If the first dependency is not set, there is no recomputation. // Dependencies reflect original IL, not current state. Check if the // SSA_NAME is still valid as well. - if (!dep1 || SSA_NAME_IN_FREE_LIST (dep1)) + if (!dep1) return false; // Don't recalculate PHIs or statements with side_effects. diff --git a/gcc/gimple-range-gori.h b/gcc/gimple-range-gori.h index 3ea4b45595b..526edc24b53 100644 --- a/gcc/gimple-range-gori.h +++ b/gcc/gimple-range-gori.h @@ -46,8 +46,8 @@ protected: bitmap_obstack m_bitmaps; private: struct rdc { - tree ssa1; // First direct dependency - tree ssa2; // Second direct dependency + unsigned int ssa1; // First direct dependency + unsigned int ssa2; // Second direct dependency bitmap bm; // All dependencies bitmap m_import; }; @@ -66,7 +66,10 @@ range_def_chain::depend1 (tree name) const unsigned v = SSA_NAME_VERSION (name); if (v >= m_def_chain.length ()) return NULL_TREE; - return m_def_chain[v].ssa1; + unsigned v1 = m_def_chain[v].ssa1; + if (!v1) + return NULL_TREE; + return ssa_name (v1); } // Return the second direct dependency for NAME, if there is one. @@ -77,7 +80,10 @@ range_def_chain::depend2 (tree name) const unsigned v = SSA_NAME_VERSION (name); if (v >= m_def_chain.length ()) return NULL_TREE; - return m_def_chain[v].ssa2; + unsigned v2 = m_def_chain[v].ssa2; + if (!v2) + return NULL_TREE; + return ssa_name (v2); } // GORI_MAP is used to accumulate what SSA names in a block can diff --git a/gcc/gimple-range.h b/gcc/gimple-range.h index 7ed4d3870b8..b8ddca59d2d 100644 --- a/gcc/gimple-range.h +++ b/gcc/gimple-range.h @@ -22,6 +22,7 @@ along with GCC; see the file COPYING3. If not see #ifndef GCC_GIMPLE_RANGE_H #define GCC_GIMPLE_RANGE_H +#include "ssa.h" #include "range.h" #include "value-query.h" #include "gimple-range-op.h" -- 2.39.2