[Bug target/70672] New: [5] Wrong code for little endian bitfield modification

[wschmidt at gcc dot gnu.org]

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

            Bug ID: 70672
           Summary: [5] Wrong code for little endian bitfield modification
           Product: gcc
           Version: 5.3.1
            Status: UNCONFIRMED
          Keywords: wrong-code
          Severity: normal
          Priority: P3
         Component: target
          Assignee: unassigned at gcc dot gnu.org
          Reporter: wschmidt at gcc dot gnu.org
                CC: dje at gcc dot gnu.org, segher at gcc dot gnu.org
  Target Milestone: ---
            Target: powerpc64le-unknown-linux-gnu

The following code compiled with -O2 with GCC 5.3 produces incorrect code. 
Correct code is generated for current trunk.

typedef unsigned long long CoreUInt64;
typedef unsigned int CoreUInt32;

CoreUInt64 shift4BtUp(CoreUInt64 a)
{
    struct Int32S
    {
      CoreUInt32 highTop;
      CoreUInt32 lowTop;
    };

      union
      {
        struct Int32S     _32store;
        CoreUInt64 _64store;
      } x;

      x._64store = a;

      x._32store.lowTop  = x._32store.highTop;
      x._32store.highTop = 0;

      return x._64store;
}

The correct code is a 32-bit left shift on little endian, and indeed trunk
produces sldi 3,3,32 as expected.  But on 5.3 we get rldicr 3,3,0,31.

After expand:

;; x._64store = a_2(D);

(insn 6 5 0 (set (reg/v:DI 125 [ x ])
        (reg/v:DI 127 [ a ])) sap.c:19 -1
     (nil))

;; x._32store.lowTop = _4;

(insn 7 6 0 (set (zero_extract:DI (reg/v:DI 125 [ x ])
            (const_int 32 [0x20])
            (const_int 0 [0]))
        (reg/v:DI 125 [ x ])) sap.c:21 -1
     (nil))

;; x._32store.highTop = 0;

(insn 8 7 9 (set (reg:DI 128)
        (const_int 0 [0])) sap.c:22 -1
     (nil))

(insn 9 8 0 (set (zero_extract:DI (reg/v:DI 125 [ x ])
            (const_int 32 [0x20])
            (const_int 32 [0x20]))
        (reg:DI 128)) sap.c:22 -1
     (nil))

;; _7 = x._64store;

(insn 10 9 0 (set (reg:DI 124 [ D.2241 ])
        (reg/v:DI 125 [ x ])) sap.c:24 -1
     (nil))

This is correct.  But then, following the first jump optimization pass, insn 7
is incorrectly removed.

I bisected the fix on trunk to r226005, in which Segher did a complete rewrite
of much of the rotate code.  Unfortunately this is a relatively large patch, so
it isn't immediately obvious where the original problem was.  There must have
been a hidden endianness assumption in there somewhere.

If it's acceptable, it would be nice to backport the whole patch, but I am not
sure whether there were follow-on patches to deal with any fallout.  Certainly
this has had a lot of burn-in time by now.

Segher, can you please advise?  Do you recall seeing a bug like this when you
were working on this last year?