Hi Bin,

On 5/16/19 11:00 AM, Bin.Cheng wrote:
> On Thu, May 16, 2019 at 11:50 PM Bill Schmidt <wschm...@linux.ibm.com> wrote:
>> Thanks, Bin and Richard -- I am out of the office until Tuesday, so will 
>> review
>> when I get back.  Bin, please CC me on SLSR patches as otherwise I might miss
>> them.  Thanks!
> Thanks for helping.  Will do it next time.
>
> Thanks,
> bin
>> Bill
>>
>>
>> On 5/16/19 6:37 AM, Richard Biener wrote:
>>> On Wed, May 15, 2019 at 6:30 AM bin.cheng <bin.ch...@linux.alibaba.com> 
>>> wrote:
>>>> Hi,
>>>> As noted in PR57534 comment #33, SLSR currently doesn't strength reduce 
>>>> memory
>>>> references in reported cases, which conflicts with its comment at the 
>>>> beginning of file.
>>>> The main reason is in functions slsr_process_ref and restructure_reference 
>>>> which
>>>> rejects MEM_REF by handled_compoenent_p in the first place.  This patch 
>>>> identifies
>>>> and creates CAND_REF for MEM_REF by restructuring base/offset.
>>>>
>>>> Note the patch only affects cases with multiple reducible MEM_REF.
>>>>
>>>> Also note, with this patch, [base + cst_offset] addressing mode would be 
>>>> generally
>>>> preferred.  I need to adjust three existing tests:
>>>> * gcc.dg/tree-ssa/isolate-5.c
>>>> * gcc.dg/tree-ssa/ssa-hoist-4.c
>>>> Though address computation is reduced out of memory reference, the 
>>>> generated
>>>> assembly is not worsened.
>>>>
>>>> * gcc.dg/tree-ssa/slsr-3.c
>>>> The generated assembly has two more instructions:
>>>> <       movslq  %edx, %rcx
>>>> <       movl    (%rsi,%rcx,4), %r9d
>>>> <       leaq    0(,%rcx,4), %rax
>>>> <       leal    2(%r9), %r8d
>>>> <       movl    %r8d, (%rdi,%rcx,4)
>>>> <       movl    4(%rsi,%rax), %ecx
>>>> <       addl    $2, %ecx
>>>> <       movl    %ecx, 4(%rdi,%rax)
>>>> <       movl    8(%rsi,%rax), %ecx
>>>> <       addl    $2, %ecx
>>>> <       movl    %ecx, 8(%rdi,%rax)
>>>> <       movl    12(%rsi,%rax), %ecx
>>>> <       addl    $2, %ecx
>>>> <       movl    %ecx, 12(%rdi,%rax)
>>>> ---
>>>>>       movslq  %edx, %rax
>>>>>       salq    $2, %rax
>>>>>       addq    %rax, %rsi
>>>>>       addq    %rax, %rdi
>>>>>       movl    (%rsi), %eax
>>>>>       addl    $2, %eax
>>>>>       movl    %eax, (%rdi)
>>>>>       movl    4(%rsi), %eax
>>>>>       addl    $2, %eax
>>>>>       movl    %eax, 4(%rdi)
>>>>>       movl    8(%rsi), %eax
>>>>>       addl    $2, %eax
>>>>>       movl    %eax, 8(%rdi)
>>>>>       movl    12(%rsi), %eax
>>>>>       addl    $2, %eax
>>>>>       movl    %eax, 12(%rdi)
>>>> Seems to me this is not deteriorating and "salq" can be saved by two 
>>>> forward propagation.
>>>>
>>>> Bootstrap and test on x86_64, any comments?
>>> The idea is good I think and the result above as well.  Leaving for Bill
>>> to have a look as well, otherwise OK.

Once again, I lost the original patch due to some glitch in my mail server,
so I apologize for not quoting the patch.

I'm initially okay with all of it until we get to the "stride_cand" treatment in
the second half of restructure_base_offset.  It looks to me like the 
transformation
here is not necessarily a good one for architectures that don't support x86's 
elaborate set of addressing modes.  Can you convince me otherwise?

I'm not certain that any of the test cases that have been modified won't be
degradations for other architectures.  Did you do any code gen comparisons 
there?
Seems like we need results from Power and Aarch64 at a minimum.

Perhaps the existing MEM_REFs are already ugly enough that the replacement 
doesn't
make things worse; I just can't prove it to myself without examples.  Could you
please run some experiments?

This is otherwise okay if we can get past that concern.

Thanks!
Bill

>>>
>>> Thanks,
>>> Richard.
>>>
>>>> Thanks,
>>>> bin
>>>>
>>>> 2019-05-15  Bin Cheng  <bin.ch...@linux.alibaba.com>
>>>>
>>>>         PR tree-optimization/57534
>>>>         * gimple-ssa-strength-reduction.c (restructure_base_offset): New.
>>>>         (restructure_reference): Call restructure_base_offset when offset 
>>>> is
>>>>         NULL.
>>>>         (slsr_process_ref): Handle MEM_REF.
>>>>
>>>> 2018-05-15  Bin Cheng  <bin.ch...@linux.alibaba.com>
>>>>
>>>>         PR tree-optimization/57534
>>>>         * gcc.dg/tree-ssa/pr57534.c: New test.
>>>>         * gcc.dg/tree-ssa/isolate-5.c: Adjust checking strings.
>>>>         * gcc.dg/tree-ssa/slsr-3.c: Ditto.
>>>>         * gcc.dg/tree-ssa/ssa-hoist-4.c: Ditto.

Reply via email to