[Bug target/50814] New: SH Target: SHAD / SHLD instructions not used on SH2A

[Bug target/50814] New: SH Target: SHAD / SHLD instructions not used on SH2A

[oleg.endo@t-online.de]

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

             Bug #: 50814
           Summary: SH Target: SHAD / SHLD instructions not used on SH2A
    Classification: Unclassified
           Product: gcc
           Version: 4.7.0
            Status: UNCONFIRMED
          Severity: enhancement
          Priority: P3
         Component: target
        AssignedTo: unassigned@gcc.gnu.org
        ReportedBy: oleg.endo@t-online.de
                CC: kkojima@gcc.gnu.org
            Target: sh2a-*-*


Although there are some insns (e.g. ashlsi3_sh2a) that are supposed to handle
dynamic shifts on SH2A, somehow the dynamic shift instructions SHAD and SHLD
are never generated, no matter what the shift amount is.


int x_shad_right (int y)
{
  return y >> 15;
}

    mov.l    .L6,r1
    sts.l    pr,@-r15
    jsr    @r1
    nop
    mov    r4,r0
    lds.l   @r15+,pr
    rts/n

    .align 2
.L6:
    .long    ___ashiftrt_r4_15


int x_shad_left (int y)
{
  return y << 15;
}

    mov    r4,r0
    shll8    r0
    shlr    r0
    rts
    shll8    r0



Using built-in specs.
COLLECT_GCC=sh-elf-gcc
COLLECT_LTO_WRAPPER=/usr/local/libexec/gcc/sh-elf/4.7.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.7.0 20111020 (experimental) (GCC) 



It is also not clear to me why SH2A seems to require different handling for
dynamic shifts than SH3 or SH4...

[Bug target/50814] SH Target: SHAD / SHLD instructions not used on SH2A

[kkojima at gcc dot gnu.org]

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

--- Comment #1 from Kazumoto Kojima <kkojima at gcc dot gnu.org> 2011-10-21 00:24:36 UTC ---
(In reply to comment #0)
> It is also not clear to me why SH2A seems to require different handling for
> dynamic shifts than SH3 or SH4...

Will be slightly different because sh2a's shad&shld are 4-byte
insns.  Perhaps something like below will work, though I don't
test it at all.

diff -up ORIG/gcc/config/sh/sh.h gcc/config/sh/sh.h
--- ORIG/gcc/config/sh/sh.h    2011-04-23 09:43:19.000000000 +0900
+++ gcc/config/sh/sh.h    2011-10-21 08:15:25.000000000 +0900
@@ -2371,7 +2371,8 @@ extern int current_function_interrupt;
 #define ACCUMULATE_OUTGOING_ARGS TARGET_ACCUMULATE_OUTGOING_ARGS

 #define SH_DYNAMIC_SHIFT_COST \
-  (TARGET_HARD_SH4 ? 1 : TARGET_SH3 ? (optimize_size ? 1 : 2) : 20)
+  (TARGET_HARD_SH4 ? 1 : TARGET_SH3 ? (optimize_size ? 1 : 2) \
+   : TARGET_SH2A ? 2 : 20)


 #define NUM_MODES_FOR_MODE_SWITCHING { FP_MODE_NONE }
diff -up ORIG/gcc/config/sh/sh.c gcc/config/sh/sh.c
--- ORIG/gcc/config/sh/sh.c    2011-07-29 09:31:42.000000000 +0900
+++ gcc/config/sh/sh.c    2011-10-21 09:03:36.000000000 +0900
@@ -3246,7 +3246,7 @@ expand_ashiftrt (rtx *operands)
   char func[18];
   int value;

-  if (TARGET_SH3)
+  if (TARGET_SH3 || TARGET_SH2A)
     {
       if (!CONST_INT_P (operands[2]))
     {
diff -up ORIG/gcc/config/sh/sh.md gcc/config/sh/sh.md
--- ORIG/gcc/config/sh/sh.md    2011-08-02 09:47:17.000000000 +0900
+++ gcc/config/sh/sh.md    2011-10-21 08:58:49.000000000 +0900
@@ -3424,15 +3424,6 @@ label:
 ;;
 ;; shift left

-(define_insn "ashlsi3_sh2a"
-  [(set (match_operand:SI 0 "arith_reg_dest" "=r")
-    (ashift:SI (match_operand:SI 1 "arith_reg_operand" "0")
-           (match_operand:SI 2 "arith_reg_operand" "r")))]
-  "TARGET_SH2A"
-  "shad    %2,%0"
-  [(set_attr "type" "arith")
-   (set_attr "length" "4")])
-
 ;; This pattern is used by init_expmed for computing the costs of shift
 ;; insns.

@@ -3441,14 +3432,14 @@ label:
     (ashift:SI (match_operand:SI 1 "arith_reg_operand" "0,0,0,0")
            (match_operand:SI 2 "nonmemory_operand" "r,M,P27,?ri")))
    (clobber (match_scratch:SI 3 "=X,X,X,&r"))]
-  "TARGET_SH3
+  "(TARGET_SH3 || TARGET_SH2A)
    || (TARGET_SH1 && satisfies_constraint_P27 (operands[2]))"
   "@
    shld    %2,%0
    add    %0,%0
    shll%O2    %0
    #"
-  "TARGET_SH3
+  "(TARGET_SH3 || TARGET_SH2A)
    && reload_completed
    && CONST_INT_P (operands[2])
    && ! satisfies_constraint_P27 (operands[2])"
@@ -3457,7 +3448,11 @@ label:
     [(set (match_dup 0) (ashift:SI (match_dup 1) (match_dup 3)))
      (clobber (match_dup 4))])]
   "operands[4] = gen_rtx_SCRATCH (SImode);"
