* Re: [PATCH 1/3] Hide and refactor IVOPTs strip_offset
@ 2023-06-21 11:43 Richard Biener
0 siblings, 0 replies; 2+ messages in thread
From: Richard Biener @ 2023-06-21 11:43 UTC (permalink / raw)
To: gcc-patches
On Wed, 21 Jun 2023, Richard Biener wrote:
> PR110243 shows strip_offset has some correctness issues, the following
> avoids using it from loop distribution which can use the more correct
> split_constant_offset from data-ref analysis instead. The patch then
> un-exports the function and refactors it to make it obvious the
> actual constant offset is only interesting in address-cases.
>
> The series is split, removing one strip_offset at a time for easier
> bisecting of code generation quality issues.
>
> Bootstrapped and tested on x86_64-unknown-linux-gnu, pushed.
>
> * tree-loop-distribution.cc (classify_builtin_st): Use
> split_constant_offset.
> * tree-ssa-loop-ivopts.h (strip_offset): Remove.
> * tree-ssa-loop-ivopts.cc (strip_offset): Make static.
> Refactor to make the offset operand optional and assert
> we are dealing with addresses if its required.
Just noticed this refactoring is a left-over and I edited it out of
this series, will push after re-bootstrapping.
Richard.
> ---
> gcc/tree-loop-distribution.cc | 9 +++++----
> gcc/tree-ssa-loop-ivopts.cc | 22 ++++++++++++++--------
> gcc/tree-ssa-loop-ivopts.h | 1 -
> 3 files changed, 19 insertions(+), 13 deletions(-)
>
> diff --git a/gcc/tree-loop-distribution.cc b/gcc/tree-loop-distribution.cc
> index 6291f941a21..cf7c197aaf7 100644
> --- a/gcc/tree-loop-distribution.cc
> +++ b/gcc/tree-loop-distribution.cc
> @@ -1756,11 +1756,12 @@ classify_builtin_st (loop_p loop, partition *partition, data_reference_p dr)
> return;
> }
>
> - poly_uint64 base_offset;
> - unsigned HOST_WIDE_INT const_base_offset;
> - tree base_base = strip_offset (base, &base_offset);
> - if (!base_offset.is_constant (&const_base_offset))
> + tree base_offset;
> + tree base_base;
> + split_constant_offset (base, &base_base, &base_offset);
> + if (!cst_and_fits_in_hwi (base_offset))
> return;
> + unsigned HOST_WIDE_INT const_base_offset = int_cst_value (base_offset);
>
> struct builtin_info *builtin;
> builtin = alloc_builtin (dr, NULL, base, NULL_TREE, size);
> diff --git a/gcc/tree-ssa-loop-ivopts.cc b/gcc/tree-ssa-loop-ivopts.cc
> index 6fbd2d59318..7978c80b39e 100644
> --- a/gcc/tree-ssa-loop-ivopts.cc
> +++ b/gcc/tree-ssa-loop-ivopts.cc
> @@ -1175,6 +1175,9 @@ contain_complex_addr_expr (tree expr)
> return res;
> }
>
> +static tree
> +strip_offset (tree expr, poly_uint64_pod *offset = nullptr);
> +
> /* Allocates an induction variable with given initial value BASE and step STEP
> for loop LOOP. NO_OVERFLOW implies the iv doesn't overflow. */
>
> @@ -2942,12 +2945,16 @@ strip_offset_1 (tree expr, bool inside_addr, bool top_compref,
>
> /* Strips constant offsets from EXPR and stores them to OFFSET. */
>
> -tree
> +static tree
> strip_offset (tree expr, poly_uint64_pod *offset)
> {
> poly_int64 off;
> tree core = strip_offset_1 (expr, false, false, &off);
> - *offset = off;
> + if (offset)
> + {
> + gcc_assert (POINTER_TYPE_P (expr));
> + *offset = off;
> + }
> return core;
> }
>
> @@ -3512,7 +3519,6 @@ add_iv_candidate_derived_from_uses (struct ivopts_data *data)
> static void
> add_iv_candidate_for_use (struct ivopts_data *data, struct iv_use *use)
> {
> - poly_uint64 offset;
> tree base;
> struct iv *iv = use->iv;
> tree basetype = TREE_TYPE (iv->base);
> @@ -3563,8 +3569,8 @@ add_iv_candidate_for_use (struct ivopts_data *data, struct iv_use *use)
>
> /* Record common candidate with constant offset stripped in base.
> Like the use itself, we also add candidate directly for it. */
> - base = strip_offset (iv->base, &offset);
> - if (maybe_ne (offset, 0U) || base != iv->base)
> + base = strip_offset (iv->base);
> + if (base != iv->base)
> {
> record_common_cand (data, base, iv->step, use);
> add_candidate (data, base, iv->step, false, use);
> @@ -3582,9 +3588,9 @@ add_iv_candidate_for_use (struct ivopts_data *data, struct iv_use *use)
> step = fold_convert (sizetype, step);
> record_common_cand (data, base, step, use);
> /* Also record common candidate with offset stripped. */
> - base = strip_offset (base, &offset);
> - if (maybe_ne (offset, 0U))
> - record_common_cand (data, base, step, use);
> + tree alt_base = strip_offset (base);
> + if (alt_base != base)
> + record_common_cand (data, alt_base, step, use);
> }
>
> /* At last, add auto-incremental candidates. Make such variables
> diff --git a/gcc/tree-ssa-loop-ivopts.h b/gcc/tree-ssa-loop-ivopts.h
> index 95148616e70..7a53ce47f10 100644
> --- a/gcc/tree-ssa-loop-ivopts.h
> +++ b/gcc/tree-ssa-loop-ivopts.h
> @@ -28,7 +28,6 @@ extern void dump_cand (FILE *, struct iv_cand *);
> extern bool contains_abnormal_ssa_name_p (tree);
> extern class loop *outermost_invariant_loop_for_expr (class loop *, tree);
> extern bool expr_invariant_in_loop_p (class loop *, tree);
> -extern tree strip_offset (tree, poly_uint64_pod *);
> bool may_be_nonaddressable_p (tree expr);
> void tree_ssa_iv_optimize (void);
>
>
--
Richard Biener <rguenther@suse.de>
SUSE Software Solutions Germany GmbH, Frankenstrasse 146, 90461 Nuernberg,
Germany; GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman;
HRB 36809 (AG Nuernberg)
^ permalink raw reply [flat|nested] 2+ messages in thread
* [PATCH 1/3] Hide and refactor IVOPTs strip_offset
@ 2023-06-21 11:30 Richard Biener
0 siblings, 0 replies; 2+ messages in thread
From: Richard Biener @ 2023-06-21 11:30 UTC (permalink / raw)
To: gcc-patches
PR110243 shows strip_offset has some correctness issues, the following
avoids using it from loop distribution which can use the more correct
split_constant_offset from data-ref analysis instead. The patch then
un-exports the function and refactors it to make it obvious the
actual constant offset is only interesting in address-cases.
The series is split, removing one strip_offset at a time for easier
bisecting of code generation quality issues.
Bootstrapped and tested on x86_64-unknown-linux-gnu, pushed.
* tree-loop-distribution.cc (classify_builtin_st): Use
split_constant_offset.
* tree-ssa-loop-ivopts.h (strip_offset): Remove.
* tree-ssa-loop-ivopts.cc (strip_offset): Make static.
Refactor to make the offset operand optional and assert
we are dealing with addresses if its required.
---
gcc/tree-loop-distribution.cc | 9 +++++----
gcc/tree-ssa-loop-ivopts.cc | 22 ++++++++++++++--------
gcc/tree-ssa-loop-ivopts.h | 1 -
3 files changed, 19 insertions(+), 13 deletions(-)
diff --git a/gcc/tree-loop-distribution.cc b/gcc/tree-loop-distribution.cc
index 6291f941a21..cf7c197aaf7 100644
--- a/gcc/tree-loop-distribution.cc
+++ b/gcc/tree-loop-distribution.cc
@@ -1756,11 +1756,12 @@ classify_builtin_st (loop_p loop, partition *partition, data_reference_p dr)
return;
}
- poly_uint64 base_offset;
- unsigned HOST_WIDE_INT const_base_offset;
- tree base_base = strip_offset (base, &base_offset);
- if (!base_offset.is_constant (&const_base_offset))
+ tree base_offset;
+ tree base_base;
+ split_constant_offset (base, &base_base, &base_offset);
+ if (!cst_and_fits_in_hwi (base_offset))
return;
+ unsigned HOST_WIDE_INT const_base_offset = int_cst_value (base_offset);
struct builtin_info *builtin;
builtin = alloc_builtin (dr, NULL, base, NULL_TREE, size);
diff --git a/gcc/tree-ssa-loop-ivopts.cc b/gcc/tree-ssa-loop-ivopts.cc
index 6fbd2d59318..7978c80b39e 100644
--- a/gcc/tree-ssa-loop-ivopts.cc
+++ b/gcc/tree-ssa-loop-ivopts.cc
@@ -1175,6 +1175,9 @@ contain_complex_addr_expr (tree expr)
return res;
}
+static tree
+strip_offset (tree expr, poly_uint64_pod *offset = nullptr);
+
/* Allocates an induction variable with given initial value BASE and step STEP
for loop LOOP. NO_OVERFLOW implies the iv doesn't overflow. */
@@ -2942,12 +2945,16 @@ strip_offset_1 (tree expr, bool inside_addr, bool top_compref,
/* Strips constant offsets from EXPR and stores them to OFFSET. */
-tree
+static tree
strip_offset (tree expr, poly_uint64_pod *offset)
{
poly_int64 off;
tree core = strip_offset_1 (expr, false, false, &off);
- *offset = off;
+ if (offset)
+ {
+ gcc_assert (POINTER_TYPE_P (expr));
+ *offset = off;
+ }
return core;
}
@@ -3512,7 +3519,6 @@ add_iv_candidate_derived_from_uses (struct ivopts_data *data)
static void
add_iv_candidate_for_use (struct ivopts_data *data, struct iv_use *use)
{
- poly_uint64 offset;
tree base;
struct iv *iv = use->iv;
tree basetype = TREE_TYPE (iv->base);
@@ -3563,8 +3569,8 @@ add_iv_candidate_for_use (struct ivopts_data *data, struct iv_use *use)
/* Record common candidate with constant offset stripped in base.
Like the use itself, we also add candidate directly for it. */
- base = strip_offset (iv->base, &offset);
- if (maybe_ne (offset, 0U) || base != iv->base)
+ base = strip_offset (iv->base);
+ if (base != iv->base)
{
record_common_cand (data, base, iv->step, use);
add_candidate (data, base, iv->step, false, use);
@@ -3582,9 +3588,9 @@ add_iv_candidate_for_use (struct ivopts_data *data, struct iv_use *use)
step = fold_convert (sizetype, step);
record_common_cand (data, base, step, use);
/* Also record common candidate with offset stripped. */
- base = strip_offset (base, &offset);
- if (maybe_ne (offset, 0U))
- record_common_cand (data, base, step, use);
+ tree alt_base = strip_offset (base);
+ if (alt_base != base)
+ record_common_cand (data, alt_base, step, use);
}
/* At last, add auto-incremental candidates. Make such variables
diff --git a/gcc/tree-ssa-loop-ivopts.h b/gcc/tree-ssa-loop-ivopts.h
index 95148616e70..7a53ce47f10 100644
--- a/gcc/tree-ssa-loop-ivopts.h
+++ b/gcc/tree-ssa-loop-ivopts.h
@@ -28,7 +28,6 @@ extern void dump_cand (FILE *, struct iv_cand *);
extern bool contains_abnormal_ssa_name_p (tree);
extern class loop *outermost_invariant_loop_for_expr (class loop *, tree);
extern bool expr_invariant_in_loop_p (class loop *, tree);
-extern tree strip_offset (tree, poly_uint64_pod *);
bool may_be_nonaddressable_p (tree expr);
void tree_ssa_iv_optimize (void);
--
2.35.3
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2023-06-21 11:43 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-06-21 11:43 [PATCH 1/3] Hide and refactor IVOPTs strip_offset Richard Biener
-- strict thread matches above, loose matches on Subject: below --
2023-06-21 11:30 Richard Biener
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).