[Bug target/39423] [4.3/4.4/4.5/4.6/4.7 Regression] [SH] performance regression: lost mov @(disp,Rn)

[Bug target/39423] [4.3/4.4/4.5/4.6/4.7 Regression] [SH] performance regression: lost mov @(disp,Rn)

[rguenth at gcc dot gnu.org]

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

Richard Guenther <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|4.3.6                       |4.4.7

--- Comment #13 from Richard Guenther <rguenth at gcc dot gnu.org> 2011-06-27 12:13:44 UTC ---
4.3 branch is being closed, moving to 4.4.7 target.

[Bug target/39423] [4.5/4.6/4.7/4.8 Regression] [SH] performance regression: lost mov @(disp,Rn)

[jakub at gcc dot gnu.org]

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

Jakub Jelinek <jakub at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|4.4.7                       |4.5.4

--- Comment #14 from Jakub Jelinek <jakub at gcc dot gnu.org> 2012-03-13 12:46:43 UTC ---
4.4 branch is being closed, moving to 4.5.4 target.

[Bug target/39423] [4.6/4.7/4.8 Regression] [SH] performance regression: lost mov @(disp,Rn)

[rguenth at gcc dot gnu.org]

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

Richard Guenther <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|4.5.4                       |4.6.4

[Bug target/39423] [4.6/4.7/4.8 Regression] [SH] performance regression: lost mov @(disp,Rn)

[olegendo at gcc dot gnu.org]

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

--- Comment #15 from Oleg Endo <olegendo at gcc dot gnu.org> 2012-07-06 22:04:15 UTC ---
Created attachment 27754
  --> http://gcc.gnu.org/bugzilla/attachment.cgi?id=27754
combine pass based patch

A possible combine pass based solution for the problem.
It fixes the case mentioned in the original description.

I've also briefly checked some CSiBE code size results with this
patch applied to rev 189338 for '-m4-single -ml -O2 -mpretend-cmove'.
It hits only a few spots, but if those are buried inside loops, it might
be a win.

I'm not sure what's the best thing to do if the mov.l displacement
goes out of range...

Chris, I know it's been a while but any old memories? :)

[Bug target/39423] [4.6/4.7/4.8 Regression] [SH] performance regression: lost mov @(disp,Rn)

[chrbr at gcc dot gnu.org]

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

--- Comment #16 from chrbr at gcc dot gnu.org 2012-07-11 09:05:03 UTC ---
humm I forgot about this case. It works in one of my dev branches, Let me
extract the uncommitted change and send it to gcc-patches.

Cheers

Christian

