[Bug regression/28170] New: Wrong result after swap byte in one word when compiled in 64-bit mode

[Bug regression/28170] New: Wrong result after swap byte in one word when compiled in 64-bit mode

[bergner at vnet dot ibm dot com]

A simple byte swap routine gets the wrong result when compiled with -m64 and
either -O1, -O2, -O3 or -Os.

bergner@vervain:~> /opt/biarch/gcc41-base/bin/gcc -O1 -m64 badswap.c
bergner@vervain:~> ./a.out
Before:   0x11  0x22  0x33  0x44  0x55  0x66  0x77  0x88
Result:   0x88  0x77  0x66  0x55  0x44  0x77  0x22  0x11
Expected: 0x88  0x77  0x66  0x55  0x44  0x33  0x22  0x11

Notice the third to last byte in "Result" is 0x77 when it should be 0x33.  This
works with -O0 and with gcc 3.4 or earlier versions.


-- 
           Summary: Wrong result after swap byte in one word when compiled
                    in 64-bit mode
           Product: gcc
           Version: 4.2.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: regression
        AssignedTo: unassigned at gcc dot gnu dot org
        ReportedBy: bergner at vnet dot ibm dot com
 GCC build triplet: powerpc64-linux
  GCC host triplet: powerpc64-linux
GCC target triplet: powerpc64-linux


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

[Bug regression/28170] Wrong result after swap byte in one word when compiled in 64-bit mode

[bergner at vnet dot ibm dot com]

------- Comment #1 from bergner at vnet dot ibm dot com  2006-06-26 18:42 -------
Created an attachment (id=11758)
 --> (http://gcc.gnu.org/bugzilla/attachment.cgi?id=11758&action=view)
Simple test case to show the problem.

The bad code generation is in change_byte_order().


-- 


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

[Bug regression/28170] Wrong result after swap byte in one word when compiled in 64-bit mode

[bergner at vnet dot ibm dot com]

------- Comment #2 from bergner at vnet dot ibm dot com  2006-06-26 18:44 -------
Forgot to mention this fails on both the 4.1 branch and mainline.  It does work
with 3.4 and earlier versions.


-- 


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

[Bug regression/28170] Wrong result after swap byte in one word when compiled in 64-bit mode

[bergner at vnet dot ibm dot com]

------- Comment #3 from bergner at vnet dot ibm dot com  2006-06-26 18:49 -------
Janis performed a regression hunt on mainline and identified this patch as the
start of the test failure:

r83568 | dje | 2004-06-23 21:19:00 +0000 (Wed, 23 Jun 2004) | 7 lines

        * config/rs6000/rs6000.c (rs6000_use_dfa_pipeline_interface): Delete.
        (TARGET_SCHED_USE_DFA_PIPELINE_INTERFACE): Redefine a hook_int_void_1.
        (insvdi_rshift_rlwimi_p): New function.
        * config/rs6000/rs6000.md (insvdi_internal2/3): New patterns.
        (extendsfdf2): Convert to define_insn_and_split.
        * config/rs6000/rs6000-protos.h (insvdi_rshift_rlwimi_p): Prototype.

    http://gcc.gnu.org/viewcvs?view=rev&rev=83568


-- 

bergner at vnet dot ibm dot com changed:

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


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

[Bug rtl-optimization/28170] Wrong result after swap byte in one word when compiled in 64-bit mode

[dje at gcc dot gnu dot org]

------- Comment #4 from dje at gcc dot gnu dot org  2006-06-26 18:51 -------
This appears to be a problem in the insvdi_internal2 and insvdi_internal3
combiner patterns.


-- 

dje at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |NEW
     Ever Confirmed|0                           |1
   Last reconfirmed|0000-00-00 00:00:00         |2006-06-26 18:51:38
               date|                            |


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

[Bug target/28170] [4.1/4.2 Regression] Wrong result after swap byte in one word when compiled in 64-bit mode

[pinskia at gcc dot gnu dot org]

-- 

pinskia at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |pinskia at gcc dot gnu dot
                   |                            |org
          Component|rtl-optimization            |target
            Summary|Wrong result after swap byte|[4.1/4.2 Regression] Wrong
                   |in one word when compiled in|result after swap byte in
                   |64-bit mode                 |one word when compiled in
                   |                            |64-bit mode
   Target Milestone|---                         |4.1.2


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

[Bug target/28170] [4.1/4.2 Regression] Wrong result after swap byte in one word when compiled in 64-bit mode

[pinskia at gcc dot gnu dot org]

------- Comment #5 from pinskia at gcc dot gnu dot org  2006-06-26 20:10 -------
As an aside, I think we should produce two lwbrx instead and then combine them
for the double word (or ldbrx which only exists on Cell as far as I know) But
that is not here or there for this problem right now.


-- 


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

[Bug target/28170] [4.1/4.2 Regression] Wrong result after swap byte in one word when compiled in 64-bit mode

[dje at gcc dot gnu dot org]

------- Comment #6 from dje at gcc dot gnu dot org  2006-06-27 01:34 -------
rs6000.c:insvdi_rshift_rlwimi_p() is wrong, specifically the lines checking the
shift count.


-- 


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

[Bug target/28170] [4.1/4.2 Regression] Wrong result after swap byte in one word when compiled in 64-bit mode

[dje at gcc dot gnu dot org]

------- Comment #7 from dje at gcc dot gnu dot org  2006-06-27 01:40 -------
I think the correct test is something like

Index: rs6000.c
===================================================================
--- rs6000.c    (revision 115003)
+++ rs6000.c    (working copy)
@@ -9794,8 +9794,8 @@
       && INTVAL (startop) > 32
       && (INTVAL (sizeop) + INTVAL (startop) < 64)
       && (INTVAL (sizeop) + INTVAL (startop) > 33)
-      && (INTVAL (sizeop) + INTVAL (startop) + INTVAL (shiftop) < 96)
-      && (INTVAL (sizeop) + INTVAL (startop) + INTVAL (shiftop) >= 64)
+      && (INTVAL (sizeop) + INTVAL (startop) + INTVAL (shiftop) < 64)
+      && (INTVAL (sizeop) + INTVAL (startop) + INTVAL (shiftop) >=32)
       && (64 - (INTVAL (shiftop) & 63)) >= INTVAL (sizeop))
     return 1;


