On 12/11/23 03:02, Richard Biener wrote:
On Sun, 10 Dec 2023, Jason Merrill wrote:
On 12/10/23 05:22, Richard Biener wrote:
Am 09.12.2023 um 21:13 schrieb Jason Merrill <ja...@redhat.com>:
On 11/2/23 21:18, Nathaniel Shead wrote:
Bootstrapped and regtested on x86-64_pc_linux_gnu.
I'm not entirely sure if the change I made to have destructors clobber
with
CLOBBER_EOL instead of CLOBBER_UNDEF is appropriate, but nothing seemed to
have
broken by doing this and I wasn't able to find anything else that really
depended on this distinction other than a warning pass. Otherwise I could
experiment with a new clobber kind for destructor calls.
It seems wrong to me: CLOBBER_EOL is documented to mean that the storage is
expiring at that point as well, which a (pseudo-)destructor does not imply;
it's perfectly valid to destroy an object and then create another in the
same storage.
We probably do want another clobber kind for end of object lifetime. And/or
one for beginning of object lifetime.
There?s not much semantically different between UNDEF and end of object but
not storage lifetime? At least for what middle-end optimizations do.
That's fine for the middle-end, but Nathaniel's patch wants to distinguish
between the clobbers at beginning and end of object lifetime in order to
diagnose stores to an out-of-lifetime object in constexpr evaluation.
Ah, I see. I did want to add CLOBBER_SOL (start-of-life) when working
on PR90348, but I always fail to finish working on that stack-slot sharing
issue. But it would be for the storage life, not object life, also
added by gimplification.
One option might be to remove the clobber at the beginning of the constructor;
are there any useful optimizations enabled by that, or is it just pedantically
breaking people's code?
It's allowing DSE to the object that was live before the new one. Not
all objects require explicit destruction (which would get you a clobber)
before storage can be re-used.
EOL is used by stack slot sharing and that operates on the underlying
storage, not individual objects live in it.
I wonder about changing the name to EOS (end of storage [duration]) to avoid
similar confusion with object lifetime?
EOS{L,D}? But sure, better names (and documentation) are appreciated.
Maybe something like this? Or shall we write out the names like
CLOBBER_OBJECT_START, CLOBBER_STORAGE_END, etc?
From 14f71a9479bd0cf4249c8c9e917a9caf3eac8c82 Mon Sep 17 00:00:00 2001
From: Jason Merrill <ja...@redhat.com>
Date: Mon, 11 Dec 2023 11:35:31 -0500
Subject: [PATCH] tree: add to clobber_kind
To: gcc-patches@gcc.gnu.org
In discussion of PR71093 it came up that more clobber_kind options would be
useful within the C++ front-end.
gcc/ChangeLog:
* tree-core.h (enum clobber_kind): Rename CLOBBER_EOL to
CLOBBER_EOSD. Add CLOBBER_BOSD, CLOBBER_BOBL, CLOBBER_EOBL.
* gimple-lower-bitint.cc
* gimple-ssa-warn-access.cc
* gimplify.cc
* tree-inline.cc
* tree-pretty-print.cc
* tree-ssa-ccp.cc: Adjust for rename.
---
gcc/tree-core.h | 13 ++++++++++---
gcc/gimple-lower-bitint.cc | 8 ++++----
gcc/gimple-ssa-warn-access.cc | 2 +-
gcc/gimplify.cc | 8 ++++----
gcc/tree-inline.cc | 4 ++--
gcc/tree-pretty-print.cc | 2 +-
gcc/tree-ssa-ccp.cc | 2 +-
7 files changed, 23 insertions(+), 16 deletions(-)
diff --git a/gcc/tree-core.h b/gcc/tree-core.h
index 04c04cf2f37..bdf14605c91 100644
--- a/gcc/tree-core.h
+++ b/gcc/tree-core.h
@@ -986,12 +986,19 @@ enum annot_expr_kind {
annot_expr_kind_last
};
-/* The kind of a TREE_CLOBBER_P CONSTRUCTOR node. */
+/* The kind of a TREE_CLOBBER_P CONSTRUCTOR node. Other than _UNDEF, these are
+ in roughly sequential order. */
enum clobber_kind {
/* Unspecified, this clobber acts as a store of an undefined value. */
CLOBBER_UNDEF,
- /* This clobber ends the lifetime of the storage. */
- CLOBBER_EOL,
+ /* Beginning of storage duration, e.g. malloc. */
+ CLOBBER_BOSD,
+ /* Beginning of object lifetime, e.g. C++ constructor. */
+ CLOBBER_BOBL,
+ /* End of object lifetime, e.g. C++ destructor. */
+ CLOBBER_EOBL,
+ /* End of storage duration, e.g. free. */
+ CLOBBER_EOSD,
CLOBBER_LAST
};
diff --git a/gcc/gimple-lower-bitint.cc b/gcc/gimple-lower-bitint.cc
index c55c32fb40d..00c3a5b20a8 100644
--- a/gcc/gimple-lower-bitint.cc
+++ b/gcc/gimple-lower-bitint.cc
@@ -806,7 +806,7 @@ bitint_large_huge::handle_operand (tree op, tree idx)
&& m_after_stmt
&& bitmap_bit_p (m_single_use_names, SSA_NAME_VERSION (op)))
{
- tree clobber = build_clobber (TREE_TYPE (m_vars[p]), CLOBBER_EOL);
+ tree clobber = build_clobber (TREE_TYPE (m_vars[p]), CLOBBER_EOSD);
g = gimple_build_assign (m_vars[p], clobber);
gimple_stmt_iterator gsi = gsi_for_stmt (m_after_stmt);
gsi_insert_after (&gsi, g, GSI_SAME_STMT);
@@ -2063,7 +2063,7 @@ bitint_large_huge::handle_operand_addr (tree op, gimple *stmt,
tree ret = build_fold_addr_expr (var);
if (!stmt_ends_bb_p (gsi_stmt (m_gsi)))
{
- tree clobber = build_clobber (atype, CLOBBER_EOL);
+ tree clobber = build_clobber (atype, CLOBBER_EOSD);
g = gimple_build_assign (var, clobber);
gsi_insert_after (&m_gsi, g, GSI_SAME_STMT);
}
@@ -2100,7 +2100,7 @@ bitint_large_huge::handle_operand_addr (tree op, gimple *stmt,
ret = build_fold_addr_expr (var);
if (!stmt_ends_bb_p (gsi_stmt (m_gsi)))
{
- tree clobber = build_clobber (m_limb_type, CLOBBER_EOL);
+ tree clobber = build_clobber (m_limb_type, CLOBBER_EOSD);
g = gimple_build_assign (var, clobber);
gsi_insert_after (&m_gsi, g, GSI_SAME_STMT);
}
@@ -3707,7 +3707,7 @@ bitint_large_huge::finish_arith_overflow (tree var, tree obj, tree type,
}
if (var)
{
- tree clobber = build_clobber (TREE_TYPE (var), CLOBBER_EOL);
+ tree clobber = build_clobber (TREE_TYPE (var), CLOBBER_EOSD);
g = gimple_build_assign (var, clobber);
gsi_insert_after (&m_gsi, g, GSI_SAME_STMT);
}
diff --git a/gcc/gimple-ssa-warn-access.cc b/gcc/gimple-ssa-warn-access.cc
index 1646bd1be14..e71e37214c5 100644
--- a/gcc/gimple-ssa-warn-access.cc
+++ b/gcc/gimple-ssa-warn-access.cc
@@ -4364,7 +4364,7 @@ void
pass_waccess::check_stmt (gimple *stmt)
{
if (m_check_dangling_p
- && gimple_clobber_p (stmt, CLOBBER_EOL))
+ && gimple_clobber_p (stmt, CLOBBER_EOSD))
{
/* Ignore clobber statements in blocks with exceptional edges. */
basic_block bb = gimple_bb (stmt);
diff --git a/gcc/gimplify.cc b/gcc/gimplify.cc
index 342e43a7f25..f7a2a4d472c 100644
--- a/gcc/gimplify.cc
+++ b/gcc/gimplify.cc
@@ -1518,7 +1518,7 @@ gimplify_bind_expr (tree *expr_p, gimple_seq *pre_p)
tmp = build_call_expr_loc (EXPR_LOCATION (*e), tmp, 2, v,
build_zero_cst (ptr_type_node));
tsi_link_after (&e, tmp, TSI_SAME_STMT);
- tmp = build_clobber (TREE_TYPE (v), CLOBBER_EOL);
+ tmp = build_clobber (TREE_TYPE (v), CLOBBER_EOSD);
tmp = fold_build2_loc (loc, MODIFY_EXPR, TREE_TYPE (v), v,
fold_convert (TREE_TYPE (v), tmp));
++e;
@@ -1651,7 +1651,7 @@ gimplify_bind_expr (tree *expr_p, gimple_seq *pre_p)
build_zero_cst (ptr_type_node));
gimplify_and_add (tmp, &cleanup);
gimple *clobber_stmt;
- tmp = build_clobber (TREE_TYPE (v), CLOBBER_EOL);
+ tmp = build_clobber (TREE_TYPE (v), CLOBBER_EOSD);
clobber_stmt = gimple_build_assign (v, tmp);
gimple_set_location (clobber_stmt, end_locus);
gimplify_seq_add_stmt (&cleanup, clobber_stmt);
@@ -1665,7 +1665,7 @@ gimplify_bind_expr (tree *expr_p, gimple_seq *pre_p)
&& !is_gimple_reg (t)
&& flag_stack_reuse != SR_NONE)
{
- tree clobber = build_clobber (TREE_TYPE (t), CLOBBER_EOL);
+ tree clobber = build_clobber (TREE_TYPE (t), CLOBBER_EOSD);
gimple *clobber_stmt;
clobber_stmt = gimple_build_assign (t, clobber);
gimple_set_location (clobber_stmt, end_locus);
@@ -7417,7 +7417,7 @@ gimplify_target_expr (tree *expr_p, gimple_seq *pre_p, gimple_seq *post_p)
{
if (flag_stack_reuse == SR_ALL)
{
- tree clobber = build_clobber (TREE_TYPE (temp), CLOBBER_EOL);
+ tree clobber = build_clobber (TREE_TYPE (temp), CLOBBER_EOSD);
clobber = build2 (MODIFY_EXPR, TREE_TYPE (temp), temp, clobber);
gimple_push_cleanup (temp, clobber, false, pre_p, true);
}
diff --git a/gcc/tree-inline.cc b/gcc/tree-inline.cc
index a4fc839a22d..e59342b1458 100644
--- a/gcc/tree-inline.cc
+++ b/gcc/tree-inline.cc
@@ -5136,7 +5136,7 @@ expand_call_inline (basic_block bb, gimple *stmt, copy_body_data *id,
&& !is_gimple_reg (*varp)
&& !(id->debug_map && id->debug_map->get (p)))
{
- tree clobber = build_clobber (TREE_TYPE (*varp), CLOBBER_EOL);
+ tree clobber = build_clobber (TREE_TYPE (*varp), CLOBBER_EOSD);
gimple *clobber_stmt;
clobber_stmt = gimple_build_assign (*varp, clobber);
gimple_set_location (clobber_stmt, gimple_location (stmt));
@@ -5208,7 +5208,7 @@ expand_call_inline (basic_block bb, gimple *stmt, copy_body_data *id,
&& !is_gimple_reg (id->retvar)
&& !stmt_ends_bb_p (stmt))
{
- tree clobber = build_clobber (TREE_TYPE (id->retvar), CLOBBER_EOL);
+ tree clobber = build_clobber (TREE_TYPE (id->retvar), CLOBBER_EOSD);
gimple *clobber_stmt;
clobber_stmt = gimple_build_assign (id->retvar, clobber);
gimple_set_location (clobber_stmt, gimple_location (old_stmt));
diff --git a/gcc/tree-pretty-print.cc b/gcc/tree-pretty-print.cc
index 0dabb6d1580..f05ef7a44a3 100644
--- a/gcc/tree-pretty-print.cc
+++ b/gcc/tree-pretty-print.cc
@@ -2624,7 +2624,7 @@ dump_generic_node (pretty_printer *pp, tree node, int spc, dump_flags_t flags,
if (TREE_CLOBBER_P (node))
{
pp_string (pp, "CLOBBER");
- if (CLOBBER_KIND (node) == CLOBBER_EOL)
+ if (CLOBBER_KIND (node) == CLOBBER_EOSD)
pp_string (pp, "(eol)");
}
else if (TREE_CODE (TREE_TYPE (node)) == RECORD_TYPE
diff --git a/gcc/tree-ssa-ccp.cc b/gcc/tree-ssa-ccp.cc
index ddcbaaaa417..245abaaf1cf 100644
--- a/gcc/tree-ssa-ccp.cc
+++ b/gcc/tree-ssa-ccp.cc
@@ -2525,7 +2525,7 @@ insert_clobber_before_stack_restore (tree saved_val, tree var,
FOR_EACH_IMM_USE_STMT (stmt, iter, saved_val)
if (gimple_call_builtin_p (stmt, BUILT_IN_STACK_RESTORE))
{
- clobber = build_clobber (TREE_TYPE (var), CLOBBER_EOL);
+ clobber = build_clobber (TREE_TYPE (var), CLOBBER_EOSD);
clobber_stmt = gimple_build_assign (var, clobber);
i = gsi_for_stmt (stmt);
--
2.39.3