(In reply to comment #15)
> Created attachment 27754 [details]
> combine pass based patch
> 
> A possible combine pass based solution for the problem.
> It fixes the case mentioned in the original description.
> 
> I've also briefly checked some CSiBE code size results with this
> patch applied to rev 189338 for '-m4-single -ml -O2 -mpretend-cmove'.
> It hits only a few spots, but if those are buried inside loops, it might
> be a win.
> 
> I'm not sure what's the best thing to do if the mov.l displacement
> goes out of range...
> 
> Chris, I know it's been a while but any old memories? :)

(In reply to comment #15)
> Created attachment 27754 [details]
> combine pass based patch
> 
> A possible combine pass based solution for the problem.
> It fixes the case mentioned in the original description.
> 
> I've also briefly checked some CSiBE code size results with this
> patch applied to rev 189338 for '-m4-single -ml -O2 -mpretend-cmove'.
> It hits only a few spots, but if those are buried inside loops, it might
> be a win.
> 
> I'm not sure what's the best thing to do if the mov.l displacement
> goes out of range...
> 
> Chris, I know it's been a while but any old memories? :)

[Bug target/39423] [4.6/4.7/4.8 Regression] [SH] performance regression: lost mov @(disp,Rn)

[chrbr at gcc dot gnu.org]

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

--- Comment #17 from chrbr at gcc dot gnu.org 2012-07-11 12:35:32 UTC ---
Created attachment 27775
  --> http://gcc.gnu.org/bugzilla/attachment.cgi?id=27775
plus add combine

Here is the patch that I've been running since some time, it also use the same
combine pattern matcher, but the goal of this patch was originally to fix up
chains of multiple mult-add instructions.
Optimizing the cst+reg addressing mode appears as a nice effects. Out of range
indexes seems to be handled as afar as I can see.

This brings a EEMBC telecom speedup of 10%.FFMPEG code size reduced to 30% on a
few objects. 
Validated on whole linux distribution, with only improvements (few regression
only bellow noise).

This patch is only for comments/illustration. Need a few polishing before
proposing. I'm having a look at your implementation to see how they compare and
possibly combined together. Both approaches look interesting.

[Bug target/39423] [4.6/4.7/4.8 Regression] [SH] performance regression: lost mov @(disp,Rn)

[olegendo at gcc dot gnu.org]

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

--- Comment #18 from Oleg Endo <olegendo at gcc dot gnu.org> 2012-07-11 15:09:02 UTC ---
(In reply to comment #17)
> Created attachment 27775 [details]
> plus add combine
> 
> Here is the patch that I've been running since some time, it also use the same
> combine pattern matcher, but the goal of this patch was originally to fix up
> chains of multiple mult-add instructions.
> Optimizing the cst+reg addressing mode appears as a nice effects. Out of range
> indexes seems to be handled as afar as I can see.
> 
> This brings a EEMBC telecom speedup of 10%.FFMPEG code size reduced to 30% on a
> few objects. 
> Validated on whole linux distribution, with only improvements (few regression
> only bellow noise).

Interesting.  
BTW, do you happen to have any (runtime) numbers for GCC 4.7.x vs current GCC
4.8?

> This patch is only for comments/illustration. Need a few polishing before
> proposing. I'm having a look at your implementation to see how they compare and
> possibly combined together. Both approaches look interesting.

I guess folding the mul-add sequences like you did should be more useful than
just
handling one mem:SI pattern.  In any case, if you find my impl useful please
let me know,
because then I'd also pop in patterns for mem:QI and mem:HI patterns.

[Bug target/39423] [4.6/4.7/4.8 Regression] [SH] performance regression: lost mov @(disp,Rn)

[chrbr at gcc dot gnu.org]

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

--- Comment #19 from chrbr at gcc dot gnu.org 2012-07-11 15:24:27 UTC ---
(In reply to comment #18)
> (In reply to comment #17)
> > Created attachment 27775 [details]
> > plus add combine
> > 
> > Here is the patch that I've been running since some time, it also use the same
> > combine pattern matcher, but the goal of this patch was originally to fix up
> > chains of multiple mult-add instructions.
> > Optimizing the cst+reg addressing mode appears as a nice effects. Out of range
> > indexes seems to be handled as afar as I can see.
> > 
> > This brings a EEMBC telecom speedup of 10%.FFMPEG code size reduced to 30% on a
> > few objects. 
> > Validated on whole linux distribution, with only improvements (few regression
> > only bellow noise).
> 
> Interesting.  
> BTW, do you happen to have any (runtime) numbers for GCC 4.7.x vs current GCC
> 4.8?
> 

for now I only track the 4.6 and 4.7 branches. the 4.8 is moving too fast, but
I could easily cheery-pick your the other SH changes (like your fix for
PR53911) 

btw I only bench on the SH4 and SH4A.

> > This patch is only for comments/illustration. Need a few polishing before
> > proposing. I'm having a look at your implementation to see how they compare and
> > possibly combined together. Both approaches look interesting.
> 
> I guess folding the mul-add sequences like you did should be more useful than
> just
> handling one mem:SI pattern.  In any case, if you find my impl useful please
> let me know,
> because then I'd also pop in patterns for mem:QI and mem:HI patterns.

sure. by the way, my patch is not complete to fix the original problem. I need
to extract other chunks that unleash it. Will post.

[Bug target/39423] [4.6/4.7/4.8 Regression] [SH] performance regression: lost mov @(disp,Rn)

[chrbr at gcc dot gnu.org]

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

--- Comment #20 from chrbr at gcc dot gnu.org 2012-07-12 15:36:59 UTC ---
> I'm having a look at your implementation to see how they compare and
> > possibly combined together. Both approaches look interesting.
> 
> I guess folding the mul-add sequences like you did should be more useful than
> just
> handling one mem:SI pattern.  In any case, if you find my impl useful please
> let me know,
> because then I'd also pop in patterns for mem:QI and mem:HI patterns.

Finally, I think both combine pattern are not exclusive and rather independent,
even if interacting each other due to combine orders.

I like your mem,plus,plus combine that look better than my folding. just there
are 2 little details that I tried while playing with it: there was a
lost of generality with the hardcoding of the shift constant, and I'm not sure
if there was a risk of clobbering a live operands[1]. For those reasons I
modified it a bit as followed. 

(define_insn_and_split "*movsi_disp"
  [(set (match_operand:SI 0 "arith_reg_dest" "=r")
        (mem:SI (plus:SI
          (plus:SI (mult:SI (match_operand:SI 1 "arith_reg_operand" "r")
                    (match_operand:SI 2 "const_int_operand" "i"))
               (match_operand:SI 3 "arith_reg_operand" "K06"))
        (match_operand:SI 4 "const_int_operand" "i"))))]
  "TARGET_SH1 && satisfies_constraint_K06 (operands[4]) && exact_log2 (INTVAL
(operands[2])) > 0"
{
  gcc_unreachable ();
  return "#";
}
  "&& 1"
  [(set (match_dup 1) (ashift:SI (match_dup 1) (match_dup 2)))
   (set (match_dup 1) (plus:SI (match_dup 1) (match_dup 3)))
   (set (match_dup 0) (mem:SI (plus:SI (match_dup 1) (match_dup 4))))]
"{
    int n = exact_log2 (INTVAL (operands[2]));
    rtx res = gen_reg_rtx (SImode);
    emit_move_insn (res, operands[1]);

    operands[1] = res;
    operands[2] = GEN_INT (n);
}"
)

(define_insn "ashlsi3_k"
  [(set (match_operand:SI 0 "arith_reg_dest" "=r,r,r")
    (ashift:SI (match_operand:SI 1 "arith_reg_operand" "0,0,0")
           (match_operand:SI 2 "const_int_operand" "M,P27,r")))]
  "TARGET_SH1"
  "@
    add    %0,%0
    shll%O2    %0
    shld    %2,%0"
  [(set_attr "type" "arith")])

Using those changes, a snipet like 

int
foo4 (long long tab[], int index)
{
  return (int)tab [index+4];
}

not compiles as:

       mov     #3,r1
        shld    r1,r5
        add     r4,r5
        rts
        mov.l   @(32,r5),r0

2)plus_mulsi

I see some interactions with the movsi_disp pattern, due to different ordering
on the matching in the combiner. I'll need to play more this them activated
together.

So I think you can go ahead with your combiner movsi_disp pattern and propose
to Kaz when ready to close this defect. Feel free to take or not my
suggestions.

I'll then go with my plus_mulsi combiner in a second time, making clear that
it's provide additional optimization opportunities without mixing the impacts.

On a related thread, for further work, I'm thinking on adding support for the
MAC instruction, now that was have the multiply and add. But this requires
exposing the MACLH registers to reload. Had anyone had a thought on this ? I'd
like to give this a try pretty soon.

Cheers

[Bug target/39423] [4.6/4.7/4.8 Regression] [SH] performance regression: lost mov @(disp,Rn)

[olegendo at gcc dot gnu.org]

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

--- Comment #21 from Oleg Endo <olegendo at gcc dot gnu.org> 2012-07-13 09:36:36 UTC ---
(In reply to comment #20)
> I like your mem,plus,plus combine that look better than my folding. just there
> are 2 little details that I tried while playing with it: there was a
> lost of generality with the hardcoding of the shift constant,

Yeah, sure.  The hardcoded '4' was just to see if/how it could work at all.

> and I'm not sure
> if there was a risk of clobbering a live operands[1]. For those reasons I
> modified it a bit as followed. 
> 
> (define_insn_and_split "*movsi_disp"
>   [(set (match_operand:SI 0 "arith_reg_dest" "=r")
>         (mem:SI (plus:SI
>           (plus:SI (mult:SI (match_operand:SI 1 "arith_reg_operand" "r")
>                     (match_operand:SI 2 "const_int_operand" "i"))
>                (match_operand:SI 3 "arith_reg_operand" "K06"))
>         (match_operand:SI 4 "const_int_operand" "i"))))]
>   "TARGET_SH1 && satisfies_constraint_K06 (operands[4]) && exact_log2 (INTVAL
> (operands[2])) > 0"
> {
>   gcc_unreachable ();
>   return "#";
> }
>   "&& 1"
>   [(set (match_dup 1) (ashift:SI (match_dup 1) (match_dup 2)))
>    (set (match_dup 1) (plus:SI (match_dup 1) (match_dup 3)))
>    (set (match_dup 0) (mem:SI (plus:SI (match_dup 1) (match_dup 4))))]
> "{
>     int n = exact_log2 (INTVAL (operands[2]));
>     rtx res = gen_reg_rtx (SImode);
>     emit_move_insn (res, operands[1]);
> 
>     operands[1] = res;
>     operands[2] = GEN_INT (n);
> }"
> )
> 

Uhm, I think it would be enough to do the following in the split preparation...

(define_insn_and_split "*movsi_disp"
  [(set (match_operand:SI 0 "arith_reg_dest" "=r")
        (mem:SI (plus:SI
          (plus:SI (mult:SI (match_operand:SI 1 "arith_reg_operand" "r")
                    (match_operand:SI 2 "const_int_operand" "i"))
               (match_operand:SI 3 "arith_reg_operand" "K06"))
        (match_operand:SI 4 "const_int_operand" "i"))))]
  "TARGET_SH1 && satisfies_constraint_K06 (operands[4]) && exact_log2 (INTVAL
(operands[2])) > 0"
{
  gcc_unreachable ();
  return "#";
}
  "&& can_create_pseudo_p ()"
  [(set (match_dup 5) (ashift:SI (match_dup 1) (match_dup 2)))
   (set (match_dup 6) (plus:SI (match_dup 5) (match_dup 3)))
   (set (match_dup 0) (mem:SI (plus:SI (match_dup 6) (match_dup 4))))]
{
    operands[5] = gen_reg_rtx (SImode);
    operands[6] = gen_reg_rtx (SImode);
    operands[2] = GEN_INT (exact_log2 (INTVAL (operands[2])));
})


