[Bug target/52898] New: SH Target: Inefficient comparison of DImode and immediate constant

[Bug target/52898] New: SH Target: Inefficient comparison of DImode and immediate constant

[olegendo at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=52898

             Bug #: 52898
           Summary: SH Target: Inefficient comparison of DImode and
                    immediate constant
    Classification: Unclassified
           Product: gcc
           Version: 4.8.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: target
        AssignedTo: unassigned@gcc.gnu.org
        ReportedBy: olegendo@gcc.gnu.org
            Target: sh*-*-*


The following ...

int test (long long a)
{
  return a == 40;
}

compiled with -O2, -O3, -Os:

        mov     #0,r2
        cmp/eq  r2,r4
        bf/s   .L38
        mov     #40,r3
        cmp/eq  r3,r5
.L38:
        rts
        movt    r0


would be better as:

    mov     #40,r3
    tst     r4,r4   ! use tst to compare against zero
    bf      .L38    ! use zero-displacement branch
    cmp/eq  r3,r5
.L38:
    rts
    movt    r0




Using built-in specs.
COLLECT_GCC=sh-elf-gcc
COLLECT_LTO_WRAPPER=/usr/local/libexec/gcc/sh-elf/4.8.0/lto-wrapper
Target: sh-elf
Configured with: ../gcc-trunk/configure --target=sh-elf --prefix=/usr/local
--enable-languages=c,c++ --enable-multilib --disable-libssp --disable-nls
--disable-werror --enable-lto --with-newlib --with-gnu-as --with-gnu-ld
--with-system-zlib
Thread model: single
gcc version 4.8.0 20120406 (experimental) (GCC)

[Bug target/52898] SH Target: Inefficient DImode comparisons

[olegendo at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=52898

--- Comment #1 from Oleg Endo <olegendo at gcc dot gnu.org> 2012-04-11 22:48:18 UTC ---
Created attachment 27138
  --> http://gcc.gnu.org/bugzilla/attachment.cgi?id=27138
Examples

In addition to the original case, I've noticed that there are more weird things
happening with DImode comparisons in general.  The attached test cases do not
cover all the possible combinations, but basically the -mcbranchdi and
-mcmpeqdi options in combination with -O2 and -Os seem not to behave as
originally intended.
With -O2 the cmpeqdi_t pattern is never used no matter which options are
specified.  I think it is the split below the cmpeqdi_t insn in sh.md, that
does this.  The code of the cmpeqdi_t insn seems already quite optimal, except
that the first cmp / tst insn should be emitted separately to get potentially
better scheduling.

Moreover, there is an unnamed tstdi_t pattern which never gets used by combine.
 What happens there, is that the and:DI is split by the middle-end into 2x
and:SI at a very early stage.  I think this is because there is no and:DI insn
defined.

[Bug target/52898] SH Target: Inefficient DImode comparisons

[olegendo at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=52898

Oleg Endo <olegendo at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |kkojima at gcc dot gnu.org

--- Comment #2 from Oleg Endo <olegendo at gcc dot gnu.org> 2012-04-11 22:52:14 UTC ---
Kaz, do you happen to know the background of the -mcbranchdi -mcmpeqdi options
and the respective patterns?  I'm wondering, why they are not enabled by
default at least for SH4, since they seem to try to utilize the
zero-displacement branch where possible ...

[Bug target/52898] SH Target: Inefficient DImode comparisons

[kkojima at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=52898

--- Comment #3 from Kazumoto Kojima <kkojima at gcc dot gnu.org> 2012-04-12 01:13:15 UTC ---
(In reply to comment #2)
I don't know about their history.  -mcbranchdi is enabled by default,
though.  See gcc/common/config/sh/sh-common.c:sh_option_optimization_table.
Unfortunately, it looks -mcmpeqdi causes many new failures on trunk.

[Bug target/52898] SH Target: Inefficient DImode comparisons

[olegendo at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=52898

--- Comment #4 from Oleg Endo <olegendo at gcc dot gnu.org> 2012-04-12 06:19:44 UTC ---
(In reply to comment #3)
> (In reply to comment #2)
> I don't know about their history.  -mcbranchdi is enabled by default,
> though.  See gcc/common/config/sh/sh-common.c:sh_option_optimization_table.

Ah, I see.  Now I'm even more confused than before :)
If -mcbranchdi is enabled by default, I would not expect to see any changes in
the generated code at all in some cases .... 

> Unfortunately, it looks -mcmpeqdi causes many new failures on trunk.

I would propose to deprecate -mcbranchdi and -mcmpeqdi and hard-code the
'enabled' behavior, i.e. remove all the non-mcbranchdi and non-mcmpeqdi code
paths in the back end code (and fixing the fails of course ...)

[Bug target/52898] SH Target: Inefficient DImode comparisons

[olegendo at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=52898

--- Comment #5 from Oleg Endo <olegendo at gcc dot gnu.org> 2012-04-12 06:59:47 UTC ---
(In reply to comment #4)
> 
> I would propose to deprecate -mcbranchdi and -mcmpeqdi and hard-code the
> 'enabled' behavior, i.e. remove all the non-mcbranchdi and non-mcmpeqdi code
> paths in the back end code (and fixing the fails of course ...)

Sorry, what I rather meant is: deprecate -mcbranchdi and -mcmpeqdi,
simplify/unify the code for those in the back end in order to implement some
reasonable default behavior without the need for -m options.

[Bug target/52898] SH Target: Inefficient DImode comparisons

[olegendo at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=52898

--- Comment #6 from Oleg Endo <olegendo at gcc dot gnu.org> 2012-09-01 22:47:26 UTC ---
I had a closer look at what is going on with cbranchdi and cmpeqdi_t.
If cmpeqdi_t is enabled through the option -mcmpeqdi it will trigger the 'if
(TARGET_CMPEQDI_T)' code paths in expand_cbranchdi4.  These will then emit the
cmpeqdi_t insn for EQ and NE cmp codes.
The problem is then that for DImode comparisons with a constant there are no
patterns that match e.g.

(set (reg:SI T_REG)
     (eq:SI (match_operand:DI 0 "arith_reg_operand" "r")
            (const_int 2)))

As far as I understand, basically what the cbranchdi pattern in the
expand_cbranchdi4 function is trying to do is to fold the branches to turn
something like this:

   cmp/eq  r0,r2
   bf      0f
   cmp/eq  r1,r3
0:
   b{f|t}  <target>

into this:

   cmp/eq  r0,r2
   b{f|t}  <target>
   cmp/eq  r1,r2
   b{f|t}  <target>

The function output_branchy_insn tries to do the same, but when emitting the
asm output.

When zero-displacement branches are fast, all DImode comparisons (if I'm not
mistaken) can be done in constant 2 cycles in either 3 or 4 opcodes + the
actual branch.  Probably it would also be better to split out the first
comparison insn from the fixed asm output and make the second comparison
conditional (similar to the abs patterns).

[Bug target/52898] SH Target: Inefficient DImode comparisons

[olegendo at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=52898

--- Comment #7 from Oleg Endo <olegendo at gcc dot gnu.org> ---
Kaz, I'd like to deprecate the mcbranchdi and mcmpeqdi options in 4.9 and make
the current default values fixed as follows:

mcbranchdi is usually enabled (it gets disabled in some SH5 case in
sh_option_override).
mcmpeqdi is always disabled.

As a side effect, it would also "fix" PR 51697 (mcbranchdi will not be disabled
for -Os).

Are you OK with the following patch for 4.9?

Index: gcc/common/config/sh/sh-common.c
===================================================================
--- gcc/common/config/sh/sh-common.c    (revision 205756)
+++ gcc/common/config/sh/sh-common.c    (working copy)
@@ -34,7 +34,6 @@
     { OPT_LEVELS_1_PLUS_SPEED_ONLY, OPT_mdiv_, "inv:minlat", 1 },
     { OPT_LEVELS_SIZE, OPT_mdiv_, SH_DIV_STR_FOR_SIZE, 1 },
     { OPT_LEVELS_0_ONLY, OPT_mdiv_, "", 1 },
-    { OPT_LEVELS_SIZE, OPT_mcbranchdi, NULL, 0 },
     /* We can't meaningfully test TARGET_SHMEDIA here, because -m
        options haven't been parsed yet, hence we'd read only the
        default.  sh_target_reg_class will return NO_REGS if this is
Index: gcc/doc/invoke.texi
===================================================================
--- gcc/doc/invoke.texi    (revision 205756)
+++ gcc/doc/invoke.texi    (working copy)
@@ -959,7 +959,7 @@
 -mindexed-addressing -mgettrcost=@var{number} -mpt-fixed @gol
 -maccumulate-outgoing-args -minvalid-symbols @gol
 -matomic-model=@var{atomic-model} @gol
--mbranch-cost=@var{num} -mzdcbranch -mno-zdcbranch -mcbranchdi -mcmpeqdi @gol
+-mbranch-cost=@var{num} -mzdcbranch -mno-zdcbranch @gol
 -mfused-madd -mno-fused-madd -mfsca -mno-fsca -mfsrra -mno-fsrra @gol
 -mpretend-cmove -mtas}

@@ -20252,15 +20252,6 @@
 enabled by default when generating code for SH4 and SH4A.  It can be
explicitly
 disabled by specifying @option{-mno-zdcbranch}.

-@item -mcbranchdi
-@opindex mcbranchdi
-Enable the @code{cbranchdi4} instruction pattern.
-
-@item -mcmpeqdi
-@opindex mcmpeqdi
-Emit the @code{cmpeqdi_t} instruction pattern even when @option{-mcbranchdi}
-is in effect.
-
 @item -mfused-madd
 @itemx -mno-fused-madd
 @opindex mfused-madd
Index: gcc/config/sh/sh.opt
===================================================================
--- gcc/config/sh/sh.opt    (revision 205756)
+++ gcc/config/sh/sh.opt    (working copy)
@@ -233,11 +233,11 @@
 Assume that zero displacement conditional branches are fast

 mcbranchdi
-Target Var(TARGET_CBRANCHDI4)
+Target Undocumented Var(TARGET_CBRANCHDI4) Warn(%qs is deprecated and has no
effect)
 Enable cbranchdi4 pattern

 mcmpeqdi
-Target Var(TARGET_CMPEQDI_T)
+Target Undocumented Var(TARGET_CMPEQDI_T) Warn(%qs is deprecated and has no
effect)
 Emit cmpeqdi_t pattern even when -mcbranchdi is in effect.

 mcut2-workaround
Index: gcc/config/sh/sh.c
===================================================================
--- gcc/config/sh/sh.c    (revision 205756)
+++ gcc/config/sh/sh.c    (working copy)
@@ -771,6 +771,11 @@
   SUBTARGET_OVERRIDE_OPTIONS;
   if (optimize > 1 && !optimize_size)
     target_flags |= MASK_SAVE_ALL_TARGET_REGS;
+
+  /* Set default values of TARGET_CBRANCHDI4 and TARGET_CMPEQDI_T.  */
+  TARGET_CBRANCHDI4 = 1;
+  TARGET_CMPEQDI_T = 0;
+
   sh_cpu = PROCESSOR_SH1;
   assembler_dialect = 0;
   if (TARGET_SH2)

[Bug target/52898] SH Target: Inefficient DImode comparisons

[kkojima at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=52898

--- Comment #8 from Kazumoto Kojima <kkojima at gcc dot gnu.org> ---
I'm OK with it.

[Bug target/52898] SH Target: Inefficient DImode comparisons

[olegendo at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=52898

--- Comment #9 from Oleg Endo <olegendo at gcc dot gnu.org> ---
Author: olegendo
Date: Sun Dec  8 22:15:59 2013
New Revision: 205794

URL: http://gcc.gnu.org/viewcvs?rev=205794&root=gcc&view=rev
Log:
    PR target/52898
    PR target/51697
    * common/config/sh/sh-common.c (sh_option_optimization_table): Remove
    OPT_mcbranchdi entry.
    * config/sh/sh.opt (mcbranchdi, mcmpeqdi): Mark as undocumented and
    emit a warning.
    * config/sh/sh.c (sh_option_override): Initialize TARGET_CBRANCHDI4
    and TARGET_CMPEQDI_T variables.
    * doc/invoke.texi (SH options): Undocument -mcbranchdi and -mcmpeqdi.

    PR target/52898
    PR target/51697
    * gcc.target/sh/pr51697.c: New.


Added:
    trunk/gcc/testsuite/gcc.target/sh/pr51697.c
Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/common/config/sh/sh-common.c
    trunk/gcc/config/sh/sh.c
    trunk/gcc/config/sh/sh.opt
    trunk/gcc/doc/invoke.texi
    trunk/gcc/testsuite/ChangeLog