[Bug target/50751] New: SH Target: Displacement addressing does not work for QImode and HImode

[Bug target/50751] New: SH Target: Displacement addressing does not work for QImode and HImode

[oleg.endo@t-online.de]

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

             Bug #: 50751
           Summary: SH Target: Displacement addressing does not work for
                    QImode and HImode
    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: sh*-*-*


Displacement addressing is only used for SImode but not
for QImode nor HImode. The following example summarizes the 
problem:

struct X
{
  char    a, b, c, d;
  short e, f;
  int   g, h;
};


int test_func_4 (X* x)
{
  return x->b + x->e + x->g;
}

compiled with: -Os -m4-single -ml -S

    mov    r4,r1
    add    #1,r1
    mov.b    @r1,r0
    add    #3,r1
    mov.w    @r1,r1
    add    r1,r0
    mov.l    @(8,r4),r1
    rts    
    add    r1,r0

would be better as:


    mov.b    @(1,r4),r0
    mov    r0,r1
    mov.w    @(4,r4),r0
    add    r0,r1
    mov.l    @(8,r4),r0
    add    r1,r0
    rts
    nop


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 20111016 (experimental) (GCC)

[Bug target/50751] SH Target: Displacement addressing does not work for QImode and HImode

[kkojima at gcc dot gnu.org]

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

--- Comment #1 from Kazumoto Kojima <kkojima at gcc dot gnu.org> 2011-10-17 00:29:55 UTC ---
This is a known issue.  See the comment just before sh.c:sh_legitimate_index_p.
Unfortunately, I guess this PR might be marked as WONTFIX.

[Bug target/50751] SH Target: Displacement addressing does not work for QImode and HImode

[oleg.endo@t-online.de]

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

--- Comment #2 from Oleg Endo <oleg.endo@t-online.de> 2011-10-17 00:37:42 UTC ---
(In reply to comment #1)
> This is a known issue.  See the comment just before sh.c:sh_legitimate_index_p.
> Unfortunately, I guess this PR might be marked as WONTFIX.

Yeah, I know this has been around for a while.
I'd like to take my chances anyway :)

Cheers,
Oleg

[Bug target/50751] SH Target: Displacement addressing does not work for QImode and HImode

[kkojima at gcc dot gnu.org]

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

--- Comment #3 from Kazumoto Kojima <kkojima at gcc dot gnu.org> 2011-10-17 00:51:15 UTC ---
(In reply to comment #2)
> Yeah, I know this has been around for a while.
> I'd like to take my chances anyway :)

Welcome to the spill-failure-for-class-'R0_REGS' club :-)

[Bug target/50751] SH Target: Displacement addressing does not work for QImode and HImode

[oleg.endo@t-online.de]

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

--- Comment #4 from Oleg Endo <oleg.endo@t-online.de> 2011-10-23 21:56:56 UTC ---
Created attachment 25582
  --> http://gcc.gnu.org/bugzilla/attachment.cgi?id=25582
Experimental patch for mov.b with displacement addressing

> (In reply to comment #2)
> 
> Welcome to the spill-failure-for-class-'R0_REGS' club :-)

The attached patch is an experimental example only.  Please ignore wrong
formatting, comments, and the fact that it cripples some of the SH2A support :}

It adds support for 'mov.b @(disp, Rm), R0' and 'mov.b R0, @(disp, Rn)'
instructions.  I haven't tested it fully, only building the CSiBE
set for -m4-single -ml. 

Although it results in suboptimal code like ...

    ...
    mov.l    @(4,r15),r2
    mov    r1,r0
    mov.b    r0,@(5,r2)
    rts
    mov    r1,r0        ! redundant

.. or like ...

    mov.b    @(1,r4),r0
    mov    r0,r4
    mov.b    @(2,r5),r0
    add    r0,r4        ! better: add r4, r0
    mov    r4,r0        ! not needed if add operands are swapped
    mov.b    r0,@(5,r6)
    rts
    mov    r4,r0        ! redundant

.. it already shows some code size improvements:

avg: -563.222222 / -0.942376 %
max: compiler        22804 -> 22928         +124 / +0.543764 %
min: OpenTCP-1.0.4   27069 -> 25989        -1080 / -3.989804 %

top 5 files
mpeg2dec-0.3.1  libmpeg2/motion_comp 
  6044 -> 4796         -1248 / -20.648577 %

libpng-1.2.5  pngrtran
  19668 -> 18904         -764 / -3.884482 %

linux-2.4.23-pre3-testplatform  arch/testplatform/kernel/traps
  6192 -> 5532          -660 / -10.658915 %

lwip-0.5.3.preproc  src/core/tcp_input
  5424 -> 5040          -384 / -7.079646 %

libmspack  test/cabextract_md5
  21780 -> 21424         -356 / -1.634527 %


The R0 clobber in the movqi expander and the explicit usage of R0 in the 
splits effectively disable some optimizations, but this is the only 
thing I could get to work so far.
I've left the straight forward but non-working patterns as comments in the
patch as a reference.  Basically, without the R0 clobber in the movqi expander
it eventually ends up like that...

error: insn does not satisfy its constraints:
(insn 737 40 42 4 (set (reg:QI 10 r10)
        (mem/c:QI (plus:SI (reg:SI 1 r1 [386])
                (const_int 1 [0x1])) [0 *D.4946_20+0 S1 A8]))
{*movqi_m_reg_disp_load}
     (nil))

internal compiler error: in reload_cse_simplify_operands, at postreload.c:403

I'm puzzled why the register allocator ignores the constraint "z" when it
starts to run out of registers.  In the error case above it tries to produce
something like 'mov.b @(1,r1),r10' which of course is impossible.
Any hints are highly appreciated.

[Bug target/50751] SH Target: Displacement addressing does not work for QImode and HImode

[kkojima at gcc dot gnu.org]

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

