[Bug target/35258] New: two memcpy calls merged incorrectly with -O1

[Bug target/35258] New: two memcpy calls merged incorrectly with -O1

[janis at gcc dot gnu dot org]

Someone at IBM working on a very large project on s390 showed me some odd code
in which two memcpy calls get merged into something that instead spreads one
character into the final destination.  A modification of that code that looks
more normal is:

char string2[9] = "1234";
void
foo (void)
{
  char temp[4];
  char *p = &string2[2];
  memcpy (&temp, &string2[1], 4);
  memcpy (p, &temp, 4);
  string2[1] = '.';
}

The result in string2 should be "1.234" but is "1.2222" (or so says the person
who reported this to me).  I'll attach the full test case with both versions of
the code.

Based on my examination of the generated s390 code, which I had never seen
before and just barely understand, this was broken in 3.0 through 3.4, OK in
4.0 through 4.2, and is broken again in 4.3.


-- 
           Summary: two memcpy calls merged incorrectly with -O1
           Product: gcc
           Version: 4.3.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: target
        AssignedTo: unassigned at gcc dot gnu dot org
        ReportedBy: janis at gcc dot gnu dot org
GCC target triplet: s390-unknown-linux-gnu


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

[Bug target/35258] two memcpy calls merged incorrectly with -O1

[janis at gcc dot gnu dot org]

------- Comment #1 from janis at gcc dot gnu dot org  2008-02-20 02:02 -------
Created an attachment (id=15185)
 --> (http://gcc.gnu.org/bugzilla/attachment.cgi?id=15185&action=view)
test case


-- 


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

[Bug target/35258] two memcpy calls merged incorrectly with -O1

[pinskia at gcc dot gnu dot org]

------- Comment #2 from pinskia at gcc dot gnu dot org  2008-02-20 02:08 -------
Hmm, &temp refers to the pointer to the array.

What happens if you use temp instead of &temp?


-- 


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

[Bug target/35258] two memcpy calls merged incorrectly with -O1

[rguenth at gcc dot gnu dot org]

------- Comment #3 from rguenth at gcc dot gnu dot org  2008-02-20 09:43 -------
This would be an executable testcase:

extern void *memcpy (void *, const void *,  __SIZE_TYPE__);
extern int memcmp(const void *s1, const void *s2, __SIZE_TYPE__ n);
extern void abort(void);

char string1[9] = "1234";
char string2[9] = "1234";

void
bar (void)
{
  unsigned int temp;
  char *p = &string1[2];

  memcpy (&temp, &string1[1], 4);
  memcpy (p, &temp, 4);
  string1[1] = '.';
}

int main()
{
  bar();
  if (memcmp (string1, "1.234", 5) != 0)
    abort ();
  return 0;
}

which I can confirm aborts on s390 but not x86_64.  bar looks like

bar:
.LFB2:
        st      %r15,60(%r15)
.LCFI0:
        larl    %r5,.L3
        ahi     %r15,-104
.LCFI1:
        larl    %r1,string1+2
        l       %r2,.L4-.L3(%r5)
        mvc     0(4,%r1),0(%r2)
        larl    %r1,string1
        mvi     1(%r1),46
        l       %r15,164(%r15)
        br      %r14


-- 

rguenth at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |NEW
     Ever Confirmed|0                           |1
           Keywords|                            |wrong-code
   Last reconfirmed|0000-00-00 00:00:00         |2008-02-20 09:43:54
               date|                            |


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

[Bug target/35258] two memcpy calls merged incorrectly with -O1

[rguenth at gcc dot gnu dot org]

------- Comment #4 from rguenth at gcc dot gnu dot org  2008-02-20 09:52 -------
Actually the asm looks correct.  mvc is a memmove operation, moving 4 bytes
from string1+1 to string1+2.


-- 

rguenth at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |krebbel at gcc dot gnu dot
                   |                            |org


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

[Bug target/35258] two memcpy calls merged incorrectly with -O1

[krebbel at gcc dot gnu dot org]

------- Comment #5 from krebbel at gcc dot gnu dot org  2008-02-20 12:59 -------
The assembler code is broken. In case of an overlap mvc copies one byte at a
time and continuing with the next after the first has been written. That's how
we use mvc for memsets.

The mvcs are merged by the dead store elimination pass. I'll try to understand
were it slips through the tests in dse and/or cse.


-- 


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

[Bug target/35258] two memcpy calls merged incorrectly with -O1

[krebbel at gcc dot gnu dot org]

------- Comment #6 from krebbel at gcc dot gnu dot org  2008-02-25 15:08 -------
Subject: Bug 35258

Author: krebbel
Date: Mon Feb 25 15:07:17 2008
New Revision: 132628

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=132628
Log:
2008-02-25  Andreas Krebbel  <krebbel1@de.ibm.com>

        PR target/35258
        * cse.c (cse_insn): Avoid creation of overlapping MEMs.
        * alias.c (nonoverlapping_memrefs_p): Export for use in other modules.
        * alias.h (nonoverlapping_memrefs_p): Likewise.

2008-02-25  Andreas Krebbel  <krebbel1@de.ibm.com>

        PR target/35258
        * gcc.dg/pr35258.c: New testcase.


Added:
    trunk/gcc/testsuite/gcc.dg/pr35258.c
Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/alias.c
    trunk/gcc/alias.h
    trunk/gcc/cse.c
    trunk/gcc/testsuite/ChangeLog


-- 


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

[Bug target/35258] two memcpy calls merged incorrectly with -O1

[krebbel at gcc dot gnu.org]

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

Andreas Krebbel <krebbel at gcc dot gnu.org> changed:

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

--- Comment #7 from Andreas Krebbel <krebbel at gcc dot gnu.org> 2010-12-02 09:41:57 UTC ---
Fixed.