[PATCH] D50432: [DAGCombiner] Reduce load widths of shifted masks

[Francois Pichet via Phabricator via llvm-commits]

fpichet reopened this revision.
fpichet added a comment.
This revision is now accepted and ready to land.

I recently resynced an out of tree backend and I got a miscompile because of this commit.
My target is Big Endian.
The C code is :

  void swap(unsigned *ptr) {
    *ptr = (*ptr & 0x0000ff00 ) << 8;		
  }

The IR is

  %0 = load i32, i32* %ptr, align 4
  %and = and i32 %0, 65280
  %shl = shl i32 %and, 8
  store i32 %shl, i32* %ptr, align 4

The resulting DAG->dump() after the function ReduceLoadWidth returned will be:

  t5: i32,ch = load<(load 4 from %ir.ptr)> t0, t2, undef:i32, main.c:8:4
  t6: i32 = Constant<65280>
    t12: i32 = add nuw t2, Constant:i32<2>, main.c:8:4
  t13: i32,ch = load<(load 1 from %ir.ptr + 2, align 2), zext from i8> t0, t12, undef:i32, main.c:8:4
      t7: i32 = and t13, Constant:i32<255>, main.c:8:4
    t9: i32 = shl t7, Constant:i32<8>, main.c:8:4
  t10: ch = store<(store 4 into %ir.ptr)> t13:1, t9, t2, undef:i32, main.c:8:4

There is a missing shl by 8 missing. (yes there should be 2 shl to be combined later)



================
Comment at: llvm/trunk/lib/CodeGen/SelectionDAG/DAGCombiner.cpp:9252
+    SDValue ShiftC = DAG.getConstant(ShAmt, DL, VT);
+    SDValue Shifted = DAG.getNode(ISD::SHL, DL, VT, SDValue(N, 0),
+                                  ShiftC);
----------------
I think this line should be: 
SDValue Shifted = DAG.getNode(ISD::SHL, DL, VT, N->getOperand(0),


Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D50432/new/

https://reviews.llvm.org/D50432