Bug 19201 - [m68k] Inefficient code for array accesses (from old PROBLEMS)
Summary: [m68k] Inefficient code for array accesses (from old PROBLEMS)
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: target (show other bugs)
Version: 4.0.0
: P2 enhancement
Target Milestone: ---
Assignee: Not yet assigned to anyone
URL: http://gcc.gnu.org/ml/gcc-patches/200...
Keywords: missed-optimization, patch
Depends on:
Blocks:
 
Reported: 2004-12-30 12:15 UTC by Steven Bosscher
Modified: 2015-12-13 13:07 UTC (History)
5 users (show)

See Also:
Host:
Target: m68k-*-*
Build:
Known to work:
Known to fail:
Last reconfirmed: 2008-09-03 02:05:25


Attachments
Patch (544 bytes, text/plain)
2005-11-20 00:22 UTC, Kazu Hirata
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Steven Bosscher 2004-12-30 12:15:37 UTC
Item 10 from the old PROBLEMS file (no test case available):

----------------------------------------
        movl a3@,a0
        movl a3@(16),a1
        clrb a0@(a1:l)

is generated and may be worse than

        movl a3@,a0
        addl a3@(16),a0
        clrb a0@

If ordering of operands is improved, many more such cases will be
generated from typical array accesses.
----------------------------------------
Comment 1 Andreas Schwab 2004-12-31 13:32:31 UTC
Test case (see also 
<http://gcc.gnu.org/ml/gcc-patches/2004-11/msg01109.html>): 
 
struct X { 
  char *a; 
  /* other members */ 
  int b; 
}; 
 
void f (struct X *x) 
{ 
  x->a[x->b] = 0; 
} 
Comment 2 Andrew Pinski 2004-12-31 14:58:00 UTC
Confirmed.
Comment 3 Kazu Hirata 2005-11-19 21:52:23 UTC
FWIW, the mainline gcc with -O2 -fomit-frame-pointer produces

f:
        move.l 4(%sp),%a0
        move.l (%a0),%a1
        move.l 4(%a0),%a0
        clr.b (%a0,%a1.l)
        rts
Comment 4 Kazu Hirata 2005-11-20 00:22:53 UTC
Created attachment 10299 [details]
Patch

With this patch, I get:

f:
        move.l 4(%sp),%a0
        move.l (%a0),%a1
        add.l 4(%a0),%a1
        clr.b (%a1)
        rts
Comment 5 Andreas Schwab 2005-11-21 23:07:39 UTC
The comment in the patch has a typo: clr.b (%a0) should be clr.b (%a1).
Comment 6 Kazu Hirata 2005-11-22 17:42:50 UTC
Andreas,

Thanks for spotting the typo.  I also updated the patch to ensure that
we are giving an address register indirect to clr.
Comment 7 Steven Bosscher 2007-12-17 08:12:39 UTC
Kazu, plans with this bug, and your patch for it?
Comment 8 Andrew Pinski 2008-09-03 02:05:23 UTC
This should not have been in waiting as it was waiting on a developer response and not the reporter.
Comment 9 Steven Bosscher 2008-09-21 13:21:20 UTC
Andreas, could you adopt the patch of comment #4 and see if it still fixes this bug?
Comment 10 Steven Bosscher 2010-02-12 22:01:46 UTC
Waiting for a m68k maintainer to do something here...
Comment 11 Steven Bosscher 2011-02-25 23:12:26 UTC
No response from m68k maintainers for almost 2.5 years.
This just clutters my bug searches. WONTFIX seems the most logical "way out".
Comment 12 Andreas Schwab 2011-02-25 23:58:09 UTC
Not a reason to close them.
Comment 13 Steven Bosscher 2011-02-26 00:17:54 UTC
Also fine, not closed. Great to see that you pay enough attention to stop the reporter from closing his own PRs. I wish you would be just as fast with actually doing something about them. It is not even clear whether these problems still exist!

If this still not OK with you, I suggest you do something about these 15+ years old problems, or close these and re-file under your own account. They may not bother you but I don't want them anymore in my list.
Comment 14 Mikael Pettersson 2011-02-26 11:35:50 UTC
(In reply to comment #13)
> Also fine, not closed. Great to see that you pay enough attention to stop the
> reporter from closing his own PRs. I wish you would be just as fast with
> actually doing something about them. It is not even clear whether these
> problems still exist!

The missed optimization still occurs with gcc-4.6, which generates:

f:
        move.l 4(%sp),%a0
        move.l (%a0),%a1
        move.l 4(%a0),%d0
        clr.b (%a1,%d0.l)
        rts
        .size   f, .-f
        .ident  "GCC: (GNU) 4.6.0 20110219 (experimental)"

I'll try Kazu's patch in my next 4.4 bootstrap/regtest.
Comment 15 Mikael Pettersson 2011-02-28 12:04:32 UTC
(In reply to comment #14)
> I'll try Kazu's patch in my next 4.4 bootstrap/regtest.

Kazu's patch appears to have been for a 4.2 code base.  I forward-ported it to 4.4.5, where it fixed the test case in a cross compiler, but unfortunately broke native bootstrap:

gengtype-lex.c: In function 'yy_get_next_buffer':
gengtype-lex.c:1663: warning: old-style function definition
gengtype-lex.c: In function 'yy_get_previous_state':
gengtype-lex.c:1795: warning: old-style function definition
gengtype-lex.c: In function 'yylex':
gengtype-lex.c:1652: error: unrecognizable insn:
(insn 2783 2782 1738 243 gengtype-lex.c:1784 (set (mem:QI (reg:SI 1 %d1 [orig:121 yy_n_chars.68 ] [121]) [0 S1 A8])
        (const_int 0 [0x0])) -1 (nil))
gengtype-lex.c:1652: internal compiler error: in extract_insn, at recog.c:2048
Please submit a full bug report,
with preprocessed source if appropriate.
See <http://gcc.gnu.org/bugs.html> for instructions.
make[3]: *** [build/gengtype-lex.o] Error 1
make[3]: Leaving directory `/mnt/scratch/objdir44/gcc'
make[2]: *** [all-stage2-gcc] Error 2
make[2]: Leaving directory `/mnt/scratch/objdir44'
make[1]: *** [stage2-bubble] Error 2
make[1]: Leaving directory `/mnt/scratch/objdir44'
make: *** [bootstrap] Error 2
Comment 16 Jeffrey A. Law 2015-12-13 13:07:17 UTC
Fixed on trunk.