Re: [PATCH v2] combine: perform jump threading at the end

[Jeff Law <law at redhat dot com>]

On 09/05/2018 06:11 AM, Richard Biener wrote:
> On Wed, Sep 5, 2018 at 2:01 PM Ilya Leoshkevich <iii@linux.ibm.com> wrote:
>>
>> gcc/ChangeLog:
>>
>> 2018-09-05  Ilya Leoshkevich  <iii@linux.ibm.com>
>>
>>         PR target/80080
>>         * combine.c (rest_of_handle_combine): Perform jump threading.
>>
>> gcc/testsuite/ChangeLog:
>>
>> 2018-09-05  Ilya Leoshkevich  <iii@linux.ibm.com>
>>
>>         PR target/80080
>>         * gcc.target/s390/pr80080-4.c: New test.
>> ---
>>  gcc/combine.c                             | 10 ++++++++--
>>  gcc/testsuite/gcc.target/s390/pr80080-4.c | 16 ++++++++++++++++
>>  2 files changed, 24 insertions(+), 2 deletions(-)
>>  create mode 100644 gcc/testsuite/gcc.target/s390/pr80080-4.c
>>
>> diff --git a/gcc/combine.c b/gcc/combine.c
>> index a2649b6d5a1..818b4c5b77d 100644
>> --- a/gcc/combine.c
>> +++ b/gcc/combine.c
>> @@ -14960,10 +14960,16 @@ rest_of_handle_combine (void)
>>         free_dominance_info (CDI_DOMINATORS);
>>        timevar_push (TV_JUMP);
>>        rebuild_jump_labels (get_insns ());
>> -      cleanup_cfg (0);
>> -      timevar_pop (TV_JUMP);
>>      }
>>
>> +  /* Combining insns can change basic blocks in a way that they end up
>> +     containing a single jump_insn. This creates an opportunity to improve code
>> +     with jump threading.  */
>> +  cleanup_cfg (CLEANUP_THREADING);
>> +
>> +  if (rebuild_jump_labels_after_combine)
>> +    timevar_pop (TV_JUMP);
> 
> cleanup_cfg pushes its own timevar so it doesn't make sense to try covering it
> with TV_JUMP.  And rebuild_jump_labels immediately pushes TV_REBUILD_JUMP.
> 
> So I suggest to remove the timevar_push/pop of TV_JUMP here.
> 
> No comment in general about the change, maybe we can detect transforms that
> make jump-threading viable and conditionalize that properly?  Note the only
> setter of CLEANUP_THREADING guards it with flag_thread_jumps so maybe better
> do it above as well (avoids cost at -O0 for example).
The sad thing is I thought we'd killed the RTL jump threading code eons ago.

THe RTL jump threading code tries to prove that the target block has no
side effects and that we can statically determine the true/false
condition for the conditional branch at the end of the block.

This is (of course) much easier to do when the target block has no insns
other than the conditional branch.  So perhaps only do this when the
target block has just the conditional?

Hard to know if that'd work here since RTL wasn't posted.

Jeff