Re: [2/5] C-SKY port: Backend implementation

[Sandra Loosemore <sandra@codesourcery.com>]

On 07/26/2018 12:06 AM, 瞿仙淼 wrote:
> 
> I wrote a case to reproduce this problem on C-SKY. C code is as follows:
> -----------------------------------------------------------------------
> int e1, e2;
> 
> void func (int a, int b, int c, int d, int f, int g)
> {
>    e1 = a > b ? f : g;
>    e2 = a > b ? c : d;
> 
>    return;
> }
> -----------------------------------------------------------------------
> 
> compile to assembler with option “-O3 -S” :
> -----------------------------------------------------------------------
> func:
>    cmplt a1, a0
>    ld.w  t1, (sp, 0)
>    ld.w  t0, (sp, 4)
>    movt  t0, t1
>    cmplt a1, a0
>    movt  a3, a2
>    lrw a1, e2
>    lrw a2, e1
>    st.w  a3, (a1, 0)
>    st.w  t0, (a2, 0)
>    rts
> -----------------------------------------------------------------------
> There is an extra “cmplt a1, a0" in the above code without cse_cc. This situation mainly occurs when a relatively short branch jump is converted into a conditional execution instruction. And the CSE pass can not reduce the same conditional comparison instruction . Here is the rtx sequence after “cse2” pass.
> 
> -----------------------------------------------------------------------
> (insn 28 13 29 2 (set (reg:CC 33 c)
>          (gt:CC (reg/v:SI 77 [ a ])
>              (reg/v:SI 78 [ b ]))) func.c:5 1099 {*cmpgtsi}
>       (nil))
> (insn 29 28 30 2 (set (reg/v:SI 82 [ g ])
>          (if_then_else:SI (eq (reg:CC 33 c)
>                  (const_int 0 [0]))
>              (reg/v:SI 82 [ g ])
>              (reg/v:SI 81 [ f ]))) func.c:5 983 {movf}
>       (expr_list:REG_DEAD (reg/v:SI 81 [ f ])
>          (expr_list:REG_DEAD (reg:CC 33 c)
>              (nil))))
> (insn 30 29 31 2 (set (reg:CC 33 c)
>          (gt:CC (reg/v:SI 77 [ a ])
>              (reg/v:SI 78 [ b ]))) func.c:5 1099 {*cmpgtsi}
>       (expr_list:REG_DEAD (reg/v:SI 78 [ b ])
>          (expr_list:REG_DEAD (reg/v:SI 77 [ a ])
>              (nil))))
> (insn 31 30 18 2 (set (reg/v:SI 80 [ d ])
>          (if_then_else:SI (eq (reg:CC 33 c)
>                  (const_int 0 [0]))
>              (reg/v:SI 80 [ d ])
>              (reg/v:SI 79 [ c ]))) func.c:5 983 {movf}
>       (expr_list:REG_DEAD (reg/v:SI 79 [ c ])
>          (expr_list:REG_DEAD (reg:CC 33 c)
>              (nil))))
> -----------------------------------------------------------------------
> 
> It doesn't seem to check the same conditional comparison instruction .So I wrote this to solve this problem, but I am not sure if this is the best way : )
> 
> PS, the same conditional comparison instruction cannot be reduced with the latest version gcc with C-SKY because I just insert the “cse_cc” after “cse1”, when I insert after “cse2”, this problem can be solved very well.

Thanks, this is very helpful.  I've verified this and I'm moving the 
pass as you suggest, adding a more detailed comment to the source to 
explain what the pass is for, and adding your test case to the 
testsuite.  This will be included when I resubmit the patches to address 
other review comments too.

Jeff, does that adequately address your concerns about whether the pass 
is useful?

-Sandra