DAGCombiner::MatchBSwapHWord fix for (t0 | t1) | (t2 | t3)

[andrew.zhogin]

Summary

DAGCombiner::MatchBSwapHWord works for ((t0 | t1) | t2) | t3.
But for symmetric case it's broken: (t0 | t1) | (t2 | t3).
This patch fixes it and adds CodeGen test cases for ARM rev16 instruction.

[andrew.zhogin]

This problem was fixed.

lib/CodeGen/SelectionDAG/DAGCombiner.cpp

 static bool isBSwapHWordElement(SDValue N, MutableArrayRef<SDNode *> Parts) {
   if (!N.getNode()->hasOneUse())
     return false;
 
   unsigned Opc = N.getOpcode();
   if (Opc != ISD::AND && Opc != ISD::SHL && Opc != ISD::SRL)
     return false;
 
-  ConstantSDNode *N1C = dyn_cast<ConstantSDNode>(N.getOperand(1));
+  ConstantSDNode *N1C = (Opc == ISD::AND) ?
+    dyn_cast<ConstantSDNode>(N.getOperand(1)) :
+    dyn_cast<ConstantSDNode>(N.getOperand(0).getOperand(1));
   if (!N1C)
     return false;
 
   unsigned Num;
   switch (N1C->getZExtValue()) {
   default:
     return false;
   case 0xFF:       Num = 0; break;

 /// => (rotl (bswap x), 16)
 SDValue DAGCombiner::MatchBSwapHWord(SDNode *N, SDValue N0, SDValue N1) {
   if (!LegalOperations)
     return SDValue();
 
   EVT VT = N->getValueType(0);
   if (VT != MVT::i32)
     return SDValue();
-  if (!TLI.isOperationLegal(ISD::BSWAP, VT))
+  if (!TLI.isOperationLegalOrCustom(ISD::BSWAP, VT))
     return SDValue();
 
   // Look for either
   // (or (or (and), (and)), (or (and), (and)))
   // (or (or (or (and), (and)), (and)), (and))
   if (N0.getOpcode() != ISD::OR)
     return SDValue();
   SDValue N00 = N0.getOperand(0);
   SDValue N01 = N0.getOperand(1);
   SDNode *Parts[4] = {};
 
   if (N1.getOpcode() == ISD::OR &&
       N00.getNumOperands() == 2 && N01.getNumOperands() == 2) {
     // (or (or (and), (and)), (or (and), (and)))
-    SDValue N000 = N00.getOperand(0);
-    if (!isBSwapHWordElement(N000, Parts))
+    if (!isBSwapHWordElement(N00, Parts))
       return SDValue();
-
-    SDValue N001 = N00.getOperand(1);
-    if (!isBSwapHWordElement(N001, Parts))
+    if (!isBSwapHWordElement(N01, Parts))
       return SDValue();
-    SDValue N010 = N01.getOperand(0);
-    if (!isBSwapHWordElement(N010, Parts))
+    SDValue N10 = N1.getOperand(0);
+    if (!isBSwapHWordElement(N10, Parts))
       return SDValue();
-    SDValue N011 = N01.getOperand(1);
-    if (!isBSwapHWordElement(N011, Parts))
+    SDValue N11 = N1.getOperand(1);
+    if (!isBSwapHWordElement(N11, Parts))
       return SDValue();
   } else {
     // (or (or (or (and), (and)), (and)), (and))
     if (!isBSwapHWordElement(N1, Parts))
       return SDValue();
     if (!isBSwapHWordElement(N01, Parts))
       return SDValue();
     if (N00.getOpcode() != ISD::OR)

test/CodeGen/ARM/rev.ll

 ; CHECK: rev16 r0, r0
   %conv = zext i16 %v to i32
   %shr4 = lshr i32 %conv, 8
   %shl = shl nuw nsw i32 %conv, 8
   %or = or i32 %shr4, %shl
   %conv3 = trunc i32 %or to i16
   ret i16 %conv3
 }
+
+; like test1, just (t0 | t1) | (t2 | t3) instead of ((t0 | t1) | t2) | t3
+define i32 @test10(i32 %x) nounwind readnone {
+entry:
+; CHECK-LABEL: test10:
+; CHECK: rev16  r0, r0
+  %byte0 = and i32 %x, 255        ; 0x000000ff
+  %byte1 = and i32 %x, 65280      ; 0x0000ff00
+  %byte2 = and i32 %x, 16711680   ; 0x00ff0000
+  %byte3 = and i32 %x, 4278190080 ; 0xff000000
+
+  %tmp0 = shl  i32 %byte0, 8
+  %tmp1 = lshr i32 %byte1, 8
+  %tmp2 = shl  i32 %byte2, 8
+  %tmp3 = lshr i32 %byte3, 8
+
+  %or0 = or i32 %tmp0, %tmp1
+  %or1 = or i32 %tmp2, %tmp3
+
+  %result = or i32 %or0, %or1
+  ret i32 %result
+}
+
+; the same as test10, but shifts before the "and"
+; ((x << 8) & 0x0000ff00) |
+; ((x >> 8) & 0x000000ff) |
+; ((x << 8) & 0xff000000) |
+; ((x >> 8) & 0x00ff0000)
+define i32 @test11(i32 %x) nounwind readnone {
+entry:
+; CHECK-LABEL: test11:
+; CHECK: rev16  r0, r0
+  %byte1 = shl  i32 %x, 8
+  %byte0 = lshr i32 %x, 8
+  %byte3 = shl  i32 %x, 8
+  %byte2 = lshr i32 %x, 8
+
+  %tmp1 = and i32 %byte1, 65280      ; 0x0000ff00
+  %tmp0 = and i32 %byte0, 255        ; 0x000000ff
+  %tmp3 = and i32 %byte3, 4278190080 ; 0xff000000
+  %tmp2 = and i32 %byte2, 16711680   ; 0x00ff0000
+
+  %or0 = or i32 %tmp0, %tmp1
+  %or1 = or i32 %tmp2, %tmp3
+
+  %result = or i32 %or0, %or1
+  ret i32 %result
+}
+