replacing the backwards threader and more

Martin Sebor msebor@gmail.com
Tue Jun 29 22:16:39 GMT 2021


On 6/28/21 6:32 PM, Aldy Hernandez wrote:
> I had changed my approach to passing -Wno-free-nonheap-object in the 
> Makefile. Can you try disabling the Makefile bit and bootstrapping, 
> cause that was still failing.

I see.  I just tested it and my patch does let the existing #pragma
suppress the warning.  I just pinged the core patch yesterday so
once it and the rest of the series gets approved there will be no
need for the Makefile workaround.

Martin

> 
> Thanks.
> Aldy
> 
> On Tue, Jun 29, 2021, 00:29 Martin Sebor <msebor@gmail.com 
> <mailto:msebor@gmail.com>> wrote:
> 
>     On 6/28/21 12:17 AM, Aldy Hernandez wrote:
>      >
>      >
>      > On 6/25/21 7:19 PM, Martin Sebor wrote:
>      >> On 6/25/21 10:20 AM, Aldy Hernandez via Gcc wrote:
>      >>> Hi folks.
>      >>>
>      >>> I'm done with benchmarking, testing and cleanups, so I'd like
>     to post
>      >>> my patchset for review.  However, before doing so, I'd like to
>      >>> address a handful of meta-issues that may affect how I post these
>      >>> patches.
>      >>>
>      >>> Trapping on differences
>      >>> =======================
>      >>>
>      >>> Originally I wanted to contribute verification code that would
>     trap
>      >>> if the legacy code threaded any edges the new code couldn't (to be
>      >>> removed after a week).  However, after having tested on various
>      >>> architectures and only running once into a missing thread, I'm
>      >>> leaning towards omitting the verification code, since it's
>     fragile,
>      >>> time consuming, and quite hacky.
>      >>>
>      >>> For the record, I have tested on x86-64, aarch64, ppc64 and
>     ppc64le.
>      >>> There is only one case, across bootstrap and regression tests
>     where
>      >>> the verification code is ever tripped (discussed below).
>      >>>
>      >>> Performance
>      >>> ===========
>      >>>
>      >>> I re-ran benchmarks as per our callgrind suite, and the penalty
>     with
>      >>> the current pipeline is 1.55% of overall compilation time.  As is
>      >>> being discussed, we should be able to mitigate this
>     significantly by
>      >>> removing other threading passes.
>      >>>
>      >>> Failing testcases
>      >>> =================
>      >>>
>      >>> I have yet to run into incorrect code being generated, but I
>     have had
>      >>> to tweak a considerable number of tests.  I have verified every
>      >>> single discrepancy and documented my changes in the testsuite
>     when it
>      >>> merited doing so.  However, there are a couple tests that trigger
>      >>> regressions and I'd like to ask for guidance on how to address
>     them.
>      >>>
>      >>> 1. gcc.c-torture/compile/pr83510.c
>      >>>
>      >>> I would like to XFAIL this.
>      >>>
>      >>> What happens here is that thread1 threads a switch statement such
>      >>> that the various cases have been split into different independent
>      >>> blocks. One of these blocks exposes an arr[i_27] access which is
>      >>> later propagated by VRP to be arr[10].  This is an invalid access,
>      >>> but the array bounds code doesn't know it is an unreachable path.
>      >>
>      >> The test has a bunch of loops that iterate over the 10 array
>     elements.
>      >> There have been bug reports about loop unrolling causing false
>     positives
>      >> -Warray-bounds (e.g., PR 92539, 92110, or 86341) so this could be
>      >> the same issue.
>      >>
>      >>>
>      >>> However, it is not until dom2 that we "know" that the value of the
>      >>> switch index is such that the path to arr[10] is unreachable.  For
>      >>> that matter, it is not until dom3 that we remove the
>     unreachable path.
>      >>
>      >> If you do XFAIL it can you please isolate a small test case and open
>      >> a bug and make it a -Warray-bounds blocker?
>      >
>      > Will do.
>      >
>      >>
>      >>>
>      >>> 2. -Wfree-nonheap-object
>      >>>
>      >>> This warning is triggered while cleaning up an auto_vec.  I see
>     that
>      >>> the va_heap::release() inline is wrapped with a pragma ignore
>      >>> "-Wfree-nonheap-object", but this is not sufficient because jump
>      >>> threading may alter uses in such a way that
>     may_emit_free_warning()
>      >>> will warn on the *inlined* location, thus bypassing the pragma.
>      >>>
>      >>> I worked around this with a mere:
>      >>>
>      >>>  > @@ -13839,6 +13839,7 @@ maybe_emit_free_warning (tree exp)
>      >>>>    location_t loc = tree_inlined_location (exp);
>      >>>> +  loc = EXPR_LOCATION (exp);
>      >>>
>      >>> but this causes a ton of Wfree-nonheap* tests to fail.  I think
>      >>> someone more knowledgeable should address this (msebor??).
>      >>
>      >> This sounds like the same problem as PR 98871.  Does the patch below
>      >> fix it?
>      >> https://gcc.gnu.org/pipermail/gcc-patches/2021-June/572515.html
>      >> If so, I suggest getting that patch in first to avoid testsuite
>      >> failures.  If it doesn't fix it I'll look into it before you commit
>      >> your changes.
>      >
>      > The latest patch in the above thread does not apply.  Do you have
>     a more
>      > recent patch?
> 
>     The first patch in the series applies cleanly for me:
>     https://gcc.gnu.org/pipermail/gcc-patches/2021-June/572516.html
>     but patch 2/4 does need a few adjustments.  With those and your
>     latest changes applied I don't see any -Wfree-nonheap-object test
>     failures.
> 
>     Martin
> 
>      >
>      >>> 3. uninit-pred-9_b.c
>      >>>
>      >>> The uninit code is getting confused with the threading and the
>     bogus
>      >>> warning in line 24 is back.  I looked at the thread, and it is
>     correct.
>      >>>
>      >>> I'm afraid all these warnings are quite fragile in the presence of
>      >>> more aggressive optimizations, and I suspect it will only get
>     worse.
>      >>
>      >>  From my recent review of open -Wmaybe-uninitialized bugs (and
>      >> the code) it does seem to be both fragile and getting worse.  I've
>      >> only found a few simple problems so far in the code but nothing that
>      >> would make a dramatic difference so I can't say if it's possible to
>      >> do much better, but I'm not done or ready to give up.  If you XFAIL
>      >> this too please open a bug for it and make it a blocker for
>      >> -Wuninitialized?
>      >
>      > Will do.
>      >
>      > Good to know these are somewhat known issues.
>      >
>      > Thanks.
>      > Aldy
>      >
> 



More information about the Gcc mailing list