> (define_insn "ashlsi3_k"
>   [(set (match_operand:SI 0 "arith_reg_dest" "=r,r,r")
>     (ashift:SI (match_operand:SI 1 "arith_reg_operand" "0,0,0")
>            (match_operand:SI 2 "const_int_operand" "M,P27,r")))]
>   "TARGET_SH1"
>   "@
>     add    %0,%0
>     shll%O2    %0
>     shld    %2,%0"
>   [(set_attr "type" "arith")])
> 

The shld insn is only available on SH3 and above or SH2A.
Allowing arbitrary shifts definitely makes sense here, but then maybe 
we should use the existing shift-expansion, although it sometimes
causes problems (like it ends up generating functions calls for shll8 on
SH2..).
Another option might be to do these combine patterns on < SH3 only 
when the resulting shift costs do not pass a certain threshold.

> Using those changes, a snipet like 
> 
> int
> foo4 (long long tab[], int index)
> {
>   return (int)tab [index+4];
> }
> 
> not compiles as:
> 
>        mov     #3,r1
>         shld    r1,r5
>         add     r4,r5
>         rts
>         mov.l   @(32,r5),r0
> 

If I recall correctly, a sequence such as "mov #3,r1; shld r1,r5" will be
slower
than "shll r5; shll2 r5" due to pipelining issues.  But that's another story.