-  [(set_attr "length" "*,*,*,4")
+  [(set_attr_alternative "length"
+     [(if_then_else
+    (ne (symbol_ref "TARGET_SH2A") (const_int 0))
+    (const_int 4) (const_int 2))
+     (const_int 2) (const_int 2) (const_int 4)])
    (set_attr "type" "dyn_shift,arith,arith,arith")])

 (define_insn "ashlhi3_k"
@@ -3584,15 +3579,6 @@ label:
 ; arithmetic shift right
 ;

-(define_insn "ashrsi3_sh2a"
-  [(set (match_operand:SI 0 "arith_reg_dest" "=r")
-    (ashiftrt:SI (match_operand:SI 1 "arith_reg_operand" "0")
-           (neg:SI (match_operand:SI 2 "arith_reg_operand" "r"))))]
-  "TARGET_SH2A"
-  "shad    %2,%0"
-  [(set_attr "type" "dyn_shift")
-   (set_attr "length" "4")])
-
 (define_insn "ashrsi3_k"
   [(set (match_operand:SI 0 "arith_reg_dest" "=r")
     (ashiftrt:SI (match_operand:SI 1 "arith_reg_operand" "0")
@@ -3687,9 +3673,13 @@ label:
   [(set (match_operand:SI 0 "arith_reg_dest" "=r")
     (ashiftrt:SI (match_operand:SI 1 "arith_reg_operand" "0")
              (neg:SI (match_operand:SI 2 "arith_reg_operand" "r"))))]
-  "TARGET_SH3"
+  "TARGET_SH3 || TARGET_SH2A"
   "shad    %2,%0"
-  [(set_attr "type" "dyn_shift")])
+  [(set_attr_alternative "length"
+     [(if_then_else
+    (ne (symbol_ref "TARGET_SH2A") (const_int 0))
+    (const_int 4) (const_int 2))])
+   (set_attr "type" "dyn_shift")])

 (define_insn "ashrsi3_n"
   [(set (reg:SI R4_REG)
@@ -3735,22 +3725,17 @@ label:

 ;; logical shift right

-(define_insn "lshrsi3_sh2a"
-  [(set (match_operand:SI 0 "arith_reg_dest" "=r")
-    (lshiftrt:SI (match_operand:SI 1 "arith_reg_operand" "0")
-             (neg:SI (match_operand:SI 2 "arith_reg_operand" "r"))))]
-  "TARGET_SH2A"
-  "shld    %2,%0"
-  [(set_attr "type" "dyn_shift")
-   (set_attr "length" "4")])
-
 (define_insn "lshrsi3_d"
   [(set (match_operand:SI 0 "arith_reg_dest" "=r")
     (lshiftrt:SI (match_operand:SI 1 "arith_reg_operand" "0")
              (neg:SI (match_operand:SI 2 "arith_reg_operand" "r"))))]
-  "TARGET_SH3"
+  "TARGET_SH3 || TARGET_SH2A"
   "shld    %2,%0"
-  [(set_attr "type" "dyn_shift")])
+  [(set_attr "type" "dyn_shift")
+   (set_attr_alternative "length"
+     [(if_then_else
+    (ne (symbol_ref "TARGET_SH2A") (const_int 0))
+    (const_int 4) (const_int 2))])])

 ;;  Only the single bit shift clobbers the T bit.

[Bug target/50814] SH Target: SHAD / SHLD instructions not used on SH2A

[oleg.endo@t-online.de]

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

--- Comment #2 from Oleg Endo <oleg.endo@t-online.de> 2011-11-27 22:38:34 UTC ---
(In reply to comment #1)

> Will be slightly different because sh2a's shad&shld are 4-byte
> insns.  Perhaps something like below will work, though I don't
> test it at all.
> 

According to the SW manual document rej09b0051_sh2a.pdf the SHAD and SHLD insns
have the same 2-byte format as on SH3:

SHAD Rm, Rn: 0100nnnnmmmm1100
SHLD Rm, Rn: 0100nnnnmmmm1101 

Am I missing something there?

[Bug target/50814] SH Target: SHAD / SHLD instructions not used on SH2A

[kkojima at gcc dot gnu.org]

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

--- Comment #3 from Kazumoto Kojima <kkojima at gcc dot gnu.org> 2011-11-28 00:09:17 UTC ---
(In reply to comment #2)
> According to the SW manual document rej09b0051_sh2a.pdf the SHAD and SHLD insns
> have the same 2-byte format as on SH3:
> 
> SHAD Rm, Rn: 0100nnnnmmmm1100
> SHLD Rm, Rn: 0100nnnnmmmm1101 
> 
> Am I missing something there?

Ugh.  You are right.  I thought so from sh2a support was introduced
at r85286.

[Bug target/50814] SH Target: SHAD / SHLD instructions not used on SH2A

[kkojima at gcc dot gnu.org]

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

--- Comment #4 from Kazumoto Kojima <kkojima at gcc dot gnu.org> 2011-11-28 04:31:51 UTC ---
Created attachment 25927
  --> http://gcc.gnu.org/bugzilla/attachment.cgi?id=25927
A patch

I'm testing the attached patch.

[Bug target/50814] SH Target: SHAD / SHLD instructions not used on SH2A

[kkojima at gcc dot gnu.org]

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

--- Comment #5 from Kazumoto Kojima <kkojima at gcc dot gnu.org> 2011-11-28 13:43:16 UTC ---
BTW, when regtesting, I've found that there are many ICEs at -O0.
A typical one is gcc.c-torture/compile/20000923-1.c with -m2a -O0:

...: error: insn does not satisfy its constraints:
(insn 142 34 35 (set (mem/c:SI (plus:SI (reg/f:SI 14 r14)
                (const_int 36 [0x24])) [0 %sfp+-16 S4 A32])
        (reg:SI 150 fpul)) ... {movsi_ie}
     (nil))
...: internal compiler error: in extract_constrain_insn_cached, at recog.c:2052

which is solved by the hunk in the patch against PR50751

--- gcc/config/sh/sh.c.orig    2011-11-28 10:03:04.000000000 +0900
+++ gcc/config/sh/sh.c    2011-11-28 15:09:01.000000000 +0900
@@ -12432,6 +12432,10 @@ sh_secondary_reload (bool in_p, rtx x, r
   if (rclass != GENERAL_REGS && REG_P (x)
       && TARGET_REGISTER_P (REGNO (x)))
     return GENERAL_REGS;
+  /* If here fall back to loading FPUL register through general regs. 
+     Happens when FPUL has to be loaded from a reg allocated on the stack.  */
+  if (rclass == FPUL_REGS && !REG_P (x))
+    return GENERAL_REGS;
   return NO_REGS;
 }

Oleg, it seems that this is the right patch for an independent issue
described in your comment.  Could you please file it to the bugzilla
and propose that patch to the gcc-patch list?

[Bug target/50814] SH Target: SHAD / SHLD instructions not used on SH2A

[kkojima at gcc dot gnu.org]

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

--- Comment #6 from Kazumoto Kojima <kkojima at gcc dot gnu.org> 2011-12-01 23:02:08 UTC ---
Author: kkojima
Date: Thu Dec  1 23:01:58 2011
New Revision: 181896

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=181896
Log:
    PR target/50814.
    * config/sh/sh.c (expand_ashiftrt): Handle TARGET_SH2A same as
    TARGET_SH3.
    (shl_sext_kind): Likewise.
    * config/sh/sh.h (SH_DYNAMIC_SHIFT_COST): Likewise.
    * config/sh/sh.md (ashlsi3_sh2a, ashrsi3_sh2a, lshrsi3_sh2a):
    Remove.
    (ashlsi3_std): Handle TARGET_SH2A same as TARGET_SH3.
    (ashlsi3): Likewise.
    (ashrsi3_d): Likewise.
    (lshrsi3_d): Likewise.
    (lshrsi3): Likewise.


Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/config/sh/sh.c
    trunk/gcc/config/sh/sh.h
    trunk/gcc/config/sh/sh.md

[Bug target/50814] SH Target: SHAD / SHLD instructions not used on SH2A

[kkojima at gcc dot gnu.org]

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

Kazumoto Kojima <kkojima at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |RESOLVED
         Resolution|                            |FIXED

--- Comment #7 from Kazumoto Kojima <kkojima at gcc dot gnu.org> 2011-12-02 23:42:56 UTC ---
Fixed on trunk.