On 6/29/21 4:27 AM, Aldy Hernandez wrote:
On 6/29/21 1:19 AM, Martin Sebor wrote:
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?
This seems like it came from the ranger branch which had slightly
different code, particularly it made use of a full ranger with
equivalences. It looks like this could have failed in the branch, but
no longer does. I have removed the comment.
Okay, thanks.
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.
As the author of the original test, I completely differ to you :).
Attached is the latest version with your suggested changes, as well as a
gimple FE test for the previously discussed failing libphobos test.
The tests look good.
In the new APIs, instead of taking vec by value can you please change
them to either by-const-reference if they don't change the vec or by-
reference if they do? I'm in the midst of changing code to do that
with the goal of eventually removing all by-value vec arguments.
Thanks
Martin
Thanks.
Aldy