> 2)plus_mulsi
> 
> I see some interactions with the movsi_disp pattern, due to different ordering
> on the matching in the combiner. I'll need to play more this them activated
> together.
> 
> So I think you can go ahead with your combiner movsi_disp pattern and propose
> to Kaz when ready to close this defect. Feel free to take or not my
> suggestions.
> 
> I'll then go with my plus_mulsi combiner in a second time, making clear that
> it's provide additional optimization opportunities without mixing the impacts.
> 

OK, I'll brush up the 'movsi_disp' stuff a little, taking our input into
account.
It might take a couple of days though.

> On a related thread, for further work, I'm thinking on adding support for the
> MAC instruction, now that was have the multiply and add. But this requires
> exposing the MACLH registers to reload. Had anyone had a thought on this ? I'd
> like to give this a try pretty soon.
> 

I have created PR 53949 for this.

[Bug target/39423] [4.6/4.7/4.8 Regression] [SH] performance regression: lost mov @(disp,Rn)

[olegendo at gcc dot gnu.org]

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

--- Comment #22 from Oleg Endo <olegendo at gcc dot gnu.org> 2012-07-30 06:48:45 UTC ---
Author: olegendo
Date: Mon Jul 30 06:48:40 2012
New Revision: 189954

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=189954
Log:
    PR target/39423
    * config/gcc/sh/sh.md (*movsi_index_disp, *movhi_index_disp): New insns.


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

