From patchwork Mon Jun 10 19:24:04 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Andrew MacLeod X-Patchwork-Id: 1945997 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@legolas.ozlabs.org Authentication-Results: legolas.ozlabs.org; dkim=pass (1024-bit key; unprotected) header.d=redhat.com header.i=@redhat.com header.a=rsa-sha256 header.s=mimecast20190719 header.b=ORuhWuYK; dkim-atps=neutral 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=server2.sourceware.org; envelope-from=gcc-patches-bounces+incoming=patchwork.ozlabs.org@gcc.gnu.org; receiver=patchwork.ozlabs.org) Received: from server2.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 (secp384r1) server-digest SHA384) (No client certificate requested) by legolas.ozlabs.org (Postfix) with ESMTPS id 4VyhZh4wncz20Py for ; Tue, 11 Jun 2024 05:24:44 +1000 (AEST) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id C03D13858CDB for ; Mon, 10 Jun 2024 19:24:38 +0000 (GMT) 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.129.124]) by sourceware.org (Postfix) with ESMTPS id D88FB3858C50 for ; Mon, 10 Jun 2024 19:24:10 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org D88FB3858C50 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=redhat.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=redhat.com ARC-Filter: OpenARC Filter v1.0.0 sourceware.org D88FB3858C50 Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=170.10.129.124 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1718047453; cv=none; b=pFztBpWyoXTdy0sfyp/z3YnC4oHrwvs7uqpaV1HTZYqtXblw+JbjJpolSoMUvDZaL9T74mSNXhJv1MAY75+lUGpPu6dM9R7z/IEsBuX56ayxWpmbKCzcECSyWcdw0lx00qrCN80y/rvBkcIkZhsROuHmVqCaG1a5wU5iTsgNTnE= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1718047453; c=relaxed/simple; bh=DSXUbM7XHn4f25qh0WoRl+Cch3jN+SCmCBITycr14+4=; h=DKIM-Signature:Message-ID:Date:MIME-Version:To:From:Subject; b=GaCIBQkr424D0No2O2pghglYNndsLXAA3QbuFMeIxzq7izsAV21U0Pd+jue5/IVYcJ8atVVjGeeu/uED0SmB7nDqJGESC1Moe58gFpwObm7V8bYHHzyhjnnTLXPY147esNtls4OAn0Fz2vwJKiEWUQxIolNklHeKTVzBQ+7zVKI= ARC-Authentication-Results: i=1; server2.sourceware.org DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1718047450; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type; bh=xc93TlCpOAZDFnULP/3FRVLtXGtOBhVUbOb0rG3JLp4=; b=ORuhWuYKLAz1HjSKewnK34sA9CMzlKC2wWeXjOYIVBBd/o0KiGQ78t6HAV6x93mNzai0jB A0a8bubvFotKuzkoLhkoy+cd0DOVj2wRO6xuQTpVbTOS5baAV5MzcaphMMqZru+pHFEz+3 CCvHxhVdIhm+s0evh9xS8/N0nyoU5OA= Received: from mail-yw1-f200.google.com (mail-yw1-f200.google.com [209.85.128.200]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-313-TeCJmNtqP-i-vayhyPN81Q-1; Mon, 10 Jun 2024 15:24:08 -0400 X-MC-Unique: TeCJmNtqP-i-vayhyPN81Q-1 Received: by mail-yw1-f200.google.com with SMTP id 00721157ae682-62a0827391aso92153457b3.1 for ; Mon, 10 Jun 2024 12:24:08 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1718047447; x=1718652247; h=subject:from:cc:to:content-language:user-agent:mime-version:date :message-id:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=P9isxdFKLBpJWrKMusJF/Bzzpi450CubPmOw+HSWK10=; b=WRuST5PZC3AoDG9jmJf0VTkzlgAe1Wb6+QYLQIw8eId7G6753PeTWbSriwKJuJvlQ7 C5w90d0Ew2pZ5W9b2wJDfASxuc5314RRrxK1mopweLSuk492v9nmUYEsvW00X87acSeD sFJ02tBbtSje43FXORDp9ETObhe+Gt0V2ZVj0X3coGoopvpc1Sq+kHgKeyR+kbfSc2EV rgnXyCdesfR0zzbEQNUOt7nXZ4Hw5hlNlFZXLih5SaeouZYsLGjujq6LD2ZyZZ3vBCHm WqS9adJYw52NTSriTEbW74Wwwt4ngBS+ayhlaPPRn5W1IU/07xaJN0MiNRhJT+88JC2h winA== X-Gm-Message-State: AOJu0YzLwYrYq785eTNKK7fEUetIDlzIKlaFjGj1n+ZmKoWsHk5a+QT5 SvL/QDo9Cte2GBPWuPFRjhgEg5u9plvqjEJ0emGycAR4Px1Hq3c620ErDOEwLtKzFqYOZ4Pq9i2 OhPn2o6nhGWsmeKFtk0Iow+8e6+bukXfmWlR9lAquVRZ1rHPxttZIaK72Zu+Tbe+ttrmPqVdq8J DwMDhvdZruJ/YwPOOyx42oXurdNm961zC6Nr9SOiY= X-Received: by 2002:a05:690c:d91:b0:61a:cde6:6542 with SMTP id 00721157ae682-62d02253a11mr58803907b3.16.1718047447566; Mon, 10 Jun 2024 12:24:07 -0700 (PDT) X-Google-Smtp-Source: AGHT+IGrNQmi/zmMhLUdtgWGHglMwm8S6GOtAK1Nz49zpdB3/YDdfPRHR4GI4+kwvbf90mxVkQaOCQ== X-Received: by 2002:a05:690c:d91:b0:61a:cde6:6542 with SMTP id 00721157ae682-62d02253a11mr58803707b3.16.1718047447058; Mon, 10 Jun 2024 12:24:07 -0700 (PDT) Received: from [192.168.0.174] ([104.219.120.74]) by smtp.gmail.com with ESMTPSA id 6a1803df08f44-6b04fa1a628sm48764666d6.136.2024.06.10.12.24.05 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Mon, 10 Jun 2024 12:24:06 -0700 (PDT) Message-ID: <7972a25c-e7b2-4cc4-a6f0-e16719f452e6@redhat.com> Date: Mon, 10 Jun 2024 15:24:04 -0400 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird To: gcc-patches Cc: "hernandez, aldy" From: Andrew MacLeod Subject: [PATCH] Move array_bounds warnings into it's own pass. X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Language: en-US X-Spam-Status: No, score=-12.4 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_H4, RCVD_IN_MSPIKE_WL, 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.30 Precedence: list List-Id: Gcc-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: gcc-patches-bounces+incoming=patchwork.ozlabs.org@gcc.gnu.org The array bounds warning pass was originally attached to the VRP pass because it wanted to leverage the context sensitive ranges available there. With ranger, we can make it a pass of its own for very little cost.  This patch does that. It removes the array_bounds_checker from VRP and makes it a solo pass that runs immediately after VRP1. The original version had VRP add any un-executable edge flags it found, but I could not find a case where after VRP cleans up the CFG the new pass needed that.  I also did not find a case where activating SCEV again for the warning pass made a difference after VRP had run.  So this patch does neither of those things. It simple enough to later add SCEV and loop analysis again if it turns out to be important. My primary motivation for removing it was to remove the second DOM walk the checker performs which depends on on-demand ranges pre-cached by  ranger.   This prevented VRP from choosing an alternative VRP solution when basic block counts are very high (PR  114855).  I also know Siddesh want to experiment with moving the pass later in the pipeline as well, which will make that task much simpler as a secondary rationale. I didn't want to mess with the internal code much. For a multitude of reasons.  I did change it so that it always uses the current range_query object instead of passing one in to the constructor.  And then I cleaned up the VRP code ot no longer take a flag on whether to invoke the warning code or not. The final bit is the pass is set to only run when flag_tree_vrp is on..  I did this primarily to preserve existing functionality, and some tests depended on it.  ie, would turn on -warray-bounds and disables tree-vrp pass (which means the  bounds checker doesnt run) ... which changes the expected warnings from the strlen pass.    I'm not going there.    there are  also tests which run at -O1 and -Wall that do not expect the bounds checker to run either.   So this dependence on the vrp flag is documented in the code an preserves existing behavior. Does anyone have any issues with any of this? Bootstraps on x86_64-pc-linux-gnu with no regressions. Andrew From aa0259784b0c0884956a627b78c0f5025d76c931 Mon Sep 17 00:00:00 2001 From: Andrew MacLeod Date: Wed, 5 Jun 2024 15:12:27 -0400 Subject: [PATCH 2/2] Move array_bounds warnings into it's own pass. Array bounds checking is currently tied to VRP. This causes issues with using laternate VRP algorithms as well as experimenting with moving the location of the warnings later. This moves it to its own pass and cleans up the vrp_pass object. * gimple-array-bounds.cc (array_bounds_checker::array_bounds_checker): Always use current range_query. (pass_data_array_bounds): New. (pass_array_bounds): New. (make_pass_array_bounds): New. * gimple-array-bounds.h (array_bounds_checker): Adjust prototype. * timevar.def (TV_TREE_ARRAY_BOUNDS): New timevar. * tree-pass.h (make_pass_array_bounds): Add prototype. * tree-vrp.cc (execute_ranger_vrp): Remove warning param and do not invoke array bounds warning pass. (pass_vrp::pass_vrp): Adjust params. (pass_vrp::close): Adjust parameters. (pass_vrp::warn_array_bounds_p): Remove. (make_pass_vrp): Remove warning param. (make_pass_early_vrp): Remove warning param. (make_pass_fast_vrp): Remove warning param. --- gcc/gimple-array-bounds.cc | 63 +++++++++++++++++++++++++++++++++----- gcc/gimple-array-bounds.h | 2 +- gcc/passes.def | 1 + gcc/timevar.def | 1 + gcc/tree-pass.h | 1 + gcc/tree-vrp.cc | 40 +++++------------------- 6 files changed, 67 insertions(+), 41 deletions(-) diff --git a/gcc/gimple-array-bounds.cc b/gcc/gimple-array-bounds.cc index 008071cd546..1d14abf3ca1 100644 --- a/gcc/gimple-array-bounds.cc +++ b/gcc/gimple-array-bounds.cc @@ -38,10 +38,12 @@ along with GCC; see the file COPYING3. If not see #include "domwalk.h" #include "tree-cfg.h" #include "attribs.h" +#include "tree-pass.h" +#include "gimple-range.h" -array_bounds_checker::array_bounds_checker (struct function *func, - range_query *qry) - : fun (func), m_ptr_qry (qry) +// Always use the current range query for the bounds checker. +array_bounds_checker::array_bounds_checker (struct function *func) + : fun (func), m_ptr_qry (get_range_query (func)) { /* No-op. */ } @@ -838,11 +840,7 @@ class check_array_bounds_dom_walker : public dom_walker { public: check_array_bounds_dom_walker (array_bounds_checker *checker) - : dom_walker (CDI_DOMINATORS, - /* Discover non-executable edges, preserving EDGE_EXECUTABLE - flags, so that we can merge in information on - non-executable edges from vrp_folder . */ - REACHABLE_BLOCKS_PRESERVING_FLAGS), + : dom_walker (CDI_DOMINATORS, REACHABLE_BLOCKS), checker (checker) { } ~check_array_bounds_dom_walker () {} @@ -888,3 +886,52 @@ array_bounds_checker::check () check_array_bounds_dom_walker w (this); w.walk (ENTRY_BLOCK_PTR_FOR_FN (fun)); } + +const pass_data pass_data_array_bounds = +{ + GIMPLE_PASS, /* type */ + "bounds", /* name */ + OPTGROUP_NONE, /* optinfo_flags */ + TV_TREE_ARRAY_BOUNDS, /* tv_id */ + PROP_ssa, /* properties_required */ + 0, /* properties_provided */ + 0, /* properties_destroyed */ + 0, /* todo_flags_start */ + ( 0 ), /* No TODOs */ +}; + +class pass_array_bounds : public gimple_opt_pass +{ +public: + pass_array_bounds (gcc::context *ctxt, const pass_data &data_) + : gimple_opt_pass (data_, ctxt), data (data_) + { } + + /* opt_pass methods: */ + opt_pass * clone () final override + { return new pass_array_bounds (m_ctxt, data); } + bool gate (function *) final override + { + // Gate on the VRP pass to preserve previous behavior. + return flag_tree_vrp && (warn_array_bounds || warn_strict_flex_arrays); + } + unsigned int execute (function *fun) final override + { + calculate_dominance_info (CDI_DOMINATORS); + // Enable ranger as the current range query. + enable_ranger (fun, false); + array_bounds_checker array_checker (fun); + array_checker.check (); + disable_ranger (fun); + return 0; + } + + private: + const pass_data &data; +}; // class pass_array_bounds + +gimple_opt_pass * +make_pass_array_bounds (gcc::context *ctxt) +{ + return new pass_array_bounds (ctxt, pass_data_array_bounds); +} diff --git a/gcc/gimple-array-bounds.h b/gcc/gimple-array-bounds.h index 3e077d0178f..aa7ca8e9730 100644 --- a/gcc/gimple-array-bounds.h +++ b/gcc/gimple-array-bounds.h @@ -27,7 +27,7 @@ class array_bounds_checker friend class check_array_bounds_dom_walker; public: - array_bounds_checker (struct function *, range_query *); + array_bounds_checker (struct function *); void check (); private: diff --git a/gcc/passes.def b/gcc/passes.def index 1cbbd413097..041229e47a6 100644 --- a/gcc/passes.def +++ b/gcc/passes.def @@ -226,6 +226,7 @@ along with GCC; see the file COPYING3. If not see NEXT_PASS (pass_merge_phi); NEXT_PASS (pass_thread_jumps_full, /*first=*/true); NEXT_PASS (pass_vrp, false /* final_p*/); + NEXT_PASS (pass_array_bounds); NEXT_PASS (pass_dse); NEXT_PASS (pass_dce); /* pass_stdarg is always run and at this point we execute diff --git a/gcc/timevar.def b/gcc/timevar.def index 8e2168e0817..6fc36859138 100644 --- a/gcc/timevar.def +++ b/gcc/timevar.def @@ -161,6 +161,7 @@ DEFTIMEVAR (TV_TREE_VRP , "tree VRP") DEFTIMEVAR (TV_TREE_VRP_THREADER , "tree VRP threader") DEFTIMEVAR (TV_TREE_EARLY_VRP , "tree Early VRP") DEFTIMEVAR (TV_TREE_FAST_VRP , "tree Fast VRP") +DEFTIMEVAR (TV_TREE_ARRAY_BOUNDS , "warn array bounds") DEFTIMEVAR (TV_TREE_COPY_PROP , "tree copy propagation") DEFTIMEVAR (TV_FIND_REFERENCED_VARS , "tree find ref. vars") DEFTIMEVAR (TV_TREE_PTA , "tree PTA") diff --git a/gcc/tree-pass.h b/gcc/tree-pass.h index 29267589eeb..edebb2be245 100644 --- a/gcc/tree-pass.h +++ b/gcc/tree-pass.h @@ -470,6 +470,7 @@ extern gimple_opt_pass *make_pass_fre (gcc::context *ctxt); extern gimple_opt_pass *make_pass_check_data_deps (gcc::context *ctxt); extern gimple_opt_pass *make_pass_copy_prop (gcc::context *ctxt); extern gimple_opt_pass *make_pass_isolate_erroneous_paths (gcc::context *ctxt); +extern gimple_opt_pass *make_pass_array_bounds (gcc::context *ctxt); extern gimple_opt_pass *make_pass_early_vrp (gcc::context *ctxt); extern gimple_opt_pass *make_pass_fast_vrp (gcc::context *ctxt); extern gimple_opt_pass *make_pass_vrp (gcc::context *ctxt); diff --git a/gcc/tree-vrp.cc b/gcc/tree-vrp.cc index 1c7b451d8fb..1f6b578f253 100644 --- a/gcc/tree-vrp.cc +++ b/gcc/tree-vrp.cc @@ -1095,8 +1095,7 @@ private: from anywhere to perform a VRP pass, including from EVRP. */ unsigned int -execute_ranger_vrp (struct function *fun, bool warn_array_bounds_p, - bool final_p) +execute_ranger_vrp (struct function *fun, bool final_p) { loop_optimizer_init (LOOPS_NORMAL | LOOPS_HAVE_RECORDED_EXITS); rewrite_into_loop_closed_ssa (NULL, TODO_update_ssa); @@ -1113,27 +1112,6 @@ execute_ranger_vrp (struct function *fun, bool warn_array_bounds_p, if (dump_file && (dump_flags & TDF_DETAILS)) ranger->dump (dump_file); - if ((warn_array_bounds || warn_strict_flex_arrays) && warn_array_bounds_p) - { - // Set all edges as executable, except those ranger says aren't. - int non_exec_flag = ranger->non_executable_edge_flag; - basic_block bb; - FOR_ALL_BB_FN (bb, fun) - { - edge_iterator ei; - edge e; - FOR_EACH_EDGE (e, ei, bb->succs) - if (e->flags & non_exec_flag) - e->flags &= ~EDGE_EXECUTABLE; - else - e->flags |= EDGE_EXECUTABLE; - } - scev_reset (); - array_bounds_checker array_checker (fun, ranger); - array_checker.check (); - } - - if (Value_Range::supports_type_p (TREE_TYPE (TREE_TYPE (current_function_decl))) && flag_ipa_vrp @@ -1330,14 +1308,13 @@ const pass_data pass_data_fast_vrp = class pass_vrp : public gimple_opt_pass { public: - pass_vrp (gcc::context *ctxt, const pass_data &data_, bool warn_p) - : gimple_opt_pass (data_, ctxt), data (data_), - warn_array_bounds_p (warn_p), final_p (false) + pass_vrp (gcc::context *ctxt, const pass_data &data_) + : gimple_opt_pass (data_, ctxt), data (data_), final_p (false) { } /* opt_pass methods: */ opt_pass * clone () final override - { return new pass_vrp (m_ctxt, data, false); } + { return new pass_vrp (m_ctxt, data); } void set_pass_param (unsigned int n, bool param) final override { gcc_assert (n == 0); @@ -1350,12 +1327,11 @@ public: if (&data == &pass_data_fast_vrp) return execute_fast_vrp (fun); - return execute_ranger_vrp (fun, warn_array_bounds_p, final_p); + return execute_ranger_vrp (fun, final_p); } private: const pass_data &data; - bool warn_array_bounds_p; bool final_p; }; // class pass_vrp @@ -1426,19 +1402,19 @@ public: gimple_opt_pass * make_pass_vrp (gcc::context *ctxt) { - return new pass_vrp (ctxt, pass_data_vrp, true); + return new pass_vrp (ctxt, pass_data_vrp); } gimple_opt_pass * make_pass_early_vrp (gcc::context *ctxt) { - return new pass_vrp (ctxt, pass_data_early_vrp, false); + return new pass_vrp (ctxt, pass_data_early_vrp); } gimple_opt_pass * make_pass_fast_vrp (gcc::context *ctxt) { - return new pass_vrp (ctxt, pass_data_fast_vrp, false); + return new pass_vrp (ctxt, pass_data_fast_vrp); } gimple_opt_pass * -- 2.45.0