-- 


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

[Bug target/28170] [4.1/4.2 Regression] Wrong result after swap byte in one word when compiled in 64-bit mode

[amodra at bigpond dot net dot au]

------- Comment #8 from amodra at bigpond dot net dot au  2006-06-27 01:41 -------
I think insvdi_rshift_rlwimi_p is totally broken.  It should look something
like the following to match the capabilities of rlwimi according to my reading
of the powerpc architecture specification.

/* Return 1 if operands will generate valid arguments to the rlwimi
   instruction for insert with right shift in 64-bit mode.  */

int
insvdi_rshift_rlwimi_p (rtx sizeop, rtx startop, rtx shiftop)
{
  /* The field that we insert into must start in the low word.  */
  if (INTVAL (startop) < 32 || INTVAL (startop) >= 64)
    return 0;

  /* And it must end there too.  */
  if (INTVAL (sizeop) < 0 || INTVAL (sizeop) + INTVAL (startop) >= 64)
    return 0;

  /* The value we are inserting must also come from the low word.
     Since the sign bit isn't in the low word, we don't need to worry
     about ashiftrt vs. lshiftrt.  */
  if (INTVAL (shiftop) + INTVAL (sizeop) >= 32)
    return 0;

  return 1;
}


-- 


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

[Bug target/28170] [4.1/4.2 Regression] Wrong result after swap byte in one word when compiled in 64-bit mode

[amodra at bigpond dot net dot au]

------- Comment #9 from amodra at bigpond dot net dot au  2006-06-27 01:47 -------
Uh, that should be "INTVAL (sizeop) <= 0".


-- 


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

[Bug target/28170] [4.1/4.2 Regression] Wrong result after swap byte in one word when compiled in 64-bit mode

[bergner at vnet dot ibm dot com]

------- Comment #10 from bergner at vnet dot ibm dot com  2006-07-06 14:32 -------
I used Alan's code changes from comments #8 and #9 on the 4.1 branch and it
bootstrapped and regression tested (32-bit and 64-bit) fine.

Mainline bootstraps and regression tests are still running.


-- 


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

[Bug target/28170] [4.1/4.2 Regression] Wrong result after swap byte in one word when compiled in 64-bit mode

[dje at gcc dot gnu dot org]

