From: Kyrylo Tkachov <[email protected]>
A REFERENCE_TYPE parameter that refers to an object designates an object that
is at least as large as its referred-to type. A method's this parameter
designates an object that is at least as large as TYPE_METHOD_BASETYPE.
tree_could_trap_p already uses this property for the this parameter, but not
for other reference parameters.
LIM therefore cannot move safe invariant loads through reference parameters.
GAPBS bfs contains this case in BUStep. Without this change, the loads
remain below conditional guards in loop 1:
<bb 6> _30 = MEM[(const struct CSRGraph *)g_15(D)].in_index_;
<bb 13> _38 = MEM[(const struct Bitmap *)front_22(D)].start_;
Factor the parameter object-size query into whole_object_param_size.
Handle reference parameters and this parameters there. Use nonnull_arg_p
as the policy gate, in line with -fdelete-null-pointer-checks. LIM can
then move both loads to the loop-1 preheader:
<bb 12> _30 = MEM[(const struct CSRGraph *)g_15(D)].in_index_;
_38 = MEM[(const struct Bitmap *)front_22(D)].start_;
Use the object bound only for rvalues. A reference or this parameter can
designate read-only storage after const_cast, so its object bound does not
prove that a store is safe.
Add tests for load motion, access bounds, the null-pointer policy,
conditional store elimination, store motion, built-in memory attributes, and
strub watermark attributes. Cover reference parameters and this parameters.
On Grace, serial GAPBS bfs with kron-23, -O3, and -mcpu=grace
reduces cycles by 6.2%.
Bootstrapped and tested on aarch64-unknown-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_1): Use whole-object parameter bounds for loads.
gcc/testsuite/ChangeLog:
* c-c++-common/strub-mem-notrap.c: New test.
* 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.
* g++.dg/tree-ssa/noncall-store-builtin.C: New test.
Signed-off-by: Kyrylo Tkachov <[email protected]>
---
gcc/doc/generic.texi | 7 +-
gcc/testsuite/c-c++-common/strub-mem-notrap.c | 21 ++++++
gcc/testsuite/g++.dg/tree-ssa/cselim-ref-1.C | 29 ++++++++
gcc/testsuite/g++.dg/tree-ssa/cselim-ref-2.C | 26 +++++++
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 | 30 ++++++++
gcc/testsuite/g++.dg/tree-ssa/lim-this-1.C | 25 +++++++
.../g++.dg/tree-ssa/noncall-store-builtin.C | 20 +++++
gcc/tree-eh.cc | 73 +++++++++++++++----
11 files changed, 278 insertions(+), 17 deletions(-)
create mode 100644 gcc/testsuite/c-c++-common/strub-mem-notrap.c
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
create mode 100644 gcc/testsuite/g++.dg/tree-ssa/noncall-store-builtin.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/c-c++-common/strub-mem-notrap.c
b/gcc/testsuite/c-c++-common/strub-mem-notrap.c
new file mode 100644
index 00000000000..ef9bbc39535
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/strub-mem-notrap.c
@@ -0,0 +1,21 @@
+/* Keep read and write attributes separate for a strub watermark update. */
+
+/* { dg-do compile } */
+/* { dg-options "-O2 -fstrub=strict -fnon-call-exceptions" } */
+/* { dg-additional-options "-fdelete-null-pointer-checks -fdump-rtl-expand" }
*/
+/* { dg-require-effective-target strub } */
+/* { dg-skip-if "" keeps_null_pointer_checks } */
+
+volatile int sink;
+
+__attribute__ ((__strub__ ("internal"), noinline))
+int
+scrubbed (int value)
+{
+ sink = value;
+ return sink;
+}
+
+/* The watermark load is nontrapping. The conditional store can trap. */
+/* { dg-final { scan-rtl-dump {\(mem/f/c:[^\n]*watermark_ptr} "expand" } } */
+/* { dg-final { scan-rtl-dump {\(mem/f:[^\n]*watermark_ptr} "expand" } } */
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..ede771ecfc0
--- /dev/null
+++ b/gcc/testsuite/g++.dg/tree-ssa/cselim-ref-1.C
@@ -0,0 +1,29 @@
+// 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 "-O2 -fdelete-null-pointer-checks -ftree-cselim
-fdump-tree-phiopt1-details" }
+// { dg-additional-options "-fallow-store-data-races" }
+// { 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..16aeceb6654
--- /dev/null
+++ b/gcc/testsuite/g++.dg/tree-ssa/cselim-ref-2.C
@@ -0,0 +1,26 @@
+// Check the full conditional store replacement after disabling the early
+// phi optimization.
+
+// { dg-do compile }
+// { dg-options "-O2 -fdelete-null-pointer-checks -ftree-cselim
-fno-ssa-phiopt -fdump-tree-cselim-details" }
+// { dg-additional-options "-fallow-store-data-races" }
+// { 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..9a9ac0b845a
--- /dev/null
+++ b/gcc/testsuite/g++.dg/tree-ssa/lim-ref-4.C
@@ -0,0 +1,30 @@
+// The object bound of a reference parameter does not make a conditional
+// store safe to execute outside its guard.
+
+// { dg-do compile }
+// { dg-options "-O2 -fdelete-null-pointer-checks -fno-tree-cselim
-fno-ssa-phiopt -fdump-tree-lim2-details" }
+// { dg-additional-options "-fallow-store-data-races" }
+// { 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/testsuite/g++.dg/tree-ssa/noncall-store-builtin.C
b/gcc/testsuite/g++.dg/tree-ssa/noncall-store-builtin.C
new file mode 100644
index 00000000000..c99b3d7b3a0
--- /dev/null
+++ b/gcc/testsuite/g++.dg/tree-ssa/noncall-store-builtin.C
@@ -0,0 +1,20 @@
+// Keep the source load of an inline memcpy nontrapping, but not its
+// destination store.
+
+// { dg-do compile }
+// { dg-options "-O2 -fdelete-null-pointer-checks -fnon-call-exceptions
-finline-stringops=memcpy -fdump-rtl-expand" }
+// { dg-require-effective-target ptr32plus }
+// { dg-skip-if "" keeps_null_pointer_checks }
+
+typedef unsigned char block[16384];
+
+void
+copy (block &dest, const block &src)
+{
+ __builtin_memcpy (dest, src, sizeof (block));
+}
+
+// { dg-final { scan-rtl-dump {\(mem(?:/[a-z])*/c(?:/[a-z])*:} "expand" } }
+// { dg-final { scan-rtl-dump {\(set[ \t\n]+\(mem(?:/[a-z])*:} "expand" } }
+// { dg-final { scan-rtl-dump-not {\(set[
\t\n]+\(mem(?:/[a-z])*/c(?:/[a-z])*:} "expand" } }
+// { dg-final { scan-assembler-not {\mmemcpy\M} } }
diff --git a/gcc/tree-eh.cc b/gcc/tree-eh.cc
index e48d9502fb0..e3b8db0b6d0 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 evaluating floating-point arithmetic. See may_trap_p for the
RTL counterpart. This routine expects only GIMPLE lhs or rhs input. LHS
@@ -2852,23 +2900,16 @@ tree_could_trap_1 (tree expr, bool lhs)
tree refsz = TYPE_SIZE_UNIT (TREE_TYPE (expr));
return ref_outside_object_p (size, off, refsz);
}
- /* See if the base is this for C++ methods. For LHS, this can still
- trap. */
- if (!lhs && 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 (!lhs)
{
- 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);
+ 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 refsz = TYPE_SIZE_UNIT (TREE_TYPE (expr));
+ return ref_outside_object_p (size, off, refsz);
+ }
}
return true;
--
2.50.1 (Apple Git-155)
