[Bug target/49263] SH Target: underutilized "TST #imm, R0" instruction

["klepikov.alex+bugs at gmail dot com" <gcc-bugzilla@gcc.gnu.org>]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=49263

--- Comment #35 from Alexander Klepikov <klepikov.alex+bugs at gmail dot com> ---
(In reply to Oleg Endo from comment #34)
> Bit-tests of char and unsigned char should be covered by the test-suite and
> should work -- at least originally.  However, what might be triggering this
> problem is the '== FLAG' comparison.  When I was working on this issue I
> only used '== 0' or '!= 0' comparison.  I can imagine that your test code
> triggers some other middle end optimizations and hence we get this.

Yes, I am sure that the problem is the '== FLAG' comparison. Before I reported
that bug, I tried to bypass it and this macro does not produce shift
instructions even on GCC 4.7:

#define BIT_MASK_IS_SET_(VALUE, BITMASK)\
        ({int _value = VALUE & BITMASK,\
        _result;\
        if (_value == BITMASK){\
                _result = 1;\
        }\
        else {\
                _result = 0;\
        }\
        _result;})

So this is definitely the comparison.

> 
> Can you try to rewrite your test code to something like this?
> 
> unsigned int f(char v){
>     return (v & FLAG) != 0;
> }
> 
> ... and see if it generates the tst instruction as expected?
> 

As I understand, you meant the following (I added new functions at the end of
file):

$ cat f.c
#define ADDR 0xFFFF0000
#define P ((unsigned char *)ADDR)
#define FLAG 0x40
#define S 7

unsigned char f_char_var(char v){
    return (v & FLAG) == FLAG;
}

unsigned char f_unsigned_char_var(unsigned char v){
    return (v & FLAG) == FLAG;
}

unsigned char f_symbol(void){
    return (*P & FLAG) == FLAG;
}

unsigned char f_symbol_zero(void){
    return (*P & FLAG) == 0;
}

unsigned char f_symbol_non_zero(void){
    return (*P & FLAG) != 0;
}

Compiler flags: -c -mrenesas -m2e -mb -O -fno-toplevel-reorder

With patch disabled:

$ cat f_clean.s
        .file   "f.c"
        .text
        .text
        .align 1
        .global _f_char_var
        .type   _f_char_var, @function
_f_char_var:
        sts.l   pr,@-r15
        mov.l   .L3,r1
        jsr     @r1
        exts.b  r4,r4
        mov     r4,r0
        and     #1,r0
        lds.l   @r15+,pr
        rts
        nop
.L4:
        .align 2
.L3:
        .long   ___ashiftrt_r4_6
        .size   _f_char_var, .-_f_char_var
        .align 1
        .global _f_unsigned_char_var
        .type   _f_unsigned_char_var, @function
_f_unsigned_char_var:
        sts.l   pr,@-r15
        mov.l   .L7,r1
        jsr     @r1
        exts.b  r4,r4
        mov     r4,r0
        and     #1,r0
        lds.l   @r15+,pr
        rts
        nop
.L8:
        .align 2
.L7:
        .long   ___ashiftrt_r4_6
        .size   _f_unsigned_char_var, .-_f_unsigned_char_var
        .align 1
        .global _f_symbol
        .type   _f_symbol, @function
_f_symbol:
        sts.l   pr,@-r15
        mov.l   .L11,r1
        mov.b   @r1,r4
        mov.l   .L12,r1
        jsr     @r1
        nop
        mov     r4,r0
        and     #1,r0
        lds.l   @r15+,pr
        rts
        nop
.L13:
        .align 2
.L11:
        .long   -65536
.L12:
        .long   ___ashiftrt_r4_6
        .size   _f_symbol, .-_f_symbol
        .align 1
        .global _f_symbol_zero
        .type   _f_symbol_zero, @function
_f_symbol_zero:
        mov.l   .L15,r1
        mov.b   @r1,r0
        tst     #64,r0
        rts
        movt    r0
.L16:
        .align 2
.L15:
        .long   -65536
        .size   _f_symbol_zero, .-_f_symbol_zero
        .align 1
        .global _f_symbol_non_zero
        .type   _f_symbol_non_zero, @function
_f_symbol_non_zero:
        sts.l   pr,@-r15
        mov.l   .L19,r1
        mov.b   @r1,r4
        mov.l   .L20,r1
        jsr     @r1
        nop
        mov     r4,r0
        and     #1,r0
        lds.l   @r15+,pr
        rts
        nop
.L21:
        .align 2
.L19:
        .long   -65536
.L20:
        .long   ___ashiftrt_r4_6
        .size   _f_symbol_non_zero, .-_f_symbol_non_zero
        .ident  "GCC: (GNU) 12.3.0"

With patch enabled:

$ cat f.s
        .file   "f.c"
        .text
        .text
        .align 1
        .global _f_char_var
        .type   _f_char_var, @function
_f_char_var:
        mov     r4,r0
        tst     #64,r0
        mov     #-1,r0
        rts
        negc    r0,r0
        .size   _f_char_var, .-_f_char_var
        .align 1
        .global _f_unsigned_char_var
        .type   _f_unsigned_char_var, @function
_f_unsigned_char_var:
        mov     r4,r0
        tst     #64,r0
        mov     #-1,r0
        rts
        negc    r0,r0
        .size   _f_unsigned_char_var, .-_f_unsigned_char_var
        .align 1
        .global _f_symbol
        .type   _f_symbol, @function
_f_symbol:
        mov.l   .L4,r1
        mov.b   @r1,r0
        tst     #64,r0
        mov     #-1,r0
        rts
        negc    r0,r0
.L5:
        .align 2
.L4:
        .long   -65536
        .size   _f_symbol, .-_f_symbol
        .align 1
        .global _f_symbol_zero
        .type   _f_symbol_zero, @function
_f_symbol_zero:
        mov.l   .L7,r1
        mov.b   @r1,r0
        tst     #64,r0
        rts
        movt    r0
.L8:
        .align 2
.L7:
        .long   -65536
        .size   _f_symbol_zero, .-_f_symbol_zero
        .align 1
        .global _f_symbol_non_zero
        .type   _f_symbol_non_zero, @function
_f_symbol_non_zero:
        mov.l   .L10,r1
        mov.b   @r1,r0
        tst     #64,r0
        mov     #-1,r0
        rts
        negc    r0,r0
.L11:
        .align 2
.L10:
        .long   -65536
        .size   _f_symbol_non_zero, .-_f_symbol_non_zero
        .ident  "GCC: (GNU) 12.3.0"

> I'm not sure what the purpose of the '-mdisable-dynshift' option would be
> here though.  For '-m2e' TARGET_DYNSHIFT is already 'false'.  So the option
> seems misnamed.

I choose that name because I wanted to disable dynamic shift instructions for
all CPUs. I did not hope that it will affect SH-2E code in such way.

I can rewrite the patch so that it only affects CPUs that do not support
dynamic shifts and disables library call for dynamic shifts. I'll do it anyway
because I need it badly. How do you think, what name of option would be better:
'-mdisable-dynshift-libcall' or '-mhw-shift'? Or if you want, please suggest
another one. Thank you!