[Bug target/39423] [4.6/4.7/4.8 Regression] [SH] performance regression: lost mov @(disp,Rn)

[olegendo at gcc dot gnu.org]

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

--- Comment #23 from Oleg Endo <olegendo at gcc dot gnu.org> 2012-08-09 15:58:08 UTC ---
Author: olegendo
Date: Thu Aug  9 15:58:04 2012
New Revision: 190259

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=190259
Log:
    PR target/39423
    * config/sh/predicates.md (mem_index_disp_operand): New predicate.
    * config/sh/sh.md (*movsi_index_disp): Rewrite insns to use the new
    mem_index_disp_operand predicate.

    PR target/39423
    * gcc.target/sh/pr39423-1.c: New.


Added:
    trunk/gcc/testsuite/gcc.target/sh/pr39423-1.c
Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/config/sh/predicates.md
    trunk/gcc/config/sh/sh.md
    trunk/gcc/testsuite/ChangeLog

[Bug target/39423] [4.6/4.7/4.8 Regression] [SH] performance regression: lost mov @(disp,Rn)

[olegendo at gcc dot gnu.org]

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

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

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

--- Comment #24 from Oleg Endo <olegendo at gcc dot gnu.org> 2012-08-09 16:55:19 UTC ---
Christian, do you have anything to add regarding this matter?

I'm not sure whether this should be back ported to 4.6.x or 4.7.x or not.  Kaz,
what do you think?

[Bug target/39423] [4.6/4.7/4.8 Regression] [SH] performance regression: lost mov @(disp,Rn)

[kkojima at gcc dot gnu.org]

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

