* [PATCH] simplify get_range_strlen interface
@ 2021-11-15 22:05 Martin Sebor
2021-11-15 22:42 ` Jeff Law
2021-11-16 16:30 ` Martin Sebor
0 siblings, 2 replies; 3+ messages in thread
From: Martin Sebor @ 2021-11-15 22:05 UTC (permalink / raw)
To: gcc-patches
[-- Attachment #1: Type: text/plain, Size: 508 bytes --]
The deeply nested PHI handling in get_range_strlen_dynamic makes
the code bigger and harder to follow than it would be if done in
its own function. The attached patch does that.
In addition, the get_range_strlen family of functions use a bitmap
to avoid infinite recursion. Rather than dynamically allocating
and freeing it on demand the attached patch simplifies the code
by using an instance of auto_bitmap. This avoids the risk of
neglecting to deallocate the bitmap.
Tested on x86_64-linux.
Martin
[-- Attachment #2: gcc-get_range_strlen_dynamic.diff --]
[-- Type: text/x-patch, Size: 10894 bytes --]
Slightly simplify get_range_strlen interface.
gcc/ChangeLog:
* gimple-fold.c (get_range_strlen): Take bitmap as an argument rather
than a pointer to it.
(get_range_strlen_tree): Same. Remove bitmap allocation. Use
an auto_bitmap.
(get_maxval_strlen): Use an auto_bitmap.
* tree-ssa-strlen.c (get_range_strlen_dynamic): Factor out PHI
handling...
(get_range_strlen_phi): ...into this function.
(printf_strlen_execute): Dump pointer query cache contents when
details are requisted.
diff --git a/gcc/gimple-fold.c b/gcc/gimple-fold.c
index 6e25a7c05db..87c211c5e3f 100644
--- a/gcc/gimple-fold.c
+++ b/gcc/gimple-fold.c
@@ -86,7 +86,7 @@ enum strlen_range_kind {
};
static bool
-get_range_strlen (tree, bitmap *, strlen_range_kind, c_strlen_data *, unsigned);
+get_range_strlen (tree, bitmap, strlen_range_kind, c_strlen_data *, unsigned);
/* Return true when DECL can be referenced from current unit.
FROM_DECL (if non-null) specify constructor of variable DECL was taken from.
@@ -1525,7 +1525,7 @@ gimple_fold_builtin_memset (gimple_stmt_iterator *gsi, tree c, tree len)
/* Helper of get_range_strlen for ARG that is not an SSA_NAME. */
static bool
-get_range_strlen_tree (tree arg, bitmap *visited, strlen_range_kind rkind,
+get_range_strlen_tree (tree arg, bitmap visited, strlen_range_kind rkind,
c_strlen_data *pdata, unsigned eltsize)
{
gcc_assert (TREE_CODE (arg) != SSA_NAME);
@@ -1849,7 +1849,7 @@ get_range_strlen_tree (tree arg, bitmap *visited, strlen_range_kind rkind,
Return true if *PDATA was successfully populated and false otherwise. */
static bool
-get_range_strlen (tree arg, bitmap *visited,
+get_range_strlen (tree arg, bitmap visited,
strlen_range_kind rkind,
c_strlen_data *pdata, unsigned eltsize)
{
@@ -1863,9 +1863,7 @@ get_range_strlen (tree arg, bitmap *visited,
return false;
/* If we were already here, break the infinite cycle. */
- if (!*visited)
- *visited = BITMAP_ALLOC (NULL);
- if (!bitmap_set_bit (*visited, SSA_NAME_VERSION (arg)))
+ if (!bitmap_set_bit (visited, SSA_NAME_VERSION (arg)))
return true;
tree var = arg;
@@ -1962,10 +1960,10 @@ get_range_strlen (tree arg, bitmap *visited,
bool
get_range_strlen (tree arg, c_strlen_data *pdata, unsigned eltsize)
{
- bitmap visited = NULL;
+ auto_bitmap visited;
tree maxbound = pdata->maxbound;
- if (!get_range_strlen (arg, &visited, SRK_LENRANGE, pdata, eltsize))
+ if (!get_range_strlen (arg, visited, SRK_LENRANGE, pdata, eltsize))
{
/* On failure extend the length range to an impossible maximum
(a valid MAXLEN must be less than PTRDIFF_MAX - 1). Other
@@ -1981,9 +1979,6 @@ get_range_strlen (tree arg, c_strlen_data *pdata, unsigned eltsize)
if (maxbound && pdata->maxbound == maxbound)
pdata->maxbound = build_all_ones_cst (size_type_node);
- if (visited)
- BITMAP_FREE (visited);
-
return !integer_all_onesp (pdata->maxlen);
}
@@ -2005,19 +2000,16 @@ get_maxval_strlen (tree arg, strlen_range_kind rkind, tree *nonstr = NULL)
/* ARG must have an integral type when RKIND says so. */
gcc_assert (rkind != SRK_INT_VALUE || INTEGRAL_TYPE_P (TREE_TYPE (arg)));
- bitmap visited = NULL;
+ auto_bitmap visited;
/* Reset DATA.MAXLEN if the call fails or when DATA.MAXLEN
is unbounded. */
c_strlen_data lendata = { };
- if (!get_range_strlen (arg, &visited, rkind, &lendata, /* eltsize = */1))
+ if (!get_range_strlen (arg, visited, rkind, &lendata, /* eltsize = */1))
lendata.maxlen = NULL_TREE;
else if (lendata.maxlen && integer_all_onesp (lendata.maxlen))
lendata.maxlen = NULL_TREE;
- if (visited)
- BITMAP_FREE (visited);
-
if (nonstr)
{
/* For callers prepared to handle unterminated arrays set
diff --git a/gcc/tree-ssa-strlen.c b/gcc/tree-ssa-strlen.c
index c0ec7d20a60..536f796f82b 100644
--- a/gcc/tree-ssa-strlen.c
+++ b/gcc/tree-ssa-strlen.c
@@ -193,6 +193,8 @@ struct laststmt_struct
} laststmt;
static int get_stridx_plus_constant (strinfo *, unsigned HOST_WIDE_INT, tree);
+static bool get_range_strlen_dynamic (tree, gimple *s, c_strlen_data *,
+ bitmap, range_query *, unsigned *);
/* Sets MINMAX to either the constant value or the range VAL is in
and returns either the constant value or VAL on success or null
@@ -1087,6 +1089,76 @@ dump_strlen_info (FILE *fp, gimple *stmt, range_query *rvals)
}
}
+/* Helper of get_range_strlen_dynamic(). See below. */
+
+static bool
+get_range_strlen_phi (tree src, gphi *phi,
+ c_strlen_data *pdata, bitmap visited,
+ range_query *rvals, unsigned *pssa_def_max)
+{
+ if (!bitmap_set_bit (visited, SSA_NAME_VERSION (src)))
+ return true;
+
+ if (*pssa_def_max == 0)
+ return false;
+
+ --*pssa_def_max;
+
+ /* Iterate over the PHI arguments and determine the minimum and maximum
+ length/size of each and incorporate them into the overall result. */
+ for (unsigned i = 0; i != gimple_phi_num_args (phi); ++i)
+ {
+ tree arg = gimple_phi_arg_def (phi, i);
+ if (arg == gimple_phi_result (phi))
+ continue;
+
+ c_strlen_data argdata = { };
+ if (!get_range_strlen_dynamic (arg, phi, &argdata, visited, rvals,
+ pssa_def_max))
+ {
+ pdata->maxlen = build_all_ones_cst (size_type_node);
+ continue;
+ }
+
+ /* Set the DECL of an unterminated array this argument refers to
+ if one hasn't been found yet. */
+ if (!pdata->decl && argdata.decl)
+ pdata->decl = argdata.decl;
+
+ if (!argdata.minlen
+ || (integer_zerop (argdata.minlen)
+ && (!argdata.maxbound
+ || integer_all_onesp (argdata.maxbound))
+ && integer_all_onesp (argdata.maxlen)))
+ {
+ /* Set the upper bound of the length to unbounded. */
+ pdata->maxlen = build_all_ones_cst (size_type_node);
+ continue;
+ }
+
+ /* Adjust the minimum and maximum length determined so far and
+ the upper bound on the array size. */
+ if (!pdata->minlen
+ || tree_int_cst_lt (argdata.minlen, pdata->minlen))
+ pdata->minlen = argdata.minlen;
+
+ if (!pdata->maxlen
+ || (argdata.maxlen
+ && TREE_CODE (argdata.maxlen) == INTEGER_CST
+ && tree_int_cst_lt (pdata->maxlen, argdata.maxlen)))
+ pdata->maxlen = argdata.maxlen;
+
+ if (!pdata->maxbound
+ || TREE_CODE (pdata->maxbound) != INTEGER_CST
+ || (argdata.maxbound
+ && tree_int_cst_lt (pdata->maxbound, argdata.maxbound)
+ && !integer_all_onesp (argdata.maxbound)))
+ pdata->maxbound = argdata.maxbound;
+ }
+
+ return true;
+}
+
/* Attempt to determine the length of the string SRC. On success, store
the length in *PDATA and return true. Otherwise, return false.
VISITED is a bitmap of visited PHI nodes. RVALS points to the valuation
@@ -1095,7 +1167,7 @@ dump_strlen_info (FILE *fp, gimple *stmt, range_query *rvals)
static bool
get_range_strlen_dynamic (tree src, gimple *stmt,
- c_strlen_data *pdata, bitmap *visited,
+ c_strlen_data *pdata, bitmap visited,
range_query *rvals, unsigned *pssa_def_max)
{
int idx = get_stridx (src, stmt);
@@ -1104,72 +1176,9 @@ get_range_strlen_dynamic (tree src, gimple *stmt,
if (TREE_CODE (src) == SSA_NAME)
{
gimple *def_stmt = SSA_NAME_DEF_STMT (src);
- if (gimple_code (def_stmt) == GIMPLE_PHI)
- {
- if (!*visited)
- *visited = BITMAP_ALLOC (NULL);
-
- if (!bitmap_set_bit (*visited, SSA_NAME_VERSION (src)))
- return true;
-
- if (*pssa_def_max == 0)
- return false;
-
- --*pssa_def_max;
-
- /* Iterate over the PHI arguments and determine the minimum
- and maximum length/size of each and incorporate them into
- the overall result. */
- gphi *phi = as_a <gphi *> (def_stmt);
- for (unsigned i = 0; i != gimple_phi_num_args (phi); ++i)
- {
- tree arg = gimple_phi_arg_def (phi, i);
- if (arg == gimple_phi_result (def_stmt))
- continue;
-
- c_strlen_data argdata = { };
- if (get_range_strlen_dynamic (arg, phi, &argdata, visited,
- rvals, pssa_def_max))
- {
- /* Set the DECL of an unterminated array this argument
- refers to if one hasn't been found yet. */
- if (!pdata->decl && argdata.decl)
- pdata->decl = argdata.decl;
-
- if (!argdata.minlen
- || (integer_zerop (argdata.minlen)
- && (!argdata.maxbound
- || integer_all_onesp (argdata.maxbound))
- && integer_all_onesp (argdata.maxlen)))
- {
- /* Set the upper bound of the length to unbounded. */
- pdata->maxlen = build_all_ones_cst (size_type_node);
- continue;
- }
-
- /* Adjust the minimum and maximum length determined
- so far and the upper bound on the array size. */
- if (!pdata->minlen
- || tree_int_cst_lt (argdata.minlen, pdata->minlen))
- pdata->minlen = argdata.minlen;
- if (!pdata->maxlen
- || (argdata.maxlen
- && tree_int_cst_lt (pdata->maxlen, argdata.maxlen)))
- pdata->maxlen = argdata.maxlen;
- if (!pdata->maxbound
- || TREE_CODE (pdata->maxbound) != INTEGER_CST
- || (argdata.maxbound
- && tree_int_cst_lt (pdata->maxbound,
- argdata.maxbound)
- && !integer_all_onesp (argdata.maxbound)))
- pdata->maxbound = argdata.maxbound;
- }
- else
- pdata->maxlen = build_all_ones_cst (size_type_node);
- }
-
- return true;
- }
+ if (gphi *phi = dyn_cast<gphi *>(def_stmt))
+ return get_range_strlen_phi (src, phi, pdata, visited, rvals,
+ pssa_def_max);
}
/* Return success regardless of the result and handle *PDATA
@@ -1286,11 +1295,11 @@ void
get_range_strlen_dynamic (tree src, gimple *stmt, c_strlen_data *pdata,
range_query *rvals)
{
- bitmap visited = NULL;
+ auto_bitmap visited;
tree maxbound = pdata->maxbound;
unsigned limit = param_ssa_name_def_chain_limit;
- if (!get_range_strlen_dynamic (src, stmt, pdata, &visited, rvals, &limit))
+ if (!get_range_strlen_dynamic (src, stmt, pdata, visited, rvals, &limit))
{
/* On failure extend the length range to an impossible maximum
(a valid MAXLEN must be less than PTRDIFF_MAX - 1). Other
@@ -1305,9 +1314,6 @@ get_range_strlen_dynamic (tree src, gimple *stmt, c_strlen_data *pdata,
MAXBOUND to SIZE_MAX. Otherwise leave it null (if it is null). */
if (maxbound && pdata->maxbound == maxbound)
pdata->maxbound = build_all_ones_cst (size_type_node);
-
- if (visited)
- BITMAP_FREE (visited);
}
/* Invalidate string length information for strings whose length might
@@ -5831,7 +5837,7 @@ printf_strlen_execute (function *fun, bool warn_only)
walker.walk (ENTRY_BLOCK_PTR_FOR_FN (fun));
if (dump_file && (dump_flags & TDF_DETAILS))
- walker.ptr_qry.dump (dump_file);
+ walker.ptr_qry.dump (dump_file, true);
ssa_ver_to_stridx.release ();
strinfo_pool.release ();
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] simplify get_range_strlen interface
2021-11-15 22:05 [PATCH] simplify get_range_strlen interface Martin Sebor
@ 2021-11-15 22:42 ` Jeff Law
2021-11-16 16:30 ` Martin Sebor
1 sibling, 0 replies; 3+ messages in thread
From: Jeff Law @ 2021-11-15 22:42 UTC (permalink / raw)
To: Martin Sebor, gcc-patches
On 11/15/2021 3:05 PM, Martin Sebor via Gcc-patches wrote:
> The deeply nested PHI handling in get_range_strlen_dynamic makes
> the code bigger and harder to follow than it would be if done in
> its own function. The attached patch does that.
>
> In addition, the get_range_strlen family of functions use a bitmap
> to avoid infinite recursion. Rather than dynamically allocating
> and freeing it on demand the attached patch simplifies the code
> by using an instance of auto_bitmap. This avoids the risk of
> neglecting to deallocate the bitmap.
>
> Tested on x86_64-linux.
>
> Martin
>
> gcc-get_range_strlen_dynamic.diff
>
> Slightly simplify get_range_strlen interface.
>
> gcc/ChangeLog:
>
> * gimple-fold.c (get_range_strlen): Take bitmap as an argument rather
> than a pointer to it.
> (get_range_strlen_tree): Same. Remove bitmap allocation. Use
> an auto_bitmap.
> (get_maxval_strlen): Use an auto_bitmap.
> * tree-ssa-strlen.c (get_range_strlen_dynamic): Factor out PHI
> handling...
> (get_range_strlen_phi): ...into this function.
> (printf_strlen_execute): Dump pointer query cache contents when
> details are requisted.
No really a bugfix, but I'll go ahead and ACK for the trunk. Obviously
the bar is going to be going up now that we're in stage3 :-)
jeff
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] simplify get_range_strlen interface
2021-11-15 22:05 [PATCH] simplify get_range_strlen interface Martin Sebor
2021-11-15 22:42 ` Jeff Law
@ 2021-11-16 16:30 ` Martin Sebor
1 sibling, 0 replies; 3+ messages in thread
From: Martin Sebor @ 2021-11-16 16:30 UTC (permalink / raw)
To: gcc-patches
On 11/15/21 3:05 PM, Martin Sebor wrote:
> The deeply nested PHI handling in get_range_strlen_dynamic makes
> the code bigger and harder to follow than it would be if done in
> its own function. The attached patch does that.
>
> In addition, the get_range_strlen family of functions use a bitmap
> to avoid infinite recursion. Rather than dynamically allocating
> and freeing it on demand the attached patch simplifies the code
> by using an instance of auto_bitmap. This avoids the risk of
> neglecting to deallocate the bitmap.
I forgot over the weekend that this change also fixes a bug:
PR 102960.
I have committed the fix in r12-5310 along with a test.
Martin
>
> Tested on x86_64-linux.
>
> Martin
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2021-11-16 16:30 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-11-15 22:05 [PATCH] simplify get_range_strlen interface Martin Sebor
2021-11-15 22:42 ` Jeff Law
2021-11-16 16:30 ` Martin Sebor
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).