------- Comment #11 from dje at gcc dot gnu dot org  2006-07-06 14:37 -------
The original code could not be "totally broken" unless Alan's code also is
totally broken because he computes many of the same tests.  Also, Alan does not
include an important test from the original code, so his code is wrong.

I am testing a variant of the original code that includes some of Alan's
simplifications.


-- 


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

[Bug target/28170] [4.1/4.2 Regression] Wrong result after swap byte in one word when compiled in 64-bit mode

[bergner at vnet dot ibm dot com]

------- Comment #12 from bergner at vnet dot ibm dot com  2006-07-06 18:00 -------
Mainline with Alan's changed bootstrapped, but showed a few regressions.

David's suggested fix from comment #7 (using mainline) bootstrapped and is
currently in the middle of running the test suite.


-- 


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

[Bug target/28170] [4.1/4.2 Regression] Wrong result after swap byte in one word when compiled in 64-bit mode

[dje at gcc dot gnu dot org]

------- Comment #13 from dje at gcc dot gnu dot org  2006-07-06 19:00 -------
Subject: Bug 28170

Author: dje
Date: Thu Jul  6 18:59:58 2006
New Revision: 115229

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=115229
Log:
2006-07-06  David Edelsohn  <edelsohn@gnu.org>

        PR target/28150
        * config/rs6000/rs6000.c (rs6000_legitimate_address): Do not allow
        PRE_{INC,DEC} of TFmode.

2006-07-06  David Edelsohn  <edelsohn@gnu.org>
            Alan Modra  <amodra@bigpond.net.au>

        PR target/28170
        * config/rs6000/rs6000.c (insvdi_rshift_rlwimi_p): Correct shiftop
        bounds. Simplify.

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


-- 


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

[Bug target/28170] [4.1/4.2 Regression] Wrong result after swap byte in one word when compiled in 64-bit mode

[steven at gcc dot gnu dot org]

------- Comment #14 from steven at gcc dot gnu dot org  2006-07-09 00:34 -------
Fixed on the trunk?  If so, please remove the "4.2" marker from this bug.


-- 


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

[Bug target/28170] [4.1 Regression] Wrong result after swap byte in one word when compiled in 64-bit mode

[pinskia at gcc dot gnu dot org]

------- Comment #15 from pinskia at gcc dot gnu dot org  2006-07-10 07:21 -------
Fixed on the mainline.


-- 

pinskia at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
      Known to fail|                            |4.1.0
      Known to work|                            |4.0.2 4.2.0
            Summary|[4.1/4.2 Regression] Wrong  |[4.1 Regression] Wrong
                   |result after swap byte in   |result after swap byte in
                   |one word when compiled in   |one word when compiled in
                   |64-bit mode                 |64-bit mode


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

[Bug target/28170] [4.1 Regression] Wrong result after swap byte in one word when compiled in 64-bit mode

[dje at gcc dot gnu dot org]

------- Comment #16 from dje at gcc dot gnu dot org  2006-07-26 20:22 -------
Subject: Bug 28170

Author: dje
Date: Wed Jul 26 20:21:49 2006
New Revision: 115764

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=115764
Log:
        Backport from mainline
        2006-07-14  Eliot Dresselhaus  <eliot@sonic.net>

        PR target/27287
        * config/rs6000/spe.md (frob_di_df_2): Add m->r alternative.

        2006-07-06  David Edelsohn  <edelsohn@gnu.org>

        PR target/28150
        * config/rs6000/rs6000.c (rs6000_legitimate_address): Do not allow
        PRE_{INC,DEC} of TFmode.

        2006-07-06  David Edelsohn  <edelsohn@gnu.org>
                    Alan Modra  <amodra@bigpond.net.au>

        PR target/28170
        * config/rs6000/rs6000.c (insvdi_rshift_rlwimi_p): Correct shiftop
        bounds. Simplify.

Modified:
    branches/gcc-4_1-branch/gcc/ChangeLog
    branches/gcc-4_1-branch/gcc/config/rs6000/rs6000.c
    branches/gcc-4_1-branch/gcc/config/rs6000/spe.md


-- 


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

[Bug target/28170] [4.1 Regression] Wrong result after swap byte in one word when compiled in 64-bit mode

[dje at gcc dot gnu dot org]

------- Comment #17 from dje at gcc dot gnu dot org  2006-07-26 20:27 -------
patch backported


-- 

dje at gcc dot gnu dot org changed:

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


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