On Mon, May 2, 2016 at 9:25 PM, H.J. Lu <hjl.to...@gmail.com> wrote:
> On Sat, Apr 30, 2016 at 2:28 PM, Patrick Palka <patr...@parcs.ath.cx> wrote:
>> `On Fri, Apr 29, 2016 at 3:15 PM, Jeff Law <l...@redhat.com> wrote:
>>> On 04/19/2016 11:50 AM, Patrick Palka wrote:
>>>
>>>> 1. This patch introduces a "regression" in gcc.dg/tree-ssa/ssa-thread-11.c
>>>> in that we no longer perform FSM threading during vrp2 but instead we
>>>> detect two new jump threading opportunities during vrp1. Not sure if
>>>> the new code is better but it is shorter. I wonder how this should be
>>>> resolved...
>>>
>>> Definitely not a regression. As you note we thread two jumps earlier
>>> utilizing your new code. With the old code we're dependent upon other
>>> simplifications occurring which eventually exposes the FSM threads that the
>>> test is checking for in vrp2.
>>>
>>> I think we just want to remove the test for FSM jump threads in VRP2. We get
>>> coverage for your test via ssa-thread-14. That should just leave the
>>> verification that we do not have irreducible loops at the end of VRP2 in
>>> ssa-thread-11. I'll make that change.
>>>
>>> I do see what appears to be a missed jump thread, but that's not affected
>>> positively or negatively by your change. There's a reasonable chance it's
>>> only exposed by normal thread jumps in VRP2 and since we don't iterate, it's
>>> left in the IL. I haven't analyzed it in detail, but my hope is that when I
>>> pull the backwards threading out into its own pass that we'll start picking
>>> up more of these secondary effects.
>>
>> Interesting info. I also spotted another minor optimization
>> opportunity within this test case, although more related to vrp than
>> to jump threading. It would require iterating over the use statements
>> of a subexpression within a conditional, and it turns out that VRP
>> already does this so it's only a matter of adding another case to test
>> for during each iteration. I'll post a patch when it's ready.
>>
>>>
>>>
>>>>
>>>> gcc/ChangeLog:
>>>>
>>>> * tree-ssa-threadedge.c (simplify_control_stmt_condition): Split
>>>> out into ...
>>>> (simplify_control_stmt_condition_1): ... here. Recurse into
>>>> BIT_AND_EXPRs and BIT_IOR_EXPRs.
>>>>
>>>> gcc/testsuite/ChangeLog:
>>>>
>>>> * gcc.dg/tree-ssa/ssa-thread-14.c: New test.
>>>> ---
>>>
>>> I fixed a few formatting nits (too long lines).
>>>
>
> This one fails on Linux/x86-64 and Linux/x86. This patch fixes it. OK
> for trunk?
Ok.
Richard.
>
> --
> H.J.
> ---
> diff --git a/gcc/testsuite/gcc.dg/tree-ssa/ssa-thread-14.c
> b/gcc/testsuite/gcc.dg/tree-ssa/ssa-thread-14.c
> index db9ed3b..e2ac2f7 100644
> --- a/gcc/testsuite/gcc.dg/tree-ssa/ssa-thread-14.c
> +++ b/gcc/testsuite/gcc.dg/tree-ssa/ssa-thread-14.c
> @@ -1,5 +1,5 @@
> /* { dg-do compile } */
> -/* { dg-additional-options "-O2 -fdump-tree-vrp" } */
> +/* { dg-additional-options "-O2 -fdump-tree-vrp-details" } */
> /* { dg-final { scan-tree-dump-times "Threaded jump" 8 "vrp1" } } */
>
> void foo (void);
