[Bug middle-end/54386] New: Unaligned mem load wrongly generated for inlined inline/static function

[Bug middle-end/54386] New: Unaligned mem load wrongly generated for inlined inline/static function

[olegendo at gcc dot gnu.org]

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

             Bug #: 54386
           Summary: Unaligned mem load wrongly generated for inlined
                    inline/static function
    Classification: Unclassified
           Product: gcc
           Version: 4.8.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: middle-end
        AssignedTo: unassigned@gcc.gnu.org
        ReportedBy: olegendo@gcc.gnu.org
            Target: sh*-*-*


I've tested this only on SH, but it might be a generic case.
The following code snippet... 


static __inline__ int
__testbit (unsigned long nr, unsigned long* a)
{
  return (*a & (1 << nr)) != 0;
}


typedef struct page2
{
 void* x[2];
 unsigned long flags;

 void* a;
 void* b;
} page2_t;



void activate_page3(struct page2 * page)
{
 if ( ! __testbit (7, &(page->flags)) )
  {
    page->a = 0;
    page->b = 0;
 }
}


...compiled with -O2 expands to the following RTL:

void activate_page3(page2*) (struct page2 * page)
{
  long unsigned int _4;
  long unsigned int _7;

;;   basic block 2, loop depth 0
;;    pred:       ENTRY
  _4 = MEM[(long unsigned int *)page_2(D) + 8B];
  _7 = _4 & 128;
  if (_7 == 0)
    goto <bb 3>;
  else
    goto <bb 4>;
;;    succ:       3
;;                4

;;   basic block 3, loop depth 0
;;    pred:       2
  page_2(D)->a = 0B;
  page_2(D)->b = 0B;
;;    succ:       4

;;   basic block 4, loop depth 0
;;    pred:       2
;;                3
  return;
;;    succ:       EXIT

}


;; Generating RTL for gimple basic block 2

;; if (_7 == 0)

(insn 7 5 8 (set (reg:QI 165)
        (mem:QI (plus:SI (reg/v/f:SI 162 [ page ])
                (const_int 8 [0x8])) [0+0 S1 A8])) sh_tmp.cpp:699 -1
     (nil))

(insn 8 7 9 (set (reg:SI 163)
        (zero_extend:SI (reg:QI 165))) sh_tmp.cpp:699 -1
     (nil))

(insn 9 8 11 (set (reg:SI 166)
        (ashift:SI (reg:SI 163)
            (const_int 24 [0x18]))) sh_tmp.cpp:699 -1
     (nil))

(insn 11 9 12 (set (reg:QI 169)
        (mem:QI (plus:SI (reg/v/f:SI 162 [ page ])
                (const_int 9 [0x9])) [0+1 S1 A8])) sh_tmp.cpp:699 -1
     (nil))

[...]


For some reason the SI mem access is converted to four QI mem accesses, which
looks like an unaligned load.  Since only one QI part is needed of the QI loads
disappear after the combine pass has done its thing.


On the other hand, removing 'static' and 'inline' for the function __testbit,
like:

int
__testbit (unsigned long nr, unsigned long* a)

would still do the inlining (as expected) and expand to (aligned) SI mem load:

void activate_page3(page2*) (struct page2 * page)
{
  long unsigned int _6;
  long unsigned int _7;

;;   basic block 2, loop depth 0
;;    pred:       ENTRY
  _6 = MEM[(long unsigned int *)page_2(D) + 8B];
  _7 = _6 & 128;
  if (_7 == 0)
    goto <bb 3>;
  else
    goto <bb 4>;
;;    succ:       3
;;                4

;;   basic block 3, loop depth 0
;;    pred:       2
  page_2(D)->a = 0B;
  page_2(D)->b = 0B;
;;    succ:       4

;;   basic block 4, loop depth 0
;;    pred:       2
;;                3
  return;
;;    succ:       EXIT

}



;; Generating RTL for gimple basic block 2

;; if (_7 == 0)

(insn 6 5 7 (set (reg:SI 164)
        (mem:SI (plus:SI (reg/v/f:SI 162 [ page ])
                (const_int 8 [0x8])) [2 MEM[(long unsigned int *)page_2(D) +
8B]+0 S4 A32])) swap.i:20 -1
     (nil))

(insn 7 6 8 (set (reg:SI 163 [ D.1692 ])
        (and:SI (reg:SI 164)
            (const_int 128 [0x80]))) swap.i:20 -1
     (nil))


The unaligned loads are generated if the __testbit function is 
- 'static'
- 'inline'
- 'static inline'



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

[Bug middle-end/54386] Unaligned mem load wrongly generated for inlined inline/static function

[gjl at gcc dot gnu.org]

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

Georg-Johann Lay <gjl at gcc dot gnu.org> changed:

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

--- Comment #1 from Georg-Johann Lay <gjl at gcc dot gnu.org> 2012-09-06 14:50:41 UTC ---
Who expands the 8-bit loads?  Is there an implicit memcpy, expanded in movmem*?

