Re: [PATCH] Try to resolve paths in threader without looking further back.

[Martin Sebor <msebor@gmail.com>]

On 10/21/21 1:17 AM, Aldy Hernandez wrote:
> On Wed, Oct 20, 2021 at 10:01 PM Jeff Law <jeffreyalaw@gmail.com> wrote:
>>
>>
>>
>> On 10/20/2021 9:15 AM, Aldy Hernandez wrote:
>>> On Wed, Oct 20, 2021 at 4:35 PM Martin Sebor <msebor@gmail.com> wrote:
>>>
>>>> I appreciate the heads up.  I'm happy that the threader has
>>>> improved.  I'm obviously not pleased that it has led to regressions
>>>> in warnings but I understand that in some cases they might be due
>>>> to limitations in the warning code.  I think the test case you have
>>>> xfailed might be one such example.  The uninitialized warnings are
>>>> exquisitely sensitive to these types of changes.  If/when this patch
>>>> is applied please reopen PR 89230 and reference this commit.
>>>>
>>>> Having said that, to maintain the quality of diagnostics,
>>>> the work that goes into these nice optimizer improvements needs
>>>> to be balanced by an effort to either update the warning code
>>>> to cope with the IL changes, or the optimizers need to take care
>>>> to avoid exposing undefined code that the warnings are designed
>>>> to detect.  I'm concerned not just that the quality of GCC 12
>>>> diagnostics has been eroding, but also that it seems to be not
>>>> just acceptable but expected.
>>> You make a very good point.  It is certainly not my intention to make
>>> life difficult for the warning maintainers, but I'm afraid I don't
>>> have sufficient knowledge in the area to improve them.
>>>
>>> There may be some low hanging fruit though.  At least in the warnings
>>> that use the ranger, there's no reason to run these passes so late in
>>> the pipeline.  You could run the warning code as early as you want,
>>> insofar as SSA is available and the CFG has been built.  Heck, you may
>>> even be able to run at -O0, though we may need some sort of value
>>> numbering.  I believe Richi even suggested this a while back.
>> Running them later in the pipeline is to take advantage of the
>> optimizers removing dead and unreachable code as much as possible. In
>> fact, that's critical to -Wuninitialized.  Optimizing away unreachable
>> paths  to avoid Wuninitialized false positives has been the major driver
>> of jump threading improvements for the last 15 years.
> 
> Ughh, that's unfortunate.  We're gonna have to come up with
> improvements to the Wuninitialized code, or a different paradigm
> altogether.  I'm afraid this will only get worse.
> 
> It is a bit ironic that jump threading helps reduce Wuninitialized
> false positives, but yet too much of it causes even more false
> positives.

For best results they need to work together.  That was also my
point.  All middle end warnings necessarily rely on optimizers
for one thing or another.  When we change one such optimization
and break the assumptions the warnings make (right or wrong),
the warnings break too.  To minimize the fallout we need to
do two things: decouple them as much as possible (minimize
the assumptions) and, when we change the optimizers, we
might need to update the warnings.  If we don't, then yes,
things are going to keep getting worse.

As an aside, in GCC 12 a good number of middle end warnings
already do run at -O0: all those in gimple-ssa-warn-access.c.
I'd like to see gimple-ssa-array-bounds invoked from the access
pass too (instead of from VRP), and eventually -Wrestrict as well.
I'm not sure about the strlen/sprintf warnings; those might need
to stay where they are because they run as part of the optimizers
there.

(By the way, I don't see range info in the access pass at -O0.
Should I?)

Martin