From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: by sourceware.org (Postfix, from userid 48) id 9EE0B3858CDB; Wed, 24 May 2023 11:40:47 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 9EE0B3858CDB DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gcc.gnu.org; s=default; t=1684928447; bh=BgOrpwI1/HRJD3P2sf62owG6qEMITzNxlbzvQAl6kH4=; h=From:To:Subject:Date:In-Reply-To:References:From; b=YUAAwdeftN4CwSPqD72jXJ0onN33G8aCtihTMOkPD5NSLzO/Vk/V2oR2YzDtsyivX CZbrMT29x3O2m63B2GeeoBhXaEGiUPC/r6o5reWPt3bvexsGqU4x+dO0PJR6M4ORWE 0Pvqru38giUYnbbOqrngGZ6oxpY0MCYbuQkSdC2E= From: "klepikov.alex+bugs at gmail dot com" To: gcc-bugs@gcc.gnu.org Subject: [Bug target/49263] SH Target: underutilized "TST #imm, R0" instruction Date: Wed, 24 May 2023 11:40:45 +0000 X-Bugzilla-Reason: CC X-Bugzilla-Type: changed X-Bugzilla-Watch-Reason: None X-Bugzilla-Product: gcc X-Bugzilla-Component: target X-Bugzilla-Version: 4.6.1 X-Bugzilla-Keywords: X-Bugzilla-Severity: enhancement X-Bugzilla-Who: klepikov.alex+bugs at gmail dot com X-Bugzilla-Status: REOPENED X-Bugzilla-Resolution: X-Bugzilla-Priority: P3 X-Bugzilla-Assigned-To: olegendo at gcc dot gnu.org X-Bugzilla-Target-Milestone: --- X-Bugzilla-Flags: X-Bugzilla-Changed-Fields: Message-ID: In-Reply-To: References: Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Bugzilla-URL: http://gcc.gnu.org/bugzilla/ Auto-Submitted: auto-generated MIME-Version: 1.0 List-Id: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=3D49263 --- Comment #35 from Alexander Klepikov --- (In reply to Oleg Endo from comment #34) > Bit-tests of char and unsigned char should be covered by the test-suite a= nd > should work -- at least originally. However, what might be triggering th= is > problem is the '=3D=3D FLAG' comparison. When I was working on this issu= e I > only used '=3D=3D 0' or '!=3D 0' comparison. I can imagine that your tes= t code > triggers some other middle end optimizations and hence we get this. Yes, I am sure that the problem is the '=3D=3D FLAG' comparison. Before I r= eported 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 =3D VALUE & BITMASK,\ _result;\ if (_value =3D=3D BITMASK){\ _result =3D 1;\ }\ else {\ _result =3D 0;\ }\ _result;}) So this is definitely the comparison. >=20 > Can you try to rewrite your test code to something like this? >=20 > unsigned int f(char v){ > return (v & FLAG) !=3D 0; > } >=20 > ... and see if it generates the tst instruction as expected? >=20 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) =3D=3D FLAG; } unsigned char f_unsigned_char_var(unsigned char v){ return (v & FLAG) =3D=3D FLAG; } unsigned char f_symbol(void){ return (*P & FLAG) =3D=3D FLAG; } unsigned char f_symbol_zero(void){ return (*P & FLAG) =3D=3D 0; } unsigned char f_symbol_non_zero(void){ return (*P & FLAG) !=3D 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 opti= on > seems misnamed. I choose that name because I wanted to disable dynamic shift instructions f= or 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 any= way because I need it badly. How do you think, what name of option would be bet= ter: '-mdisable-dynshift-libcall' or '-mhw-shift'? Or if you want, please suggest another one. Thank you!=