On 25/01/2024 10:29, Maxim Kuvyrkov wrote:
> After fwprop improvement in r14-8319-g86de9b66480, codegen in
> bics_3.c test changed from "bics" to "bic" instruction, with
> the overall instruction stream remaining at the same quality.
>
> This patch makes the scan-assembler directive accept both
> "bics" and "bic".
>
> BEFORE r14-8319-g86de9b66480:
> bicsr0, r0, r1 @ 9 [c=4 l=4] *andsi_notsi_si_compare0_scratch
> mov r0, #1 @ 23[c=4 l=4] *thumb2_movsi_vfp/1
> it eq
> moveq r0, #0 @ 26[c=8 l=4] *p *thumb2_movsi_vfp/2
> bx lr @ 29[c=8 l=4] *thumb2_return
>
> AFTER r14-8319-g86de9b66480:
> bic r0, r0, r1 @ 8 [c=4 l=4] andsi_notsi_si
> subsr0, r0, #0 @ 22[c=4 l=4] cmpsi2_addneg/0
> it ne
> movne r0, #1 @ 23[c=8 l=4] *p *thumb2_movsi_vfp/2
> bx lr @ 26[c=8 l=4] *thumb2_return
>
> gcc/testsuite/ChangeLog:
>
> PR target/113542
> * gcc.target/arm/bics_3.c: Update scan-assembler directive.
> ---
> gcc/testsuite/gcc.target/arm/bics_3.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/gcc/testsuite/gcc.target/arm/bics_3.c
> b/gcc/testsuite/gcc.target/arm/bics_3.c
> index e056b264e15..c5bed3c92d2 100644
> --- a/gcc/testsuite/gcc.target/arm/bics_3.c
> +++ b/gcc/testsuite/gcc.target/arm/bics_3.c
> @@ -35,6 +35,6 @@ main (void)
>return 0;
> }
>
> -/* { dg-final { scan-assembler-times "bics\tr\[0-9\]+, r\[0-9\]+, r\[0-9\]+"
> 2 } } */
> -/* { dg-final { scan-assembler-times "bics\tr\[0-9\]+, r\[0-9\]+, r\[0-9\]+,
> .sl #2" 1 } } */
> +/* { dg-final { scan-assembler-times "bics?\tr\[0-9\]+, r\[0-9\]+,
> r\[0-9\]+" 2 } } */
> +/* { dg-final { scan-assembler-times "bics?\tr\[0-9\]+, r\[0-9\]+,
> r\[0-9\]+, .sl #2" 1 } } */
>
The test was added (r6-823-g0454e698401a3e) specifically to check that a BICS
instruction was being generated. Whether or not that is right is somewhat
debatable, but this change seems to be papering over a different issue.
Either we should generate BICS, making this change incorrect, or we should
disable the test for thumb code on the basis that this isn't really a win.
But really, we should fix the compiler to do better here. We really want
something like
BICS r0, r0, r1 // r0 is 0 or non-zero
MOVNE r0, #1 // convert all non-zero to 1
in Arm state (ie using the BICS instruction to set the result to zero); and in
thumb2, perhaps something like:
BICS r0, r0, r1
ITne
MOVNE r0, #1
or maybe even better:
BIC r0, r0, r1
SUBS r1, r0, #1
SBC r0, r0, r1
which is slightly better than BICS because SUBS breaks a condition-code chain
(all the flag bits are set).
There are similar quality issues for other NE(arith-op, 0) cases; we just don't
have tests for those.
R.
