On 6/28/21 10:21 AM, Aldy Hernandez via Gcc-patches wrote:
This is the ranger-based backwards threader. It is divided into two
parts: the solver and the path discovery bits.
The solver is generic enough, that it may be of use to other passes,
so it's been abstracted into its own separate class/file. Andrew and
I have already gone over it, so I don't think a review is necessary.
Besides, it's technically an extension of the ranger infrastructure.
On the other hand, the path discovery bits could benefit from the
watchful eye of the jump threading experts.
Documenting the solver in a [ranger-tech] post is on my TODO list,
as I think it would be useful as an example of GORI as a general
tool, outside the VRP world.
As I have mentioned elsewhere, I have gone through each test and
documented the reasons why they were adjusted (when useful). The
reviewer(s) may benefit from looking at the test notes.
I have added a --param=threader-mode={ranger,legacy} option, which I
hope to remove shortly after. It has been useful for diagnosing
issues in the past, though perhaps not so much now. I've left it
in case there's a remote interest in using it during stage1, but
removing it could be a huge cleanup to tree-ssa-threadbackward.c.
If/when accepted, I will open 2-3 PRs with the XFAILed tests as
requested. I am still working on distilling a C counterpart for
the libphobos missing thread edge. It'll hopefully be ready by the
time the review is done.
A version of this patchset with the verification code has
been tested on x86-64, ppc64, ppc64le, and aarch64 (all Linux).
I am currently re-testing on x86-64 Linux, but will not re-test on the
rest of the architectures because...OMG aarch6 is so slow!
I applied the series and ran a subset of tests and didn't see any
failures, just the three XPASSes below. The Wfree-nonheap-object
tests you mentioned in the other post all pass. Looks like you
got past that problem?
XPASS: gcc.dg/uninit-pr61112.c pr61112 (test for bogus messages, line 32)
XPASS: gcc.dg/uninit-pr61112.c pr61112 (test for bogus messages, line 46)
XPASS: gcc.dg/uninit-pr61112.c pr61112 (test for bogus messages, line 60)
A couple of comments on the tests below (I haven't looked at the meat
of the patch):
Thanks.
Aldy
Aldy Hernandez (2):
Implement basic block path solver.
Backwards jump threader rewrite with ranger.
gcc/Makefile.in | 6 +
gcc/flag-types.h | 7 +
gcc/params.opt | 17 +
.../g++.dg/debug/dwarf2/deallocator.C | 3 +-
gcc/testsuite/gcc.c-torture/compile/pr83510.c | 33 ++
gcc/testsuite/gcc.dg/Wrestrict-22.c | 3 +
The change here just adds the comment:
+/* This looks like the threader caused the entire loop to collapse, and the
+ warning pass can't determine the arguments to memcpy. */
+
Since the test passes I'm not sure I understand what the comment
is trying to say. Is it still accurate and necessary?
gcc/testsuite/gcc.dg/loop-unswitch-2.c | 2 +-
gcc/testsuite/gcc.dg/old-style-asm-1.c | 5 +-
gcc/testsuite/gcc.dg/pr68317.c | 4 +-
gcc/testsuite/gcc.dg/pr97567-2.c | 2 +-
gcc/testsuite/gcc.dg/predict-9.c | 4 +-
gcc/testsuite/gcc.dg/shrink-wrap-loop.c | 53 ++
gcc/testsuite/gcc.dg/sibcall-1.c | 10 +
.../gcc.dg/tree-ssa/builtin-sprintf-3.c | 5 +-
I wonder if breaking up the test function into five, one for each
of the tests it does, would be a better way to avoid the IL changes
than disabling all the threading passes. Like in the attached patch.
Martin
gcc/testsuite/gcc.dg/tree-ssa/pr21001.c | 1 +
gcc/testsuite/gcc.dg/tree-ssa/pr21294.c | 1 +
gcc/testsuite/gcc.dg/tree-ssa/pr21417.c | 2 +-
gcc/testsuite/gcc.dg/tree-ssa/pr21458-2.c | 2 +-
gcc/testsuite/gcc.dg/tree-ssa/pr21563.c | 2 +-
gcc/testsuite/gcc.dg/tree-ssa/pr49039.c | 2 +-
gcc/testsuite/gcc.dg/tree-ssa/pr61839_1.c | 2 +-
gcc/testsuite/gcc.dg/tree-ssa/pr61839_3.c | 2 +-
gcc/testsuite/gcc.dg/tree-ssa/pr77445-2.c | 2 +-
.../gcc.dg/tree-ssa/ranger-threader-1.c | 20 +
.../gcc.dg/tree-ssa/ranger-threader-2.c | 39 ++
.../gcc.dg/tree-ssa/ranger-threader-3.c | 41 ++
.../gcc.dg/tree-ssa/ranger-threader-4.c | 83 +++
gcc/testsuite/gcc.dg/tree-ssa/split-path-4.c | 4 +-
.../gcc.dg/tree-ssa/ssa-dom-thread-11.c | 2 +-
.../gcc.dg/tree-ssa/ssa-dom-thread-12.c | 2 +-
.../gcc.dg/tree-ssa/ssa-dom-thread-14.c | 1 +
.../gcc.dg/tree-ssa/ssa-dom-thread-18.c | 5 +-
.../gcc.dg/tree-ssa/ssa-dom-thread-6.c | 4 +-
.../gcc.dg/tree-ssa/ssa-dom-thread-7.c | 1 +
gcc/testsuite/gcc.dg/tree-ssa/ssa-fre-48.c | 2 +-
gcc/testsuite/gcc.dg/tree-ssa/ssa-thread-11.c | 1 +
gcc/testsuite/gcc.dg/tree-ssa/ssa-thread-12.c | 2 +-
gcc/testsuite/gcc.dg/tree-ssa/ssa-thread-14.c | 1 +
gcc/testsuite/gcc.dg/tree-ssa/vrp02.c | 2 +-
gcc/testsuite/gcc.dg/tree-ssa/vrp03.c | 2 +-
gcc/testsuite/gcc.dg/tree-ssa/vrp05.c | 2 +-
gcc/testsuite/gcc.dg/tree-ssa/vrp06.c | 2 +-
gcc/testsuite/gcc.dg/tree-ssa/vrp07.c | 2 +-
gcc/testsuite/gcc.dg/tree-ssa/vrp09.c | 2 +-
gcc/testsuite/gcc.dg/tree-ssa/vrp19.c | 2 +-
gcc/testsuite/gcc.dg/tree-ssa/vrp20.c | 2 +-
gcc/testsuite/gcc.dg/tree-ssa/vrp33.c | 2 +-
gcc/testsuite/gcc.dg/vect/bb-slp-16.c | 7 +
.../gcc.target/i386/avx2-vect-aggressive.c | 2 +-
gcc/tree-ssa-path-solver.cc | 310 ++++++++++++
gcc/tree-ssa-path-solver.h | 85 ++++
gcc/tree-ssa-threadbackward.c | 475 +++++++++++++++++-
gcc/tree-ssa-threadedge.c | 20 +-
gcc/tree-ssa-threadedge.h | 3 +-
gcc/tree-ssa-threadupdate.c | 12 +-
gcc/tree-ssa-threadupdate.h | 2 +-
.../libgomp.graphite/force-parallel-4.c | 1 +
.../libgomp.graphite/force-parallel-8.c | 2 +
58 files changed, 1261 insertions(+), 54 deletions(-)
create mode 100644 gcc/testsuite/gcc.dg/tree-ssa/ranger-threader-1.c
create mode 100644 gcc/testsuite/gcc.dg/tree-ssa/ranger-threader-2.c
create mode 100644 gcc/testsuite/gcc.dg/tree-ssa/ranger-threader-3.c
create mode 100644 gcc/testsuite/gcc.dg/tree-ssa/ranger-threader-4.c
create mode 100644 gcc/tree-ssa-path-solver.cc
create mode 100644 gcc/tree-ssa-path-solver.h
diff --git a/gcc/testsuite/gcc.dg/tree-ssa/builtin-sprintf-3.c b/gcc/testsuite/gcc.dg/tree-ssa/builtin-sprintf-3.c
index fae2a1b73ea..b2e005bc716 100644
--- a/gcc/testsuite/gcc.dg/tree-ssa/builtin-sprintf-3.c
+++ b/gcc/testsuite/gcc.dg/tree-ssa/builtin-sprintf-3.c
@@ -3,7 +3,10 @@
that the sprintf return value (or value range) optimization is not
performed for an unknown string. */
/* { dg-do compile } */
-/* { dg-options "-O2 -Wall -Werror -fdump-tree-optimized -fprintf-return-value" } */
+/* { dg-options "-O2 -Wall -Werror -fdump-tree-optimized" } */
+
+/* Note: Threader will duplicate BBs such that there are multiple
+ string_*_fail calls on certain paths. */
#define INT_MAX __INT_MAX__
#define INT_MIN (-INT_MAX - 1)
@@ -15,7 +18,7 @@ extern void string_lt_0_fail ();
extern void string_eq_0_fail ();
extern void string_gt_0_fail ();
-void test_string (char *d, const char *s)
+void test_string_eq_min (char *d, const char *s)
{
int n = __builtin_sprintf (d, "%-s", s);
@@ -23,13 +26,36 @@ void test_string (char *d, const char *s)
or INT_MAX. (This is a white box test based on knowing that
the optimization computes its own values of the two constants.) */
if (n == INT_MIN) string_eq_min_fail ();
+}
+
+void test_string_eq_max (char *d, const char *s)
+{
+ int n = __builtin_sprintf (d, "%-s", s);
+
if (n == INT_MAX) string_eq_max_fail ();
+}
+
+void test_string_lt_0 (char *d, const char *s)
+{
+ int n = __builtin_sprintf (d, "%-s", s);
/* The return value could be negative when strlen(s) is in excess
of 4095 (the maximum number of bytes a single directive is required
to handle). */
if (n < 0) string_lt_0_fail ();
+}
+
+void test_string_eq_0 (char *d, const char *s)
+{
+ int n = __builtin_sprintf (d, "%-s", s);
+
if (n == 0) string_eq_0_fail ();
+}
+
+void test_string_gt_0 (char *d, const char *s)
+{
+ int n = __builtin_sprintf (d, "%-s", s);
+
if (n > 0) string_gt_0_fail ();
}