--- Comment #25 from Kazumoto Kojima <kkojima at gcc dot gnu.org> 2012-08-09 22:44:41 UTC ---
(In reply to comment #24)
> I'm not sure whether this should be back ported to 4.6.x or 4.7.x or not.  Kaz,
> what do you think?

Looks a bit intrusive for the stable branches.

[Bug target/39423] [4.6/4.7/4.8 Regression] [SH] performance regression: lost mov @(disp,Rn)

[chrbr at gcc dot gnu.org]

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

--- Comment #26 from chrbr at gcc dot gnu.org 2012-08-10 08:08:45 UTC ---
(In reply to comment #24)
> Christian, do you have anything to add regarding this matter?
> 
> I'm not sure whether this should be back ported to 4.6.x or 4.7.x or not.  Kaz,
> what do you think?

I got an ICE in elimination_costs_in_insn, at reload1.c:3654 when applying to
the 4.7 branch, but it seems to be resolved on trunk (hoping it's not hidden)
so I guess you can close this defect as resolved.

I'm about to submit a separate patch for the second opportunity I mentioned in
comment #17, as soon as it's polished up and I can assess some performance.

[Bug target/39423] [4.6/4.7/4.8 Regression] [SH] performance regression: lost mov @(disp,Rn)

[olegendo at gcc dot gnu.org]

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

--- Comment #27 from Oleg Endo <olegendo at gcc dot gnu.org> 2012-08-10 12:26:52 UTC ---
(In reply to comment #26)
> 
> I got an ICE in elimination_costs_in_insn, at reload1.c:3654 when applying to
> the 4.7 branch, but it seems to be resolved on trunk (hoping it's not hidden)
> so I guess you can close this defect as resolved.

This is probably due to the splitter part of the *movsi_index_disp patterns:

  [(set (match_dup 5) (ashift:SI (match_dup 1) (match_dup 2)))
   (set (match_dup 6) (plus:SI (match_dup 5) (match_dup 3)))
   (set (match_dup 0) (match_dup 7))]

The left shift won't match after this has been split, because there's no such
shift pattern.  On 4.7 it could be avoided by manually emitting the appropriate
shift insn in the preparation block of the splitter.  However, I'm not sure
whether it will be correct, due to the scratch regs and T_REG clobber in the
left shift patterns etc ... 
On trunk it is OK because the log2 shift sequences of the *movsi_index_disp
patterns will never clobber the T_REG... at least that's what I initially
thought, which is obviously wrong.  Left shifts of 15, 23, 29, 31 clobber T_REG
(if they are not converted to dynamic shifts).  These shift amounts are
probably not very likely to happen (e.g. tab[i * 8192 + 4]).  Still, they
should be handled.  I'll have a look at it, so let's keep this PR open for a
little bit longer...

[Bug target/39423] [4.6/4.7/4.8 Regression] [SH] performance regression: lost mov @(disp,Rn)

[rmansfield at qnx dot com]

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

Ryan Mansfield <rmansfield at qnx dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |rmansfield at qnx dot com

--- Comment #28 from Ryan Mansfield <rmansfield at qnx dot com> 2012-08-10 13:14:33 UTC ---
(In reply to comment #27)
> (In reply to comment #26)
> > 
> > I got an ICE in elimination_costs_in_insn, at reload1.c:3654 when applying to
> > the 4.7 branch, but it seems to be resolved on trunk (hoping it's not hidden)

This ICE does happen on trunk (rev190294). I have a testcase for it that I'm
reducing. There was a second ICE introduced by 190259 as well. 

/home/ryan/ice2.i:9312:1: error: unrecognizable insn:
 }
 ^
(insn 780 647 781 28 (set (reg:SI 530)
        (ashift:SI (reg:SI 147 t)
            (const_int 1 [0x1]))) /home/ryan/ice2.i:9270 -1
     (nil))
/home/ryan/ice2.i:9312:1: internal compiler error: in extract_insn, at
recog.c:2125

Should I attach the testcases to this PR or open a new PR?

[Bug target/39423] [4.6/4.7/4.8 Regression] [SH] performance regression: lost mov @(disp,Rn)

[olegendo at gcc dot gnu.org]

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

--- Comment #29 from Oleg Endo <olegendo at gcc dot gnu.org> 2012-08-10 13:26:52 UTC ---
(In reply to comment #28)
> 
> This ICE does happen on trunk (rev190294). I have a testcase for it that I'm
> reducing. There was a second ICE introduced by 190259 as well. 
> 
> /home/ryan/ice2.i:9312:1: error: unrecognizable insn:
>  }
>  ^
> (insn 780 647 781 28 (set (reg:SI 530)
>         (ashift:SI (reg:SI 147 t)
>             (const_int 1 [0x1]))) /home/ryan/ice2.i:9270 -1
>      (nil))
> /home/ryan/ice2.i:9312:1: internal compiler error: in extract_insn, at
> recog.c:2125
> 
> Should I attach the testcases to this PR or open a new PR?

This left shift ICE looks like a problem I introduced with changes in PR 54089.
Please post the testcase for this in PR 54089.

BTW, I've confirmed my guess in comment #27 that the following function

int test00 (int tab[], int i)
{
  return tab[i * 8192 + 4];
}

will currently ICE when compiled for -m2 and will produce potentially wrong
code around the T_REG for -m2a and -m3.  Checking it out...

[Bug target/39423] [4.6/4.7/4.8 Regression] [SH] performance regression: lost mov @(disp,Rn)

[rmansfield at qnx dot com]

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

--- Comment #30 from Ryan Mansfield <rmansfield at qnx dot com> 2012-08-10 13:36:42 UTC ---
Created attachment 27983
  --> http://gcc.gnu.org/bugzilla/attachment.cgi?id=27983
preprocessed src

Testcase that reproduces elimination_costs_in_insn ICE with -m4 -O2

[Bug target/39423] [4.6/4.7/4.8 Regression] [SH] performance regression: lost mov @(disp,Rn)

[olegendo at gcc dot gnu.org]

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

--- Comment #31 from Oleg Endo <olegendo at gcc dot gnu.org> 2012-08-12 13:23:24 UTC ---
Author: olegendo
Date: Sun Aug 12 13:23:20 2012
New Revision: 190326

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=190326
Log:
    PR target/39423
    * config/sh/predicates.md (mem_index_disp_operand): Check for
    arith_reg_operand instead of REG_P.

    PR target/39423
    * gcc.c-torture/compile/pr39423-1.c: New.
    * gcc.c-torture/compile/pr39423-2.c: New.


Added:
    trunk/gcc/testsuite/gcc.c-torture/compile/pr39423-1.c
    trunk/gcc/testsuite/gcc.c-torture/compile/pr39423-2.c
Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/config/sh/predicates.md
    trunk/gcc/testsuite/ChangeLog

[Bug target/39423] [4.6/4.7/4.8 Regression] [SH] performance regression: lost mov @(disp,Rn)

[olegendo at gcc dot gnu.org]

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

--- Comment #32 from Oleg Endo <olegendo at gcc dot gnu.org> 2012-08-16 23:16:57 UTC ---
Author: olegendo
Date: Thu Aug 16 23:16:53 2012
New Revision: 190458

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=190458
Log:
    PR target/39423
    * config/sh/sh.md (*movsi_index_disp, *movhi_index_disp): Handle
    potential T_REG clobber.  Convert zero extending split to
    insn_and_split.


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

[Bug target/39423] [4.6/4.7/4.8 Regression] [SH] performance regression: lost mov @(disp,Rn)

[olegendo at gcc dot gnu.org]

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

--- Comment #33 from Oleg Endo <olegendo at gcc dot gnu.org> 2012-08-21 23:35:02 UTC ---
Author: olegendo
Date: Tue Aug 21 23:34:54 2012
New Revision: 190579

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=190579
Log:
    PR target/39423
    * config/sh/sh.md (*movhi_index_disp): Add support for SH2A movu.w insn.

    PR target/39423
    * gcc.target/sh/pr39423-2.c: New.


Added:
    trunk/gcc/testsuite/gcc.target/sh/pr39423-2.c
Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/config/sh/sh.md
    trunk/gcc/testsuite/ChangeLog

[Bug target/39423] [4.6/4.7/4.8 Regression] [SH] performance regression: lost mov @(disp,Rn)

[olegendo at gcc dot gnu.org]

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

--- Comment #34 from Oleg Endo <olegendo at gcc dot gnu.org> 2012-08-29 19:05:35 UTC ---
Christian, regarding your message on the patches list:
http://gcc.gnu.org/ml/gcc-patches/2012-08/msg01861.html

I ended up 'fixing' the issue by folding the patterns with iterators. 
Committed with other foldings as rev. 190781.
There are now 3 patterns: "*movsi_index_disp_load", "*movhi_index_disp_load",
"*mov<mode>_index_disp_store".

On a related matter... I've been thinking about PR 50749 (auto-inc-dec
opportunities) and PR 53911 (displacement addressing base address selection).
As far as I understand it, the proper way of fixing those would be implementing
something like General Offset Assignment (GOA) or Address Register Assignment
(ARA).  Just to see how it could be done, I've replaced the auto-inc-dec pass
with my own very early prototype implementation for optimizing address mode
selection (currently only with SH addr modes in mind, but could be extended to
apply to other targets, too).
I'm not sure whether I will be able to get it into shape for 4.8, but early
results show that it conflicts with the combine patterns introduced in the
fixes for this PR, as it would try to turn the scaled index + displacement
addresses into post-inc addresses or normal displacement addresses.  The
problem here is that SH officially does not support scaled index + displacement
addressing, and address mode selection / auto-inc-dec is done before combine.

Anyway, until the proper way has been figured out the combine patterns should
do as a temporary fix.  BTW, DImode and SFmode/DFmode for SH2A are still
missing ...

[Bug target/39423] [4.7/4.8/4.9 Regression] [SH] performance regression: lost mov @(disp,Rn)

[jakub at gcc dot gnu.org]

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

Jakub Jelinek <jakub at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|4.6.4                       |4.7.4

--- Comment #35 from Jakub Jelinek <jakub at gcc dot gnu.org> 2013-04-12 15:16:33 UTC ---
GCC 4.6.4 has been released and the branch has been closed.

[Bug target/39423] [4.7/4.8/4.9 Regression] [SH] performance regression: lost mov @(disp,Rn)

[olegendo at gcc dot gnu.org]

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

--- Comment #36 from Oleg Endo <olegendo at gcc dot gnu.org> ---
This is annoying:

int
foo (int tab[], int index)
{
  return tab[index+1] + tab[index+2];
}

-O2 -m4 -mb:

        add     #1,r5
        mov     r4,r1
        shll2   r5
        add     r5,r1
        mov     r5,r0
        mov.l   @(4,r1),r1
        mov.l   @(r0,r4),r2
        add     r1,r2
        rts
        mov     r2,r0

should be:

        shll2   r5
        add     r4,r5
        mov.l   @(4,r5),r0
        mov.l   @(8,r5),r1
        rts
        add     r1,r0

Somehow this is a typical problem for combine based 'fixes' -- it works for a
single case, but falls apart when there are multiple uses of the same pattern.

[Bug target/39423] [4.7/4.8/4.9/4.10 Regression] [SH] performance regression: lost mov @(disp,Rn)

[rguenth at gcc dot gnu.org]

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

Richard Biener <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|4.7.4                       |4.8.4

--- Comment #37 from Richard Biener <rguenth at gcc dot gnu.org> ---
The 4.7 branch is being closed, moving target milestone to 4.8.4.

[Bug target/39423] [4.8/4.9/5 Regression] [SH] performance regression: lost mov @(disp,Rn)

[jakub at gcc dot gnu.org]

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

Jakub Jelinek <jakub at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|4.8.4                       |4.8.5

--- Comment #38 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
GCC 4.8.4 has been released.

[Bug target/39423] [4.8/4.9/5 Regression] [SH] performance regression: lost mov @(disp,Rn)

[olegendo at gcc dot gnu.org]

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

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

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

--- Comment #39 from Oleg Endo <olegendo at gcc dot gnu.org> ---
I'd like to close this PR since the immediate problem at hand has been fixed
with a work-around.