--- Comment #5 from Kazumoto Kojima <kkojima at gcc dot gnu.org> 2011-10-24 23:05:08 UTC ---
(In reply to comment #4)
It seems that clobbering R0 in that expander is simply papering
over the real problem.  Although the reload issue beyonds me,
.ira dump file about that impossible insn which doesn't satisfy
the "z" constraint would be a starting point.

[Bug target/50751] SH Target: Displacement addressing does not work for QImode and HImode

[oleg.endo@t-online.de]

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

--- Comment #6 from Oleg Endo <oleg.endo@t-online.de> 2011-10-26 22:36:31 UTC ---
Created attachment 25621
  --> http://gcc.gnu.org/bugzilla/attachment.cgi?id=25621
Experimental not working patch for mov.b with displacement addressing

[Bug target/50751] SH Target: Displacement addressing does not work for QImode and HImode

[oleg.endo@t-online.de]

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

--- Comment #7 from Oleg Endo <oleg.endo@t-online.de> 2011-10-26 23:07:08 UTC ---
Created attachment 25622
  --> http://gcc.gnu.org/bugzilla/attachment.cgi?id=25622
asmcons and ira pass log for the reload failure of "z" insn constraint

(In reply to comment #5)
> It seems that clobbering R0 in that expander is simply papering
> over the real problem.  

Yes, it is very much.

> Although the reload issue beyonds me,
> .ira dump file about that impossible insn which doesn't satisfy
> the "z" constraint would be a starting point.

I've been trying to make some sense out of it since, but I'm a bit clueless at
the moment here.

The following function is a reduced failure example:

int fail (char* buf0, char* buf1)
{
  char a,b,c;

  a = buf0[0] + buf1[1];
  b = buf0[5] + buf1[4];
  c = buf0[15] + buf1[14];
  return a+b-c;
}

In the attached log the problematic pseudo reg is reg:QI 191, where it is
reloaded into r1... 

Reloads for insn # 13
Reload 0: reload_in (QI) = (reg:QI 191 [ MEM[(char *)buf1_4(D) + 4B] ])
    reload_out (SI) = (reg:SI 1 r1 [193])
    GENERAL_REGS, RELOAD_OTHER (opnum = 0)


Strange thing is that the following variant does not cause the error:

int fail (char* buf0, char* buf1, char* out)
{
  char a,b,c;

  a = buf0[0] + buf1[1];
  b = buf0[5] + buf1[4];
  c = buf0[15] + buf1[14];
  *out = a+b-c;
  return *out;
}

[Bug target/50751] SH Target: Displacement addressing does not work for QImode and HImode

[kkojima at gcc dot gnu.org]

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

--- Comment #8 from Kazumoto Kojima <kkojima at gcc dot gnu.org> 2011-10-27 02:31:35 UTC ---
(In reply to comment #7)
> Created attachment 25622 [details]
> asmcons and ira pass log for the reload failure of "z" insn constraint

The original insn 13 was

(insn 13 12 14 3 (set (reg:SI 193)
        (plus:SI (subreg:SI (reg:QI 191 [ MEM[(char *)buf1_4(D) + 4B] ]) 0)
            (subreg:SI (reg:QI 192 [ MEM[(char *)buf0_1(D) + 5B] ]) 0)))

and RA chooses r1 and r0 as the registers to where memories will
be loaded.  The problem is we have no direct way to load buf1[4]
to r1.  In such situation, a secondary reload is needed.  See
the description of TARGET_SECONDARY_RELOAD in the gcc manual.
Here is a trial:

--- ORIG/trunk/gcc/config/sh/sh.c    2011-10-16 10:18:53.000000000 +0900
+++ trunk/gcc/config/sh/sh.c    2011-10-27 10:13:21.000000000 +0900
@@ -12430,6 +12453,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 (rclass == GENERAL_REGS && mode == QImode
+      && MEM_P (x) && GET_CODE (XEXP (x, 0)) == PLUS
+      && CONST_INT_P (XEXP (XEXP (x, 0), 1)))
+    return R0_REGS;
   return NO_REGS;
 }

The ICE for your testcase went away with it, though I've got

../../../INTEST/trunk/zlib/trees.c: In function 'send_tree':
../../../INTEST/trunk/zlib/trees.c:797:1: error: unable to find a register to
spill in class 'R0_REGS'
../../../INTEST/trunk/zlib/trees.c:797:1: error: this is the insn:
(insn 415 414 416 28 (set (mem:QI (plus:SI (reg/f:SI 6 r6 [orig:742
s_34(D)->pending_buf ] [742])
                (reg:SI 7 r7 [orig:307 D.4248 ] [307])) [0 *D.4249_209+0 S1
A8])
        (reg:QI 746 [ s_34(D)->bi_buf ]))
../../../INTEST/trunk/zlib/trees.c:780 206 {*movqi_m_reg_reg_store}
     (expr_list:REG_DEAD (reg:QI 746 [ s_34(D)->bi_buf ])
        (expr_list:REG_DEAD (reg/f:SI 6 r6 [orig:742 s_34(D)->pending_buf ]
[742])
            (expr_list:REG_DEAD (reg:SI 7 r7 [orig:307 D.4248 ] [307])
                (nil)))))
../../../INTEST/trunk/zlib/trees.c:797:1: internal compiler error: in
spill_failure, at reload1.c:2118

when bootstrapping.

[Bug target/50751] SH Target: Displacement addressing does not work for QImode and HImode

[oleg.endo@t-online.de]

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

--- Comment #9 from Oleg Endo <oleg.endo@t-online.de> 2011-10-27 09:23:47 UTC ---
Created attachment 25625
  --> http://gcc.gnu.org/bugzilla/attachment.cgi?id=25625
Stripped reload test case from bzip2

(In reply to comment #8)
> 
> and RA chooses r1 and r0 as the registers to where memories will
> be loaded.  The problem is we have no direct way to load buf1[4]
> to r1.  In such situation, a secondary reload is needed.  See
> the description of TARGET_SECONDARY_RELOAD in the gcc manual.
> Here is a trial:

Thanks a lot!

> 
> The ICE for your testcase went away with it, though I've got
> [...]
> spill_failure, at reload1.c:2118
> 
> when bootstrapping.

So this passes the ball to the mov.b (R0,Rn),Rm insn...
The same happens with bzip2 in CSiBE.  I have attached a stripped down
snippet that reproduces the problem.

IRA's last words on that one are:

Using reg 2 for reload 0
Spilling for insn 365.
Using reg 2 for reload 0
Spilling for insn 371.
Using reg 3 for reload 0
Spilling for insn 374.
Using reg 2 for reload 0
Spilling for insn 377.
Using reg 0 for reload 1
reload failure for reload 2

Reloads for insn # 377
Reload 0: reload_in (SI) = (plus:SI (reg/f:SI 15 r15)
                                                    (const_int 16 [0x10]))
    GENERAL_REGS, RELOAD_FOR_INPUT_ADDRESS (opnum = 2), can't combine
    reload_in_reg: (plus:SI (reg/f:SI 15 r15)
                                                    (const_int 16 [0x10]))
Reload 1: reload_in (SI) = (reg:SI 3 r3 [693])
    R0_REGS, RELOAD_FOR_INPUT (opnum = 0)
    reload_in_reg: (reg:SI 3 r3 [693])
Reload 2: R0_REGS, RELOAD_FOR_INPUT_ADDRESS (opnum = 2), can't combine,
secondary_reload_p
Reload 3: reload_in (QI) = (mem/c:QI (plus:SI (reg/f:SI 15 r15)
                                                        (const_int 16 [0x10]))
[6 %sfp+-60 S1 A32])
    GENERAL_REGS, RELOAD_FOR_INPUT (opnum = 2), can't combine
    reload_in_reg: (subreg:QI (reg/v:SI 177 [ curr ]) 0)
    secondary_in_reload = 2

[Bug target/50751] SH Target: Displacement addressing does not work for QImode and HImode

[oleg.endo@t-online.de]

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

--- Comment #10 from Oleg Endo <oleg.endo@t-online.de> 2011-10-27 21:10:47 UTC ---
@@ -12430,6 +12453,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 (rclass == GENERAL_REGS && mode == QImode
+      && MEM_P (x) && GET_CODE (XEXP (x, 0)) == PLUS
+      && CONST_INT_P (XEXP (XEXP (x, 0), 1))
+      && INTVAL (XEXP (XEXP (x, 0), 1)) < 16)
+    return R0_REGS;
   return NO_REGS;
 }


Makes the stripped down bszip2 test pass, but the same problem pops up
in other CSiBE sources.

[Bug target/50751] SH Target: Displacement addressing does not work for QImode and HImode

[oleg.endo@t-online.de]

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

--- Comment #11 from Oleg Endo <oleg.endo@t-online.de> 2011-10-27 21:54:17 UTC ---
Created attachment 25639
  --> http://gcc.gnu.org/bugzilla/attachment.cgi?id=25639
Stripped reload test case from jpeg-6b

[Bug target/50751] SH Target: Displacement addressing does not work for QImode and HImode

[kkojima at gcc dot gnu.org]

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

--- Comment #12 from Kazumoto Kojima <kkojima at gcc dot gnu.org> 2011-10-27 22:30:39 UTC ---
It seems that base_reg+index_reg addressing requires special
handling in RA and the move insn like

(define_insn "*movqi_m_reg_reg_store"
  [(set (mem:QI (plus:SI (match_operand:SI 0 "arith_reg_operand" "%z")
             (match_operand:SI 1 "arith_reg_operand" "r")))
    (match_operand:QI 2 "arith_reg_operand" "r"))]
  "TARGET_SH1"
  "mov.b    %2,@(%0,%1)"
  [(set_attr "type" "store")])

might be unexpected for RA.

[Bug target/50751] SH Target: Displacement addressing does not work for QImode and HImode

[oleg.endo@t-online.de]

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

--- Comment #13 from Oleg Endo <oleg.endo@t-online.de> 2011-11-02 00:15:44 UTC ---
Created attachment 25684
  --> http://gcc.gnu.org/bugzilla/attachment.cgi?id=25684
Proposed patch to add QImode displacement addressing

This should be better now.  I have also put back support for the SH2A
displacement addressing insns.  The CSiBE set compiles fine for -m2a,
-m4-single, -m4a-single, both big and little endian.  However, it introduces
new failures in the testsuite.  I have examined a few of them and one common
problem seems to be of this nature:

udivmod4.c:59:1: error: insn does not satisfy its constraints:
(insn 313 312 194 (set (reg:SI 150 fpul)
        (mem/c:SI (plus:SI (reg/f:SI 15 r15)
                (const_int 8 [0x8])) [0 %sfp+-72 S4 A32])) udivmod4.c:54 192
{movsi_ie}
     (nil))
udivmod4.c:59:1: internal compiler error: in extract_constrain_insn_cached, at
recog.c:2052

.. when compiled with -O0 -m4-single -mb.

This is mainly caused by the following added lines in sh_legitimate_index_p:

+      if ((GET_MODE_SIZE (mode) == 1) && (unsigned) INTVAL (op) < 16)
+    return true;

Apparently this makes something believe that loading the FPUL register from a
displacement address is possible, which is of course not the case.  However, I
can't see any connection there...

[Bug target/50751] SH Target: Displacement addressing does not work for QImode and HImode

[kkojima at gcc dot gnu.org]

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

--- Comment #14 from Kazumoto Kojima <kkojima at gcc dot gnu.org> 2011-11-02 00:57:59 UTC ---
(In reply to comment #13)
> Apparently this makes something believe that loading the FPUL register from a
> displacement address is possible, which is of course not the case.  However, I
> can't see any connection there...

.ira dump would be your friend, though I suspect that your patch
triggered off some other reload problem like PR48596.  Could you
try the change in #5 of that PR?

[Bug target/50751] SH Target: Displacement addressing does not work for QImode and HImode

[oleg.endo@t-online.de]

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

--- Comment #15 from Oleg Endo <oleg.endo@t-online.de> 2011-11-02 01:38:26 UTC ---
(In reply to comment #14)
> .ira dump would be your friend, though I suspect that your patch
> triggered off some other reload problem like PR48596.  Could you
> try the change in #5 of that PR?

I just tried it out, but it seems it has no effect in this case.

[Bug target/50751] SH Target: Displacement addressing does not work for QImode and HImode

[oleg.endo@t-online.de]

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

Oleg Endo <oleg.endo@t-online.de> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #25684|0                           |1
        is obsolete|                            |

--- Comment #16 from Oleg Endo <oleg.endo@t-online.de> 2011-11-17 23:58:11 UTC ---
Created attachment 25848
  --> http://gcc.gnu.org/bugzilla/attachment.cgi?id=25848
Proposed patch to add QImode displacement addressing

Just wanted to share some progress on this issue...

The FPUL case from before seems to be solved, but another issue is left which
is related to vector modes.  For example the 20050113-1.c target test case
compiled with -O0 causes:

20050113-1.c:16:1: internal compiler error: in change_address_1, at
emit-rtl.c:2001
Please submit a full bug report,
with preprocessed source if appropriate.

This is because of the following in function sh_cannot_change_mode_class:

  if (to == SFmode && VECTOR_MODE_P (from) && GET_MODE_INNER (from) == SFmode)
    return (reg_classes_intersect_p (GENERAL_REGS, rclass));


IRA's last words on that are:

Reloads for insn # 52
Reload 0: FPUL_REGS, RELOAD_FOR_INPUT_ADDRESS (opnum = 1), can't combine,
secondary_reload_p
    reload_reg_rtx: (reg:SI 150 fpul)
Reload 1: reload_in (SI) = (reg:SI 65 fr1 [orig:171 <retval> ] [171])
    GENERAL_REGS, RELOAD_FOR_INPUT (opnum = 1)
    reload_in_reg: (reg:SI 65 fr1 [orig:171 <retval> ] [171])
    reload_reg_rtx: (reg/i:SI 0 r0)
    secondary_in_reload = 0
deleting insn with uid = 26.

[Bug target/50751] SH Target: Displacement addressing does not work for QImode and HImode

[oleg.endo@t-online.de]

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

Oleg Endo <oleg.endo@t-online.de> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #25848|0                           |1
        is obsolete|                            |

--- Comment #17 from Oleg Endo <oleg.endo@t-online.de> 2011-11-28 12:26:42 UTC ---
Created attachment 25932
  --> http://gcc.gnu.org/bugzilla/attachment.cgi?id=25932
Proposed patch to add QImode displacement addressing

An updated version of the patch.  Tested against rev 181404, resolves some
previous failures for SH2A but also introduces the following libstdc++ related
failures:

FAIL: 22_locale/ctype/is/wchar_t/11740.cc (test for excess errors)
FAIL: 22_locale/money_get/get/char/5.cc execution test
FAIL: 22_locale/money_put/put/char/12971.cc execution test
FAIL: 22_locale/money_put/put/char/39168.cc execution test
FAIL: 22_locale/money_put/put/char/4.cc execution test
FAIL: 22_locale/money_put/put/char/5.cc execution test
FAIL: 22_locale/money_put/put/char/6.cc execution test
FAIL: 22_locale/money_put/put/wchar_t/12971.cc execution test
FAIL: 22_locale/money_put/put/wchar_t/39168.cc execution test
FAIL: 22_locale/money_put/put/wchar_t/4.cc execution test
FAIL: 22_locale/money_put/put/wchar_t/5.cc execution test
FAIL: 22_locale/money_put/put/wchar_t/6.cc execution test

Still investigating...

[Bug target/50751] SH Target: Displacement addressing does not work for QImode and HImode

[oleg.endo@t-online.de]

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

Oleg Endo <oleg.endo@t-online.de> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #25932|0                           |1
        is obsolete|                            |

--- Comment #18 from Oleg Endo <oleg.endo@t-online.de> 2011-12-11 00:24:17 UTC ---
Created attachment 26046
  --> http://gcc.gnu.org/bugzilla/attachment.cgi?id=26046
Proposed patch to add QImode displacement addressing

This version of the patch was tested against rev 182090 and did not introduce
new failures.

It allows the generation of QImode displacement addressing move insns, but only
if the addresses can be fixed before reload.  If reload needs to load/store
e.g. pseudos on the stack where displacement addressing would be required the
generated code is not as good unfortunately.  One good example for this kind of
scenario is the gcc.dg/compat/struct-by-value-11 test from the testsuite, which
shows that there is still some room for improvement.
Still, on the CSiBE I could observe a code size decrease of -1.2% avg, where
mpeg2dec goes down to -5.65% and libmpeg2/motion_comp down to -20% (-m4 -ml
taken as example).  Due to SH2A's 4 byte QImode displacement insn the benefit
on SH2A code is not that big (-0.2% avg).

There are probably smarter ways of doing what the patch does.  I have also
tried out implementing it with predicates and constraints, few load/store insns
and lots of alternatives in the insns.  However, reload would refuse to select
the displacement addressing due to pressure on R0 in many cases.

Would something like the attached patch be acceptable (after some cleanups)? 
If so, I'd also start adding HImode displacement addressing support.

[Bug target/50751] SH Target: Displacement addressing does not work for QImode and HImode

[kkojima at gcc dot gnu.org]

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

--- Comment #19 from Kazumoto Kojima <kkojima at gcc dot gnu.org> 2011-12-11 23:57:13 UTC ---
(In reply to comment #18)
The results look way better now.  I've tested your latest patch for
sh4-unknown-linux-gnu and found no new regressions for gcc testsuite.
CSiBE with "-O2 -fpic" on that target shows that 144 improvements and
28 dis-improvements for size on 896 files.  The worst case is
-4.34783 net/ipv4/ip_forward 704 736
which looks the case of the high r0 register pressure.  The best one is
25.7426 arch/testplatform/kernel/traps 10160 8080
which looks to be very impressive.

>   /* We want to enable the use of SUBREGs as a means to
>      VEC_SELECT a single element of a vector.  */
>+
>+  /* This effectively disallows using GENERAL_REGS for SFmode vector subregs.
>+     This can be problematic when SFmode vector subregs need to be accessed
>+     on the stack with displacement addressing, as it happens with -O0.
>+     Thus we allow the mode change for -O0.  */
>   if (to == SFmode && VECTOR_MODE_P (from) && GET_MODE_INNER (from) == SFmode)
>-    return (reg_classes_intersect_p (GENERAL_REGS, rclass));
>+    return optimize ? (reg_classes_intersect_p (GENERAL_REGS, rclass)) : false;

Rather than that, I guess that the QI/HImode disp addressing would
be an optimization unneeded for -O0 in the first place.  Perhaps
something like -mpreferdisp option and TARGET_PREFER_DISP macro
which are enable by default but disable at -O0 might be help.  It'll
also help some unfortunate anormallies for which those optimizations
will generate worse codes.

> There are probably smarter ways of doing what the patch does.  I have also
> tried out implementing it with predicates and constraints, few load/store insns
> and lots of alternatives in the insns.  However, reload would refuse to select
> the displacement addressing due to pressure on R0 in many cases.

Maybe.  Implementing it with predicates and constraints would be
smarter if possible but may be difficult because the register
allocator handles the "m" constraint specially.

> Would something like the attached patch be acceptable (after some cleanups)? 
> If so, I'd also start adding HImode displacement addressing support.

I think so, though we are in stage 3 and have to wait the trunk returns
to stage 1 or 2 for committing such changes.  You have the time for
implementing HImode support.
BTW, the changes for white spaces, spells and other clean-ups which
are not essential for this work should be separated into another patch.

[Bug target/50751] SH Target: Displacement addressing does not work for QImode and HImode

[oleg.endo@t-online.de]

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

--- Comment #20 from Oleg Endo <oleg.endo@t-online.de> 2011-12-12 02:11:12 UTC ---
(In reply to comment #19)
> The results look way better now.  I've tested your latest patch for
> sh4-unknown-linux-gnu and found no new regressions for gcc testsuite.
> CSiBE with "-O2 -fpic" on that target shows that 144 improvements and
> 28 dis-improvements for size on 896 files.  The worst case is
> -4.34783 net/ipv4/ip_forward 704 736
> which looks the case of the high r0 register pressure.  The best one is
> 25.7426 arch/testplatform/kernel/traps 10160 8080
> which looks to be very impressive.

That looks nice!  Thanks for checking it out!
I haven't ran CSiBE with "-O2 -fpic", only with "-Os -mpretend-cmove
-mfused-madd -freg-struct-return".  I will use your params and try to see what
is happening in those cases where it gets worse.  Maybe it can be "fixed" with
a peephole.

> 
> >   /* We want to enable the use of SUBREGs as a means to
> >      VEC_SELECT a single element of a vector.  */
> >+
> >+  /* This effectively disallows using GENERAL_REGS for SFmode vector subregs.
> >+     This can be problematic when SFmode vector subregs need to be accessed
> >+     on the stack with displacement addressing, as it happens with -O0.
> >+     Thus we allow the mode change for -O0.  */
> >   if (to == SFmode && VECTOR_MODE_P (from) && GET_MODE_INNER (from) == SFmode)
> >-    return (reg_classes_intersect_p (GENERAL_REGS, rclass));
> >+    return optimize ? (reg_classes_intersect_p (GENERAL_REGS, rclass)) : false;


> >+     Thus we allow the mode change for -O0.  */

.. should be _disallow_ of course... 
Another note that should have went into the comment:

As far as I could observe it, this is mainly triggered by the following in
sh_legitimate_index_p:

+      if (mode == QImode && (unsigned) INTVAL (op) < 16)
+    return true;

.. probably because this makes the generic "m" constraint match QImode
displacement addressing and then it tries using it.

> Rather than that, I guess that the QI/HImode disp addressing would
> be an optimization unneeded for -O0 in the first place.  Perhaps
> something like -mpreferdisp option and TARGET_PREFER_DISP macro
> which are enable by default but disable at -O0 might be help.

Yeah, could also be an option. 

> It'll also help some unfortunate anormallies for which those optimizations
> will generate worse codes.

You mean, by giving the user the option to turn off displacement addressing for
e.g. some specific files / modules by specifying -mno-preferdisp or something
like that?  By anomalies do you mean code that gets worse because of too much
pressure on R0 and all the reloads around it, or do you have any other bad use
cases?

BTW, the vector mode handling seems a bit unfinished (see also PR13423).  I was
planning to address that at a later point...


> Maybe.  Implementing it with predicates and constraints would be
> smarter if possible but may be difficult because the register
> allocator handles the "m" constraint specially.

Yes, the "m" constraint is an obstacle in this case.
What I've tried out is splitting it into a memory constraint that allows
displacement addressing and another memory constraint that disallows it ("Snd",
"Sdd" - they are added by the patch) and use those in the move / sign extend
insns instead of the generic "m" constraint.  For example something like that:

(define_insn "*extendqisi2"
  [(set (match_operand:SI 0 "arith_reg_dest" "=z,r,r")
    (sign_extend:SI
     (match_operand:QI 1 "general_movsrc_operand" Sdd,Snd,r")))]
  "TARGET_SH1"
  "@
    mov.b    %1,%0
    mov.b    %1,%0
    exts.b    %1,%0"
  [(set_attr "type" "load,load,arith")])

This basically seems to work. But when there are consecutive loads, reload
would use displacement addressing for the first load, but not for the following
loads because R0 will be already allocated at that point.  Ideally, reload
should take into account that "reloading around R0" is in most cases more
efficient than other strategies, especially on SH4.  However, I'm not sure
whether changing reload for this issue is a good idea ;)

Another thing I could try out is to have load/store insns that allow arbitrary
operands in displacement addressing like on SH2A, and split them into two insns
of one load/store and one reg-reg move after reload.  But that would probably
require the R0 clobber in the expander which could make worse code in cases
where displacement addressing is not used, I guess.
Do you think this approach could make sense?

> I think so, though we are in stage 3 and have to wait the trunk returns
> to stage 1 or 2 for committing such changes.

I was afraid you might say something like this :T

> You have the time for implementing HImode support.

Yep, sure.  I've noticed that the latest version of the patch seems to fix some
more testsuite failures.  I will investigate which hunk is responsible for the
fixes so that could be pulled out from the patch.  OK?

> BTW, the changes for white spaces, spells and other clean-ups which
> are not essential for this work should be separated into another patch.

Ah yeah, sure.  Will pull them out and submit as a cleanup patch to the
patches-list.

[Bug target/50751] SH Target: Displacement addressing does not work for QImode and HImode

[kkojima at gcc dot gnu.org]

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

--- Comment #21 from Kazumoto Kojima <kkojima at gcc dot gnu.org> 2011-12-12 22:08:18 UTC ---
(In reply to comment #20)
> As far as I could observe it, this is mainly triggered by the following in
> sh_legitimate_index_p:
>
> +      if (mode == QImode && (unsigned) INTVAL (op) < 16)
> +    return true;

It seems that, with that hunk, recog.c:offsettable_address_addr_space_p
returns always true for V2SF mode.  Without that hunk, it returns false
for that case.  There are comments and lines in that function like

      /* Use QImode because an odd displacement may be automatically invalid
     for any wider mode.  But it should be valid for a single byte.  */
      good = (*addressp) (QImode, y, as);

where addrssp is *memory_address_addr_space_p which returns true with
that hunk.

> You mean, by giving the user the option to turn off displacement addressing for
> e.g. some specific files / modules by specifying -mno-preferdisp or something
> like that?  By anomalies do you mean code that gets worse because of too much
> pressure on R0 and all the reloads around it, or do you have any other bad use
> cases?

Yes and yes.  Although I didn't look all dis-improvements,
it looks r0 pressure is the primary factor.

> Another thing I could try out is to have load/store insns that allow arbitrary
> operands in displacement addressing like on SH2A, and split them into two insns
> of one load/store and one reg-reg move after reload.  But that would probably
> require the R0 clobber in the expander which could make worse code in cases
> where displacement addressing is not used, I guess.
> Do you think this approach could make sense?

I guess that it could make worse code in some situations as you say.

> Yep, sure.  I've noticed that the latest version of the patch seems to fix some
> more testsuite failures.  I will investigate which hunk is responsible for the
> fixes so that could be pulled out from the patch.  OK?

Sounds great.

[Bug target/50751] SH Target: Displacement addressing does not work for QImode and HImode

[olegendo at gcc dot gnu.org]

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

olegendo at gcc dot gnu.org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |ASSIGNED
   Last reconfirmed|                            |2012-02-26
                 CC|                            |olegendo at gcc dot gnu.org
     Ever Confirmed|0                           |1

[Bug target/50751] SH Target: Displacement addressing does not work for QImode and HImode

[olegendo at gcc dot gnu.org]

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

--- Comment #22 from Oleg Endo <olegendo at gcc dot gnu.org> 2012-03-19 18:59:22 UTC ---
Author: olegendo
Date: Mon Mar 19 18:59:16 2012
New Revision: 185534

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=185534
Log:
    PR target/50751
    * config/sh/sh.h (CONST_OK_FOR_K04, CONST_OK_FOR_K12, DISP_ADDR_P,
    DISP_ADDR_OFFSET): New macros.
    * config/sh/sh.c (sh_address_cost): Add SH2A special case.
    (sh_legitimate_index_p): Allow QImode displacements for non-SH2A.
    (sh_legitimize_address): Add QImode displacement handling.
    (sh_cannot_change_mode_class): Disallow GENERAL_REGS for SFmode 
    vector subregs.
    (sh_secondary_reload): Add QImode displacement handling.
    * config/sh/predicates.md (movsrc_no_disp_mem_operand): New predicate.
    * config/sh/constraints.md (K04, Snd, Sdd): New constraints.
    * config/sh/sh.md (extendqisi2): Remove constraints from expander.
    (*extendqisi2_compact): Rename to *extendqisi2_compact_reg, restrict
    to register operands only.
    (*extendqisi2_compact_mem_disp, *extendqisi2_compact_snd): New insns.
    (extendqihi2): Change insn to expander.
    (*extendqihi2_compact_reg): New insn.
    (movqi_i, movqi): Replace with ... 
    (movqi, *movqi_reg_reg, *movqi_store_mem_disp12,
    *movqi_load_mem_disp, *movqi_load_mem_disp): ... these.
    Add new peepholes for QImode displacement addressing.


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

[Bug target/50751] SH Target: Displacement addressing does not work for QImode and HImode

[olegendo at gcc dot gnu.org]

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

--- Comment #23 from Oleg Endo <olegendo at gcc dot gnu.org> 2012-03-21 20:27:59 UTC ---
Author: olegendo
Date: Wed Mar 21 20:27:50 2012
New Revision: 185619

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=185619
Log:
    PR target/50751
    * gcc/target/sh/pr50751-1.c: New.
    * gcc/target/sh/pr50751-2.c: New.
    * gcc/target/sh/pr50751-3.c: New.


Added:
    trunk/gcc/testsuite/gcc.target/sh/pr50751-1.c
    trunk/gcc/testsuite/gcc.target/sh/pr50751-2.c
    trunk/gcc/testsuite/gcc.target/sh/pr50751-3.c
Modified:
    trunk/gcc/testsuite/ChangeLog

[Bug target/50751] SH Target: Displacement addressing does not work for QImode and HImode

[olegendo at gcc dot gnu.org]

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

--- Comment #24 from Oleg Endo <olegendo at gcc dot gnu.org> 2012-03-27 20:14:57 UTC ---
Author: olegendo
Date: Tue Mar 27 20:14:44 2012
New Revision: 185894

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=185894
Log:
    PR target/50751
    * config/sh/sh.c (sh_legitimize_address, sh_legitimize_reload_address):
    Rearrange conditional logic.  Move displacement address calculations
    to ...
    (sh_find_mov_disp_adjust): ... this new function.


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

[Bug target/50751] SH Target: Displacement addressing does not work for QImode and HImode

[olegendo at gcc dot gnu.org]

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

--- Comment #25 from Oleg Endo <olegendo at gcc dot gnu.org> 2012-04-05 18:43:50 UTC ---
Author: olegendo
Date: Thu Apr  5 18:43:45 2012
New Revision: 186169

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=186169
Log:
    PR target/50751
    * config/sh/sh.c (sh_find_mov_disp_adjust): Take machine_mode as the
    first argument instead of mode size.  Move displacement calculations
    to ...
    (mov_insn_size, max_mov_insn_displacement, mov_insn_alignment_mask): 
    ... these new functions.
    (disp_adjust): Remove max_mov_disp field.
    (sh_legitimate_index_p): Use max_mov_insn_displacement and
    mov_insn_alignment_mask.
    (sh_address_cost): Use max_mov_insn_displacement.


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

[Bug target/50751] SH Target: Displacement addressing does not work for QImode and HImode

[olegendo at gcc dot gnu.org]

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

--- Comment #26 from Oleg Endo <olegendo at gcc dot gnu.org> 2012-04-11 11:35:36 UTC ---
Author: olegendo
Date: Wed Apr 11 11:35:32 2012
New Revision: 186311

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=186311
Log:
    PR target/50751
    * config/sh/sh-protos.h (sh_legitimate_index_p): Add new arguments
    consider_sh2a and allow_zero.
    * config/sh/sh.c (sh_legitimate_index_p): Likewise.
    (disp_addr_displacement): New function.
    (sh_address_cost): Use disp_addr_displacement function instead
    of DISP_ADDR_OFFSET.
    (sh_legitimate_address_p): Adapt to changed
    sh_legitimate_index_p declaration.
    (sh_find_mov_disp_adjust): Remove HImode check.
    (sh_secondary_reload): Add HImode case.  Use satisfies_constraint_Sdd,
    disp_addr_displacement and max_mov_insn_displacement.
    (max_mov_insn_displacement): Remove HImode check.
    * config/sh/sh.h (CONST_OK_FOR_K04, CONST_OK_FOR_K12, 
    DISP_ADDR_P, DISP_ADDR_OFFSET): Remove.
    * config/sh/constraints.md (K05, K13): New constraints.
    (K12): Correct comment.
    (Sdd): Do not use DISP_ADDR_P macro.
    (Snd, Sbw): Use satisfies_constraint_Sdd.
    * config/sh/sh.md (extendhisi2): Remove constraints from expander.
    (*extendhisi2_compact, movhi_i): Remove.
    (*extendhisi2_compact_reg, *extendhisi2_compact_mem_disp,
    *extendhisi2_compact_mem_disp, *extendhisi2_compact_snd,
    *movhi_reg_reg, *movhi_store_mem_disp05, *movhi_store_mem_disp13,
    *movhi_load_mem_disp, *movhi_load_mem_disp, *movhi): New insns.
    (*extendqisi2_compact_mem_disp, *extendqisi2_compact_mem_disp,
    *movqi_store_mem_disp04, *movqi_store_mem_disp12, *movqi_load_mem_disp,
    *movqi_load_mem_disp): Use sh_legitimate_index_p instead of
    CONST_OK_FOR_Kxx.
    Add new peepholes for HImode displacement addressing.


Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/config/sh/constraints.md
    trunk/gcc/config/sh/predicates.md
    trunk/gcc/config/sh/sh-protos.h
    trunk/gcc/config/sh/sh.c
    trunk/gcc/config/sh/sh.h
    trunk/gcc/config/sh/sh.md

[Bug target/50751] SH Target: Displacement addressing does not work for QImode and HImode

[olegendo at gcc dot gnu.org]

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

--- Comment #27 from Oleg Endo <olegendo at gcc dot gnu.org> 2012-04-11 23:01:19 UTC ---
Author: olegendo
Date: Wed Apr 11 23:01:12 2012
New Revision: 186365

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=186365
Log:
    PR target/50751
    * gcc/target/sh/pr50751-4.c: New.
    * gcc/target/sh/pr50751-5.c: New.
    * gcc/target/sh/pr50751-6.c: New.
    * gcc/target/sh/pr50751-7.c: New.


Added:
    trunk/gcc/testsuite/gcc.target/sh/pr50751-4.c
    trunk/gcc/testsuite/gcc.target/sh/pr50751-5.c
    trunk/gcc/testsuite/gcc.target/sh/pr50751-6.c
    trunk/gcc/testsuite/gcc.target/sh/pr50751-7.c
Modified:
    trunk/gcc/testsuite/ChangeLog

[Bug target/50751] SH Target: Displacement addressing does not work for QImode and HImode

[olegendo at gcc dot gnu.org]

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

--- Comment #28 from Oleg Endo <olegendo at gcc dot gnu.org> 2012-04-19 09:29:41 UTC ---
The prophecy in comment #3 finally came true (again) while I was testing the
patch for PR 52941 ;)
The problem is that when reload tries to swap regs from/to stack, it will try
to use the QI/HI displacement addressing patterns to access the stack (which is
good).  However, if the following insn has an R0 clobber (such as the atomic
sequences) reload will bail out instead of introducing another R0 <-> Rn move
after the QI/HI load/store in order to satisfy the displacement addressing
constraints.
I'm now trying to work around this by adding special insn_and_split patterns
for the reload phase and removing the displacement addressing special cases in
sh_secondary_reload....

[Bug target/50751] SH Target: Displacement addressing does not work for QImode and HImode

[olegendo at gcc dot gnu.org]

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

--- Comment #29 from Oleg Endo <olegendo at gcc dot gnu.org> 2012-04-30 19:37:50 UTC ---
(In reply to comment #28)
> I'm now trying to work around this by adding special insn_and_split patterns
> for the reload phase and removing the displacement addressing special cases in
> sh_secondary_reload....

... without any useful results.
The failing test case is gcc.c-torture/compile/sync-1.c

The problematic insns are:

(insn 218 215 220 3 (set (reg:HI 448 [ D.41678+2 ])
        (subreg:HI (reg:SI 439 [ D.41678 ]) 2))
     (nil))

(insn 220 218 237 3 (parallel [
            (set (reg:SI 447)
                (unspec_volatile:SI [
                        (mem:HI (reg/f:SI 303) [0 S2 A16])
                        (reg:HI 448 [ D.41678+2 ])
                        (reg:HI 442 [ D.41685 ])
                    ] UNSPECV_CMPXCHG_1))
            (set (mem:HI (reg/f:SI 303) [0 S2 A16])
   ...

For example, when R0 is allocated for 448 (which is required to do the
displacement addressing) and then reload tries to allocate regs for 448 or 442,
it will try to allocate them in R0_REGS class only.  This doesn't work of
course, because the atomic insns clobber R0, and it results in a spill failure
for the atomic insn.  Then there is also the case, when subsequent atomic insns
refer to a a reg that was allocated to R0 before.
Ideally, reload should insert a reg-reg move and try to allocate regs outside
the R0_REGS class, but it doesn't.

[Bug target/50751] SH Target: Displacement addressing does not work for QImode and HImode

[olegendo at gcc dot gnu.org]

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

--- Comment #30 from Oleg Endo <olegendo at gcc dot gnu.org> 2012-08-09 15:51:25 UTC ---
Author: olegendo
Date: Thu Aug  9 15:51:20 2012
New Revision: 190257

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=190257
Log:
    PR target/50751
    * config/sh/sh.md (*extendqisi2_compact_reg, *extendhisi2_compact_reg):
    Use arith_reg_operand predicate instead of register_operand.
    * config/sh/predicates.md (movsrc_no_disp_mem_operand): Accept
    only mem, simplify.


Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/config/sh/predicates.md
    trunk/gcc/config/sh/sh.md

[Bug target/50751] SH Target: Displacement addressing does not work for QImode and HImode

[olegendo at gcc dot gnu.org]

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

--- Comment #31 from Oleg Endo <olegendo at gcc dot gnu.org> 2012-08-14 17:54:35 UTC ---
Author: olegendo
Date: Tue Aug 14 17:54:28 2012
New Revision: 190395

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=190395
Log:
    PR target/50751
    * config/sh/constraints.md (Sra): New constraint.
    * config/sh/predicates.md (simple_mem_operand,
    displacement_mem_operand, zero_extend_movu_operand): New predicates.
    (zero_extend_operand): Check zero_extend_movu_operand for SH2A.
    * config/sh/sh.md (*zero_extendqisi2_disp_mem,
    *zero_extendhisi2_disp_mem): Add new insns and two new related
    peephole2 patterns.

    PR target/50751
    * gcc.target/sh/pr50751-8.c: New.


Added:
    trunk/gcc/testsuite/gcc.target/sh/pr50751-8.c
Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/config/sh/constraints.md
    trunk/gcc/config/sh/predicates.md
    trunk/gcc/config/sh/sh.md
    trunk/gcc/testsuite/ChangeLog

[Bug target/50751] SH Target: Displacement addressing does not work for QImode and HImode

[olegendo at gcc dot gnu.org]

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

--- Comment #32 from Oleg Endo <olegendo at gcc dot gnu.org> ---
Author: olegendo
Date: Tue Nov 26 11:48:16 2013
New Revision: 205390

URL: http://gcc.gnu.org/viewcvs?rev=205390&root=gcc&view=rev
Log:
    PR target/58314
    PR target/50751
    * config/sh/sh.c (max_mov_insn_displacement, disp_addr_displacement):
    Prefix function names with 'sh_'.  Make them non-static.
    * config/sh/sh-protos.h (sh_disp_addr_displacement,
    sh_max_mov_insn_displacement): Add declarations.
    * config/sh/constraints.md (Q): Reject QImode.
    (Sdd): Use match_code "mem".
    (Snd): Fix erroneous matching of non-memory operands.
    * config/sh/predicates.md (short_displacement_mem_operand): New
    predicate.
    (general_movsrc_operand): Disallow PC relative QImode loads.
    * config/sh/sh.md (*mov<mode>_reg_reg): Remove it.
    (*movqi, *movhi): Merge both insns into...
    (*mov<mode>): ... this new insn.  Replace generic 'm' constraints with
    'Snd' and 'Sdd' constraints.  Calculate insn length dynamically based
    on the operand types.


Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/config/sh/constraints.md
    trunk/gcc/config/sh/predicates.md
    trunk/gcc/config/sh/sh-protos.h
    trunk/gcc/config/sh/sh.c
    trunk/gcc/config/sh/sh.md

[Bug target/50751] SH Target: Displacement addressing does not work for QImode and HImode

[olegendo at gcc dot gnu.org]

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

--- Comment #33 from Oleg Endo <olegendo at gcc dot gnu.org> ---
Author: olegendo
Date: Fri Dec  6 19:34:23 2013
New Revision: 205759

URL: http://gcc.gnu.org/viewcvs?rev=205759&root=gcc&view=rev
Log:
    Backport from mainline
    2013-11-26  Oleg Endo  <olegendo@gcc.gnu.org>

    PR target/58314
    PR target/50751
    * config/sh/sh.c (max_mov_insn_displacement, disp_addr_displacement):
    Prefix function names with 'sh_'.  Make them non-static.
    * config/sh/sh-protos.h (sh_disp_addr_displacement,
    sh_max_mov_insn_displacement): Add declarations.
    * config/sh/constraints.md (Q): Reject QImode.
    (Sdd): Use match_code "mem".
    (Snd): Fix erroneous matching of non-memory operands.
    * config/sh/predicates.md (short_displacement_mem_operand): New
    predicate.
    (general_movsrc_operand): Disallow PC relative QImode loads.
    * config/sh/sh.md (*mov<mode>_reg_reg): Remove it.
    (*movqi, *movhi): Merge both insns into...
    (*mov<mode>): ... this new insn.  Replace generic 'm' constraints with
    'Snd' and 'Sdd' constraints.  Calculate insn length dynamically based
    on the operand types.


Modified:
    branches/gcc-4_8-branch/gcc/ChangeLog
    branches/gcc-4_8-branch/gcc/config/sh/constraints.md
    branches/gcc-4_8-branch/gcc/config/sh/predicates.md
    branches/gcc-4_8-branch/gcc/config/sh/sh-protos.h
    branches/gcc-4_8-branch/gcc/config/sh/sh.c
    branches/gcc-4_8-branch/gcc/config/sh/sh.md

[Bug target/50751] SH Target: Displacement addressing does not work for QImode and HImode

[olegendo at gcc dot gnu.org]

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

--- Comment #34 from Oleg Endo <olegendo at gcc dot gnu.org> ---
(In reply to Oleg Endo from comment #29)
> (In reply to comment #28)
> > I'm now trying to work around this by adding special insn_and_split patterns
> > for the reload phase and removing the displacement addressing special cases in
> > sh_secondary_reload....
> 
> ... without any useful results.
> The failing test case is gcc.c-torture/compile/sync-1.c
> 
> The problematic insns are:
> 
> (insn 218 215 220 3 (set (reg:HI 448 [ D.41678+2 ])
>         (subreg:HI (reg:SI 439 [ D.41678 ]) 2))
>      (nil))
> 
> (insn 220 218 237 3 (parallel [
>             (set (reg:SI 447)
>                 (unspec_volatile:SI [
>                         (mem:HI (reg/f:SI 303) [0 S2 A16])
>                         (reg:HI 448 [ D.41678+2 ])
>                         (reg:HI 442 [ D.41685 ])
>                     ] UNSPECV_CMPXCHG_1))
>             (set (mem:HI (reg/f:SI 303) [0 S2 A16])
>    ...
> 
> For example, when R0 is allocated for 448 (which is required to do the
> displacement addressing) and then reload tries to allocate regs for 448 or
> 442, it will try to allocate them in R0_REGS class only.  This doesn't work
> of course, because the atomic insns clobber R0, and it results in a spill
> failure for the atomic insn.  Then there is also the case, when subsequent
> atomic insns refer to a a reg that was allocated to R0 before.
> Ideally, reload should insert a reg-reg move and try to allocate regs
> outside the R0_REGS class, but it doesn't.

I've tried to reproduce this on trunk rev 205756 (4.9) and the problem seems to
be gone.

[Bug target/50751] SH Target: Displacement addressing does not work for QImode and HImode

[olegendo at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
         Depends on|                            |55212

--- Comment #36 from Oleg Endo <olegendo at gcc dot gnu.org> ---
(In reply to Oleg Endo from comment #35)
> (In reply to Oleg Endo from comment #34)
> > 
> Compiling with -Os -m4 -ml -matomic-model=soft-gusa, I get:
> 
> 
> Probably we really should try switching to LRA (PR 55212).

I've tried compiling the problematic test case with the sh-lra branch (LRA
enabled) and '-Os -m4 -ml -matomic-model=soft-gusa' and '-Os -m4 -mb
-matomic-model=soft-gusa'.  The problem seems to be gone for those parameters,
but when compiling for '-O2 -m4 -ml -matomic-model=soft-gusa' it crashes:

internal compiler error: in check_rtl, at lra.c:1928
 TEST_FUNCS (complex_float_add, _Complex float, , += 1, 0, 20000)
 ^
0x8523d00 check_rtl
    ../../gcc-sh-lra/gcc/lra.c:1928
0x85273f7 lra(_IO_FILE*)
    ../../gcc-sh-lra/gcc/lra.c:2318
0x84e6823 do_reload
    ../../gcc-sh-lra/gcc/ira.c:5306
0x84e6823 execute
    ../../gcc-sh-lra/gcc/ira.c:5465

[Bug target/50751] SH Target: Displacement addressing does not work for QImode and HImode

[olegendo at gcc dot gnu.org]

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

--- Comment #37 from Oleg Endo <olegendo at gcc dot gnu.org> ---
Author: olegendo
Date: Sun Dec  7 22:57:11 2014
New Revision: 218469

URL: https://gcc.gnu.org/viewcvs?rev=218469&root=gcc&view=rev
Log:
gcc/
    PR target/50751
    * config/sh/sh.md (extendqihi2): Allow only for TARGET_SH1.

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

[Bug target/50751] SH Target: Displacement addressing does not work for QImode and HImode

[olegendo at gcc dot gnu.org]

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

--- Comment #38 from Oleg Endo <olegendo at gcc dot gnu.org> ---
Author: olegendo
Date: Sun Dec  7 22:58:49 2014
New Revision: 218470

URL: https://gcc.gnu.org/viewcvs?rev=218470&root=gcc&view=rev
Log:
gcc/
    Backport from mainline
    2014-12-07  Oleg Endo  <olegendo@gcc.gnu.org>

    PR target/50751
    * config/sh/sh.md (extendqihi2): Allow only for TARGET_SH1.

Modified:
    branches/gcc-4_9-branch/gcc/ChangeLog
    branches/gcc-4_9-branch/gcc/config/sh/sh.md

[Bug target/50751] SH Target: Displacement addressing does not work for QImode and HImode

[olegendo at gcc dot gnu.org]

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

--- Comment #39 from Oleg Endo <olegendo at gcc dot gnu.org> ---
Author: olegendo
Date: Sun Dec  7 23:00:30 2014
New Revision: 218471

URL: https://gcc.gnu.org/viewcvs?rev=218471&root=gcc&view=rev
Log:
gcc/
    Backport from mainline
    2014-12-07  Oleg Endo  <olegendo@gcc.gnu.org>

    PR target/50751
    * config/sh/sh.md (extendqihi2): Allow only for TARGET_SH1.

Modified:
    branches/gcc-4_8-branch/gcc/ChangeLog
    branches/gcc-4_8-branch/gcc/config/sh/sh.md

[Bug target/50751] SH Target: Displacement addressing does not work for QImode and HImode

[olegendo at gcc dot gnu.org]

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

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

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

--- Comment #40 from Oleg Endo <olegendo at gcc dot gnu.org> ---
I'd like to close this PR as fixed.  The R0 splill failure issues are problems
on their own, which are just emphasized by QIHImode displacement addressing
insns.