[Bug middle-end/54386] Unaligned mem load wrongly generated for inlined inline/static function

[olegendo at gcc dot gnu.org]

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

--- Comment #2 from Oleg Endo <olegendo at gcc dot gnu.org> 2012-09-06 16:03:50 UTC ---
(In reply to comment #1)
> Who expands the 8-bit loads?  Is there an implicit memcpy, expanded in movmem*?

I haven't investigated further into the issue.  The logs in the original
description are from the RTL expand pass, i.e. the first RTL pass.

[Bug middle-end/54386] Unaligned mem load wrongly generated for inlined inline/static function

[olegendo at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Target|sh*-*-*                     |sh*-*-* arm*
             Status|UNCONFIRMED                 |NEW
   Last reconfirmed|                            |2012-10-29
     Ever Confirmed|0                           |1

--- Comment #3 from Oleg Endo <olegendo at gcc dot gnu.org> 2012-10-29 20:38:08 UTC ---
I've checked this issue again on rev 192482.  It seems to be target
independent, as it also happens on ARM.
The following:

static inline int readint (int* x)
{
  return *x;
}

int test (int* x)
{
  return readint (x);
}

compiles to (-O2):

test:
        ldrb    r3, [r0]
    ldrb    r1, [r0, #1]
    ldrb    r2, [r0, #2]
    orr    r3, r3, r1, asl #8
    ldrb    r0, [r0, #3]
    orr    r3, r3, r2, asl #16
    orr    r0, r3, r0, asl #24
    bx    lr

[Bug middle-end/54386] Unaligned mem load wrongly generated for inlined inline/static function

[mikpe at it dot uu.se]

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

Mikael Pettersson <mikpe at it dot uu.se> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |mikpe at it dot uu.se

--- Comment #4 from Mikael Pettersson <mikpe at it dot uu.se> 2012-10-29 22:18:16 UTC ---
(In reply to comment #3)
> I've checked this issue again on rev 192482.  It seems to be target
> independent, as it also happens on ARM.
> The following:
> 
> static inline int readint (int* x)
> {
>   return *x;
> }
> 
> int test (int* x)
> {
>   return readint (x);
> }
> 
> compiles to (-O2):
> 
> test:
>         ldrb    r3, [r0]
>     ldrb    r1, [r0, #1]
>     ldrb    r2, [r0, #2]
>     orr    r3, r3, r1, asl #8
>     ldrb    r0, [r0, #3]
>     orr    r3, r3, r2, asl #16
>     orr    r0, r3, r0, asl #24
>     bx    lr

I see this poor code too on armv5tel-linux-gnueabi with 4.8-20121028.
4.7 and 4.6 generate just two instructions for test: a ldr and bx.

[Bug middle-end/54386] [4.8 Regression] Unaligned mem load wrongly generated for inlined inline/static function

[olegendo at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
            Summary|Unaligned mem load wrongly  |[4.8 Regression] Unaligned
                   |generated for inlined       |mem load wrongly generated
                   |inline/static function      |for inlined inline/static
                   |                            |function

--- Comment #5 from Oleg Endo <olegendo at gcc dot gnu.org> 2012-10-29 22:23:25 UTC ---
(In reply to comment #4)
> 
> I see this poor code too on armv5tel-linux-gnueabi with 4.8-20121028.
> 4.7 and 4.6 generate just two instructions for test: a ldr and bx.

Thanks for checking it on earlier versions.

[Bug middle-end/54386] [4.8 Regression] Unaligned mem load wrongly generated for inlined inline/static function

[olegendo at gcc dot gnu.org]

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

--- Comment #6 from Oleg Endo <olegendo at gcc dot gnu.org> 2012-10-30 09:04:37 UTC ---
Just for the record, this seems to happen only for mem loads.  Mem stores
expand as expected.

[Bug middle-end/54386] [4.8 Regression] Unaligned mem load wrongly generated for inlined inline/static function

[mikpe at it dot uu.se]

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

Mikael Pettersson <mikpe at it dot uu.se> changed:

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

--- Comment #7 from Mikael Pettersson <mikpe at it dot uu.se> 2012-10-30 19:28:15 UTC ---
I'm seeing the same code quality regression on sparc64-linux, another
strict-alignment target.

The regression started with r185470:
http://gcc.gnu.org/ml/gcc-cvs/2012-03/msg00800.html
http://gcc.gnu.org/ml/gcc-patches/2012-03/msg00989.html

Patch author CC:d.

[Bug middle-end/54386] [4.8 Regression] Unaligned mem load wrongly generated for inlined inline/static function

[ebotcazou at gcc dot gnu.org]

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

Eric Botcazou <ebotcazou at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Target|sh*-*-* arm*                |sh*-*-* arm*-*-* sparc*-*-*
                 CC|                            |ebotcazou at gcc dot
                   |                            |gnu.org
   Target Milestone|---                         |4.8.0
           Severity|normal                      |major

--- Comment #8 from Eric Botcazou <ebotcazou at gcc dot gnu.org> 2012-10-31 08:24:00 UTC ---
Ugh.  ARM and SPARC are primary targets, so this is a major regression.

[Bug tree-optimization/54386] [4.8 Regression] Unaligned mem load wrongly generated for inlined inline/static function

[jamborm at gcc dot gnu.org]

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

--- Comment #9 from Martin Jambor <jamborm at gcc dot gnu.org> 2012-11-27 20:46:58 UTC ---
I have proposed a patch on the mailing list:

http://gcc.gnu.org/ml/gcc-patches/2012-11/msg02265.html

Any testing, especially on strict-alignment platforms I do not have access to,
would be very welcome.

[Bug tree-optimization/54386] [4.8 Regression] Unaligned mem load wrongly generated for inlined inline/static function

[jamborm at gcc dot gnu.org]

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

--- Comment #10 from Martin Jambor <jamborm at gcc dot gnu.org> 2012-11-30 16:11:43 UTC ---
Author: jamborm
Date: Fri Nov 30 16:11:33 2012
New Revision: 193998

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=193998
Log:
2012-11-30  Martin Jambor  <mjambor@suse.cz>

    PR middle-end/52890
    PR tree-optimization/55415
    PR tree-optimization/54386
    PR target/55448
    * ipa-prop.c (ipa_modify_call_arguments): Be optimistic when
    get_pointer_alignment_1 returns false and the base was not a
    dereference.
    * tree-sra.c (access_precludes_ipa_sra_p): New parameter req_align,
    added check for required alignment.  Update the user.

    * testsuite/gcc.dg/ipa/ipa-sra-7.c: New test.
    * testsuite/gcc.dg/ipa/ipa-sra-8.c: Likewise.
    * testsuite/gcc.dg/ipa/ipa-sra-9.c: Likewise.
    * testsuite/gcc.target/i386/pr55448.c: Likewise.


Added:
    trunk/gcc/testsuite/gcc.dg/ipa/ipa-sra-7.c
    trunk/gcc/testsuite/gcc.dg/ipa/ipa-sra-8.c
    trunk/gcc/testsuite/gcc.dg/ipa/ipa-sra-9.c
    trunk/gcc/testsuite/gcc.target/i386/pr55448.c
Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/ipa-prop.c
    trunk/gcc/testsuite/ChangeLog
    trunk/gcc/tree-sra.c

[Bug tree-optimization/54386] [4.8 Regression] Unaligned mem load wrongly generated for inlined inline/static function

[jamborm at gcc dot gnu.org]

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

Martin Jambor <jamborm at gcc dot gnu.org> changed:

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

--- Comment #11 from Martin Jambor <jamborm at gcc dot gnu.org> 2012-12-03 13:27:50 UTC ---
*** Bug 55415 has been marked as a duplicate of this bug. ***

[Bug tree-optimization/54386] [4.8 Regression] Unaligned mem load wrongly generated for inlined inline/static function

[jamborm at gcc dot gnu.org]

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

Martin Jambor <jamborm at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |kretz at kde dot org

--- Comment #12 from Martin Jambor <jamborm at gcc dot gnu.org> 2012-12-03 13:28:48 UTC ---
*** Bug 55448 has been marked as a duplicate of this bug. ***

[Bug tree-optimization/54386] [4.8 Regression] Unaligned mem load wrongly generated for inlined inline/static function

[jamborm at gcc dot gnu.org]

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

Martin Jambor <jamborm at gcc dot gnu.org> changed:

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

--- Comment #13 from Martin Jambor <jamborm at gcc dot gnu.org> 2012-12-03 13:32:36 UTC ---
This is now fixed.  Thanks for patience and all the testcases.

[Bug tree-optimization/54386] [4.8 Regression] Unaligned mem load wrongly generated for inlined inline/static function

[olegendo at gcc dot gnu.org]

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

--- Comment #14 from Oleg Endo <olegendo at gcc dot gnu.org> 2012-12-04 09:53:05 UTC ---
(In reply to comment #13)
> This is now fixed.  Thanks for patience and all the testcases.

Great, thanks!
I'll add an SH testcase for this, too.

[Bug tree-optimization/54386] [4.8 Regression] Unaligned mem load wrongly generated for inlined inline/static function

[olegendo at gcc dot gnu.org]

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

--- Comment #15 from Oleg Endo <olegendo at gcc dot gnu.org> 2013-02-04 22:41:47 UTC ---
Author: olegendo
Date: Mon Feb  4 22:41:44 2013
New Revision: 195742

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=195742
Log:
    PR tree-optimization/54386
    * gcc.target/sh/pr54386.c: New.


Added:
    trunk/gcc/testsuite/gcc.target/sh/pr54386.c
Modified:
    trunk/gcc/testsuite/ChangeLog