From: Kyrylo Tkachov <[email protected]>
tree_could_trap_p already uses the base type of a method to prove that
an in-bounds load through its this pointer cannot trap. A PARM_DECL
whose REFERENCE_TYPE refers to an object type has the corresponding
middle-end contract. It designates an object at least as large as the
referred-to type. Fortran uses POINTER_TYPE instead when a parameter
can be absent. IPA-SRA also treats REFERENCE_TYPE parameters as safe
to dereference.
GAP Benchmark Suite bfs exposes the missed opportunity. BUStep takes
the graph and frontier bitmaps by reference. The loads of g.in_index_
and front.start_ are invariant, but LIM keeps them below a conditional
guard:
<bb 6> _30 = MEM[(const CSRGraph *)g_15(D)].in_index_;
<bb 7> _38 = MEM[(const Bitmap *)front_22(D)].start_;
Factor the parameter object-size query out of tree_could_trap_p.
Handle object reference parameters and the first parameter of a
METHOD_TYPE. Use nonnull_arg_p as the common policy gate. This keeps
both cases consistent with -fdelete-null-pointer-checks. Use the
referred-to type or method base type to bound an access. LIM can then
move both loads out of the loop.
Moving statement _30 = MEM[(const CSRGraph *)g_15(D)].in_index_;
out of loop 1.
Moving statement _38 = MEM[(const Bitmap *)front_22(D)].start_;
out of loop 1.
The object-size fact proves only that an in-bounds load is safe. It
does not prove that the storage is writable. A reference can designate
read-only storage after const_cast, while a path that does not store
remains valid. cselim must not replace
if (b)
a = d | p;
with an unconditional store such as
a = b ? d | p : p;
Treat stores through reference parameters as possibly trapping in
lhs_could_trap_p. Use that store-specific predicate for conditional
store elimination and LIM store motion.
Document the REFERENCE_TYPE and METHOD_TYPE parameter contracts. Add
load-hoisting tests for the object bound and null-pointer policy. Add
conditional-store tests for early phiopt, full cselim, and LIM. Cover
both const and non-const references.
On Grace, serial GAPBS bfs with kron-23, -O3 and -mcpu=neoverse-v2
reduces cycles by 7.5%.
Bootstrapped and tested on aarch64-none-linux-gnu.
Ok for trunk?
Thanks,
Kyrill
gcc/ChangeLog:
* doc/generic.texi (REFERENCE_TYPE): Document the object
contract for reference parameters.
(METHOD_TYPE): Document the object contract for the this
parameter.
* tree-eh.cc (parm_decl_from_ptr): New function.
(whole_object_param_size): New function.
(tree_could_trap_p): Use whole_object_param_size.
(lhs_could_trap_p): Keep stores through reference parameters
possibly trapping.
* tree-ssa-loop-im.cc (can_sm_ref_p): Use lhs_could_trap_p.
* tree-ssa-phiopt.cc (cond_store_replacement): Likewise.
(cond_store_replacement_limited): Update the comment.
gcc/testsuite/ChangeLog:
* g++.dg/tree-ssa/cselim-ref-1.C: New test.
* g++.dg/tree-ssa/cselim-ref-2.C: New test.
* g++.dg/tree-ssa/lim-ref-1.C: New test.
* g++.dg/tree-ssa/lim-ref-2.C: New test.
* g++.dg/tree-ssa/lim-ref-3.C: New test.
* g++.dg/tree-ssa/lim-ref-4.C: New test.
* g++.dg/tree-ssa/lim-this-1.C: New test.
Signed-off-by: Kyrylo Tkachov <[email protected]>
---
gcc/doc/generic.texi | 7 +-
gcc/testsuite/g++.dg/tree-ssa/cselim-ref-1.C | 28 ++++++++
gcc/testsuite/g++.dg/tree-ssa/cselim-ref-2.C | 25 +++++++
gcc/testsuite/g++.dg/tree-ssa/lim-ref-1.C | 21 ++++++
gcc/testsuite/g++.dg/tree-ssa/lim-ref-2.C | 21 ++++++
gcc/testsuite/g++.dg/tree-ssa/lim-ref-3.C | 22 ++++++
gcc/testsuite/g++.dg/tree-ssa/lim-ref-4.C | 29 ++++++++
gcc/testsuite/g++.dg/tree-ssa/lim-this-1.C | 25 +++++++
gcc/tree-eh.cc | 70 +++++++++++++++++---
gcc/tree-ssa-loop-im.cc | 12 +---
gcc/tree-ssa-phiopt.cc | 13 +---
11 files changed, 242 insertions(+), 31 deletions(-)
create mode 100644 gcc/testsuite/g++.dg/tree-ssa/cselim-ref-1.C
create mode 100644 gcc/testsuite/g++.dg/tree-ssa/cselim-ref-2.C
create mode 100644 gcc/testsuite/g++.dg/tree-ssa/lim-ref-1.C
create mode 100644 gcc/testsuite/g++.dg/tree-ssa/lim-ref-2.C
create mode 100644 gcc/testsuite/g++.dg/tree-ssa/lim-ref-3.C
create mode 100644 gcc/testsuite/g++.dg/tree-ssa/lim-ref-4.C
create mode 100644 gcc/testsuite/g++.dg/tree-ssa/lim-this-1.C
diff --git a/gcc/doc/generic.texi b/gcc/doc/generic.texi
index 42a04e34eda..e61b151cd86 100644
--- a/gcc/doc/generic.texi
+++ b/gcc/doc/generic.texi
@@ -512,7 +512,10 @@ Used to represent pointer types, and pointer to data
member types. The
@item REFERENCE_TYPE
Used to represent reference types. The @code{TREE_TYPE} gives the type
-to which this type refers.
+to which this type refers. A @code{PARM_DECL} whose reference type refers
+to an object type must designate an object whose size is at least that of
+the referred-to type. Front ends must use @code{POINTER_TYPE} for a
+parameter that can be absent or otherwise fail to designate such an object.
@item FUNCTION_TYPE
Used to represent the type of non-member functions and of static member
@@ -537,6 +540,8 @@ The type of @code{*this}, i.e., the class of which
functions of this
type are a member, is given by the @code{TYPE_METHOD_BASETYPE}. The
@code{TYPE_ARG_TYPES} is the parameter list, as for a
@code{FUNCTION_TYPE}, and includes the @code{this} argument.
+The @code{this} argument must designate an object whose size is at least
+that of @code{TYPE_METHOD_BASETYPE}.
@item ARRAY_TYPE
Used to represent array types. The @code{TREE_TYPE} gives the type of
diff --git a/gcc/testsuite/g++.dg/tree-ssa/cselim-ref-1.C
b/gcc/testsuite/g++.dg/tree-ssa/cselim-ref-1.C
new file mode 100644
index 00000000000..06bec81d97e
--- /dev/null
+++ b/gcc/testsuite/g++.dg/tree-ssa/cselim-ref-1.C
@@ -0,0 +1,28 @@
+// A reference parameter proves that an in-bounds load is safe. It does not
+// prove that the referenced object is writable. Keep the store conditional.
+
+// { dg-do compile }
+// { dg-options "-Ofast -fdelete-null-pointer-checks -ftree-cselim
-fdump-tree-phiopt1-details" }
+// { dg-skip-if "" keeps_null_pointer_checks }
+
+void
+f (const int &a, int b, int d)
+{
+ int &c = const_cast<int &> (a);
+ int p = a;
+ if (b)
+ c = d | p;
+}
+
+void
+g (int &a, int b, int d)
+{
+ // A non-const reference can designate a const object after const_cast.
+ // The path that does not store is still valid.
+ int p = a;
+ if (b)
+ a = d | p;
+}
+
+// { dg-final { scan-tree-dump-times "if \\(b_" 2 "phiopt1" } }
+// { dg-final { scan-tree-dump-not "Conditional store replacement happened"
"phiopt1" } }
diff --git a/gcc/testsuite/g++.dg/tree-ssa/cselim-ref-2.C
b/gcc/testsuite/g++.dg/tree-ssa/cselim-ref-2.C
new file mode 100644
index 00000000000..74016e1cc29
--- /dev/null
+++ b/gcc/testsuite/g++.dg/tree-ssa/cselim-ref-2.C
@@ -0,0 +1,25 @@
+// Check the full conditional store replacement after disabling the early
+// phi optimization.
+
+// { dg-do compile }
+// { dg-options "-Ofast -fdelete-null-pointer-checks -ftree-cselim
-fno-ssa-phiopt -fdump-tree-cselim-details" }
+// { dg-skip-if "" keeps_null_pointer_checks }
+
+void
+f (const int &a, int b, int d)
+{
+ if (b)
+ const_cast<int &> (a) = d;
+}
+
+void
+g (int &a, int b, int d)
+{
+ // A non-const reference can designate a const object after const_cast.
+ // The path that does not store is still valid.
+ if (b)
+ a = d;
+}
+
+// { dg-final { scan-tree-dump-times "if \\(b_" 2 "cselim" } }
+// { dg-final { scan-tree-dump-not "Conditional store replacement happened"
"cselim" } }
diff --git a/gcc/testsuite/g++.dg/tree-ssa/lim-ref-1.C
b/gcc/testsuite/g++.dg/tree-ssa/lim-ref-1.C
new file mode 100644
index 00000000000..fcb9a7cbd7b
--- /dev/null
+++ b/gcc/testsuite/g++.dg/tree-ssa/lim-ref-1.C
@@ -0,0 +1,21 @@
+// A parameter of reference type is bound to a valid object of the referenced
+// type, so a load from it cannot trap and may be hoisted out of the loop even
+// though it is only executed on some iterations.
+
+// { dg-do compile }
+// { dg-options "-O2 -fdelete-null-pointer-checks -fdump-tree-lim2-details" }
+// { dg-skip-if "" keeps_null_pointer_checks }
+
+struct S { int n; int *p; };
+
+int
+f (S &s, int n)
+{
+ int r = 0;
+ for (int i = 0; i < n; ++i)
+ if (i & 1)
+ r += s.p[i];
+ return r;
+}
+
+// { dg-final { scan-tree-dump "Moving statement \[^\n\r]*->p;" "lim2" } }
diff --git a/gcc/testsuite/g++.dg/tree-ssa/lim-ref-2.C
b/gcc/testsuite/g++.dg/tree-ssa/lim-ref-2.C
new file mode 100644
index 00000000000..e06cb512c1b
--- /dev/null
+++ b/gcc/testsuite/g++.dg/tree-ssa/lim-ref-2.C
@@ -0,0 +1,21 @@
+// -fno-delete-null-pointer-checks prevents nonnull_arg_p from using the
+// implicit nonnull property of a C++ reference, so the conditional load must
+// stay in the loop.
+
+// { dg-do compile }
+// { dg-options "-O2 -fno-delete-null-pointer-checks -fdump-tree-lim2-details"
}
+
+struct S { int n; int *p; };
+
+int
+f (S &s, int n)
+{
+ int r = 0;
+ for (int i = 0; i < n; ++i)
+ if (i & 1)
+ r += s.p[i];
+ return r;
+}
+
+// { dg-final { scan-tree-dump "Memory reference \[^\n\r]*->p" "lim2" } }
+// { dg-final { scan-tree-dump-not "Moving statement \[^\n\r]*->p;" "lim2" } }
diff --git a/gcc/testsuite/g++.dg/tree-ssa/lim-ref-3.C
b/gcc/testsuite/g++.dg/tree-ssa/lim-ref-3.C
new file mode 100644
index 00000000000..f4a611d9c47
--- /dev/null
+++ b/gcc/testsuite/g++.dg/tree-ssa/lim-ref-3.C
@@ -0,0 +1,22 @@
+// The referenced type bounds what may be speculated: an access past the end
+// of the referenced object still counts as trapping and stays in the loop.
+
+// { dg-do compile }
+// { dg-options "-O2 -fdelete-null-pointer-checks -fdump-tree-lim2-details" }
+// { dg-skip-if "" keeps_null_pointer_checks }
+
+struct Small { int a; };
+struct Big { int a; long b[8]; };
+
+long
+g (Small &s, int n)
+{
+ long r = 0;
+ for (int i = 0; i < n; ++i)
+ if (i & 1)
+ r += reinterpret_cast<Big &> (s).b[3];
+ return r;
+}
+
+// { dg-final { scan-tree-dump "Memory reference \[^\n\r]*struct Big" "lim2" }
}
+// { dg-final { scan-tree-dump-not "Moving statement \[^\n\r]*struct Big"
"lim2" } }
diff --git a/gcc/testsuite/g++.dg/tree-ssa/lim-ref-4.C
b/gcc/testsuite/g++.dg/tree-ssa/lim-ref-4.C
new file mode 100644
index 00000000000..8873cc2dad8
--- /dev/null
+++ b/gcc/testsuite/g++.dg/tree-ssa/lim-ref-4.C
@@ -0,0 +1,29 @@
+// The object bound of a reference parameter does not make a conditional
+// store safe to execute outside its guard.
+
+// { dg-do compile }
+// { dg-options "-Ofast -fdelete-null-pointer-checks -fno-tree-cselim
-fno-ssa-phiopt -fdump-tree-lim2-details" }
+// { dg-skip-if "" keeps_null_pointer_checks }
+
+struct S { int value; };
+
+void
+f (const S &s, int n, int store)
+{
+ for (int i = 0; i < n; ++i)
+ if (store)
+ const_cast<S &> (s).value = 1;
+}
+
+void
+g (S &s, int n, int store)
+{
+ // A non-const reference can designate a const object after const_cast.
+ // The path that does not store is still valid.
+ for (int i = 0; i < n; ++i)
+ if (store)
+ s.value = 1;
+}
+
+// { dg-final { scan-tree-dump-times "Memory reference \[^\n\r]*value" 2
"lim2" } }
+// { dg-final { scan-tree-dump-not "Executing store motion of \[^\n\r]*value"
"lim2" } }
diff --git a/gcc/testsuite/g++.dg/tree-ssa/lim-this-1.C
b/gcc/testsuite/g++.dg/tree-ssa/lim-this-1.C
new file mode 100644
index 00000000000..dbfd79a96b5
--- /dev/null
+++ b/gcc/testsuite/g++.dg/tree-ssa/lim-this-1.C
@@ -0,0 +1,25 @@
+// -fno-delete-null-pointer-checks prevents nonnull_arg_p from using the
+// implicit nonnull property of a method's this parameter, so the conditional
+// load must stay in the loop.
+
+// { dg-do compile }
+// { dg-options "-O2 -fno-delete-null-pointer-checks -fdump-tree-lim2-details"
}
+
+struct S
+{
+ int *values;
+ int f (int);
+};
+
+int
+S::f (int n)
+{
+ int r = 0;
+ for (int i = 0; i < n; ++i)
+ if (i & 1)
+ r += values[i];
+ return r;
+}
+
+// { dg-final { scan-tree-dump "Memory reference \[^\n\r]*->values" "lim2" } }
+// { dg-final { scan-tree-dump-not "Moving statement \[^\n\r]*->values;"
"lim2" } }
diff --git a/gcc/tree-eh.cc b/gcc/tree-eh.cc
index 0835f55af08..ddff13fd272 100644
--- a/gcc/tree-eh.cc
+++ b/gcc/tree-eh.cc
@@ -2752,6 +2752,54 @@ ref_outside_object_p (tree size, poly_offset_int off,
tree refsz)
return false;
}
+/* If PTR is a PARM_DECL or its default SSA definition, return the
+ PARM_DECL. Otherwise return NULL_TREE. */
+
+static tree
+parm_decl_from_ptr (tree ptr)
+{
+ if (TREE_CODE (ptr) == SSA_NAME)
+ {
+ if (!SSA_NAME_IS_DEFAULT_DEF (ptr))
+ return NULL_TREE;
+ ptr = SSA_NAME_VAR (ptr);
+ }
+
+ return ptr && TREE_CODE (ptr) == PARM_DECL ? ptr : NULL_TREE;
+}
+
+/* If PTR is a parameter of the current function, or the default definition
+ of one, that is known to designate a whole object, return the size of that
+ object in bytes. Otherwise return NULL_TREE.
+
+ Two kinds of parameter qualify. The this pointer of a method points to
+ an object of the method base type. A parameter whose reference type
+ refers to an object type is bound to an object of the referenced type.
+ In both cases the pointed-to object is at least as large as that type. */
+
+static tree
+whole_object_param_size (tree ptr)
+{
+ tree type;
+
+ if (!cfun)
+ return NULL_TREE;
+
+ ptr = parm_decl_from_ptr (ptr);
+ if (!ptr)
+ return NULL_TREE;
+
+ if (TREE_CODE (TREE_TYPE (ptr)) == REFERENCE_TYPE)
+ type = TREE_TYPE (TREE_TYPE (ptr));
+ else if (TREE_CODE (TREE_TYPE (cfun->decl)) == METHOD_TYPE
+ && ptr == DECL_ARGUMENTS (cfun->decl))
+ type = TYPE_METHOD_BASETYPE (TREE_TYPE (cfun->decl));
+ else
+ return NULL_TREE;
+
+ return nonnull_arg_p (ptr) ? TYPE_SIZE_UNIT (type) : NULL_TREE;
+}
+
/* Return true if EXPR can trap, as in dereferencing an invalid pointer
location or floating point arithmetic. C.f. the rtl version, may_trap_p.
This routine expects only GIMPLE lhs or rhs input. */
@@ -2851,19 +2899,11 @@ tree_could_trap_p (tree expr)
tree refsz = TYPE_SIZE_UNIT (TREE_TYPE (expr));
return ref_outside_object_p (size, off, refsz);
}
- if (cfun
- && TREE_CODE (TREE_TYPE (cfun->decl)) == METHOD_TYPE
- && ((TREE_CODE (TREE_OPERAND (expr, 0)) == SSA_NAME
- && SSA_NAME_IS_DEFAULT_DEF (TREE_OPERAND (expr, 0))
- && (SSA_NAME_VAR (TREE_OPERAND (expr, 0))
- == DECL_ARGUMENTS (cfun->decl)))
- || TREE_OPERAND (expr, 0) == DECL_ARGUMENTS (cfun->decl)))
+ if (tree size = whole_object_param_size (TREE_OPERAND (expr, 0)))
{
poly_offset_int off = mem_ref_offset (expr);
if (maybe_lt (off, 0))
return true;
- tree size = TYPE_SIZE_UNIT
- (TYPE_METHOD_BASETYPE (TREE_TYPE (cfun->decl)));
tree refsz = TYPE_SIZE_UNIT (TREE_TYPE (expr));
return ref_outside_object_p (size, off, refsz);
}
@@ -2918,13 +2958,23 @@ tree_could_trap_p (tree expr)
}
}
-/* Returns true if LHS is known not to trap as a store. */
+/* Return true if LHS can trap as a store. */
bool
lhs_could_trap_p (tree lhs)
{
tree lhsbase = get_base_address (lhs);
if (tree_could_trap_p (lhs))
return true;
+ /* A reference can designate read-only storage. Its whole-object property
+ makes an in-bounds load safe, but does not make an unconditional store
+ safe. */
+ if (TREE_CODE (lhsbase) == MEM_REF
+ || TREE_CODE (lhsbase) == TARGET_MEM_REF)
+ {
+ tree ptr = parm_decl_from_ptr (TREE_OPERAND (lhsbase, 0));
+ if (ptr && TREE_CODE (TREE_TYPE (ptr)) == REFERENCE_TYPE)
+ return true;
+ }
/* tree_could_trap_p is a predicate for loads, so check
for readonly memory explicitly. */
if ((DECL_P (lhsbase) && TREE_READONLY (lhsbase))
diff --git a/gcc/tree-ssa-loop-im.cc b/gcc/tree-ssa-loop-im.cc
index 41264560009..eb2d0b171c1 100644
--- a/gcc/tree-ssa-loop-im.cc
+++ b/gcc/tree-ssa-loop-im.cc
@@ -3351,8 +3351,6 @@ ref_in_loop_hot_body::operator () (mem_ref_loc *loc)
static bool
can_sm_ref_p (class loop *loop, im_mem_ref *ref)
{
- tree base;
-
/* Can't hoist unanalyzable refs. */
if (!MEM_ANALYZABLE (ref))
return false;
@@ -3371,14 +3369,8 @@ can_sm_ref_p (class loop *loop, im_mem_ref *ref)
if (tree_could_throw_p (ref->mem.ref))
return false;
- /* If it can trap, it must be always executed in LOOP.
- Readonly memory locations may trap when storing to them, but
- tree_could_trap_p is a predicate for rvalues, so check that
- explicitly. */
- base = get_base_address (ref->mem.ref);
- if ((tree_could_trap_p (ref->mem.ref)
- || (DECL_P (base) && TREE_READONLY (base))
- || TREE_CODE (base) == STRING_CST)
+ /* If a store can trap, it must be always executed in LOOP. */
+ if (lhs_could_trap_p (ref->mem.ref)
/* ??? We can at least use false here, allowing loads? We
are forcing conditional stores if the ref is not always
stored to later anyway. So this would only guard
diff --git a/gcc/tree-ssa-phiopt.cc b/gcc/tree-ssa-phiopt.cc
index 784221c83b3..f0566728464 100644
--- a/gcc/tree-ssa-phiopt.cc
+++ b/gcc/tree-ssa-phiopt.cc
@@ -3325,15 +3325,8 @@ cond_store_replacement (basic_block middle_bb,
basic_block join_bb, edge e0,
/* If LHS is an access to a local variable without address-taken
(or when we allow data races) and known not to trap, we could
always safely move down the store. */
- tree base;
if (ref_can_have_store_data_races (lhs)
- || tree_could_trap_p (lhs)
- /* tree_could_trap_p is a predicate for rvalues, so check
- for readonly memory explicitly. */
- || ((base = get_base_address (lhs))
- && ((DECL_P (base)
- && TREE_READONLY (base))
- || TREE_CODE (base) == STRING_CST)))
+ || lhs_could_trap_p (lhs))
return false;
}
@@ -3717,8 +3710,8 @@ cond_store_replacement_limited (basic_block middle_bb,
basic_block join_bb,
if (b)
nontrapping = c;
- For case 3, nontrapping needs to satisfy tree_could_trap_p.
- In both cases ref_can_have_store_data_races needs to be satisfy.
+ For case 3, the store must not trap.
+ In both cases ref_can_have_store_data_races must be satisfied.
*/
/* Maybe the load/local non-escaped variable case. */
--
2.50.1 (Apple Git-155)
