[DAGCombine] Match more patterns for half word bswap

[deadalnix]

Summary

It ensures that the bswap is generated even when a part of the subtree already matches a bswap transform.

[RKSimon]

vector tests?

lib/CodeGen/SelectionDAG/DAGCombiner.cpp

5773 ↗

(On Diff #222511)

Vector support by using isConstOrConstSplat ?

5774 ↗

(On Diff #222511)

!C || C->getAPIntValue() != 16 (helps avoid fuzz test failures)

[deadalnix]

Use isConstOrConstSplat and getAPIntValue

[RKSimon]

LGTM

[spatel]

I reverted this patch because it caused many bots to fail on AArch64/ARM/AMDGPU codegen tests like:
http://lab.llvm.org:8011/builders/clang-x86_64-debian-fast/builds/18791/steps/test/logs/stdio

[deadalnix]

Fix erroneously inverted condition and ensuring previously broken tests are now passing again.

[spatel]

LGTM - I applied the patch locally and ran 'make check' and don't see failures now.

llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp

static bool isBSwapHWordElement(SDValue N, MutableArrayRef<SDNode *> Parts) {
 
   if (Parts[MaskByteOffset])
     return false;
 
   Parts[MaskByteOffset] = N0.getOperand(0).getNode();
   return true;
 }
 
+// Match 2 elements of a packed halfword bswap.
+static bool isBSwapHWordPair(SDValue N, MutableArrayRef<SDNode *> Parts) {
+  if (N.getOpcode() == ISD::OR)
+    return isBSwapHWordElement(N.getOperand(0), Parts) &&
+           isBSwapHWordElement(N.getOperand(1), Parts);
+
+  if (N.getOpcode() == ISD::SRL && N.getOperand(0).getOpcode() == ISD::BSWAP) {
+    ConstantSDNode *C = isConstOrConstSplat(N.getOperand(1));
+    if (!C || C->getAPIntValue() != 16)
+      return false;
+    Parts[0] = Parts[1] = N.getOperand(0).getOperand(0).getNode();
+    return true;
+  }
+
+  return false;
+}
+
 /// Match a 32-bit packed halfword bswap. That is
 /// ((x & 0x000000ff) << 8) |
 /// ((x & 0x0000ff00) >> 8) |
 /// ((x & 0x00ff0000) << 8) |
 /// ((x & 0xff000000) >> 8)
 /// => (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.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);
+  // (or (bswaphpair), (bswaphpair))
+  // (or (or (bswaphpair), (and)), (and))
+  // (or (or (and), (bswaphpair)), (and))
   SDNode *Parts[4] = {};
 
-  if (N1.getOpcode() == ISD::OR &&
-      N00.getNumOperands() == 2 && N01.getNumOperands() == 2) {
+  if (isBSwapHWordPair(N0, Parts)) {
     // (or (or (and), (and)), (or (and), (and)))
-    if (!isBSwapHWordElement(N00, Parts))
-      return SDValue();
-
-    if (!isBSwapHWordElement(N01, Parts))
-      return SDValue();
-    SDValue N10 = N1.getOperand(0);
-    if (!isBSwapHWordElement(N10, Parts))
+    if (!isBSwapHWordPair(N1, Parts))
       return SDValue();
-    SDValue N11 = N1.getOperand(1);
-    if (!isBSwapHWordElement(N11, Parts))
-      return SDValue();
-  } else {
+  } else if (N0.getOpcode() == ISD::OR) {
     // (or (or (or (and), (and)), (and)), (and))
     if (!isBSwapHWordElement(N1, Parts))
       return SDValue();
-    if (!isBSwapHWordElement(N01, Parts))
-      return SDValue();
-    if (N00.getOpcode() != ISD::OR)
-      return SDValue();
-    SDValue N000 = N00.getOperand(0);
-    if (!isBSwapHWordElement(N000, Parts))
+    SDValue N00 = N0.getOperand(0);
+    SDValue N01 = N0.getOperand(1);
+    if (!(isBSwapHWordElement(N01, Parts) && isBSwapHWordPair(N00, Parts)) &&
+        !(isBSwapHWordElement(N00, Parts) && isBSwapHWordPair(N01, Parts)))
       return SDValue();
-    SDValue N001 = N00.getOperand(1);
-    if (!isBSwapHWordElement(N001, Parts))
-      return SDValue();
-  }
+  } else
 
   // Make sure the parts are all coming from the same node.
   if (Parts[0] != Parts[1] || Parts[0] != Parts[2] || Parts[0] != Parts[3])
     return SDValue();
 
   SDLoc DL(N);
   SDValue BSwap = DAG.getNode(ISD::BSWAP, DL, VT,
                               SDValue(Parts[0], 0));

llvm/test/CodeGen/X86/bswap_tree.ll

 declare i32 @llvm.bswap.i32(i32)
 
 ; Match a 32-bit packed halfword bswap, with some subtree
 ; already converted to a bswap.
 define i32 @test3(i32 %x) nounwind {
 ; CHECK-LABEL: test3:
 ; CHECK:       # %bb.0:
 ; CHECK-NEXT:    movl {{[0-9]+}}(%esp), %eax
-; CHECK-NEXT:    movl %eax, %ecx
-; CHECK-NEXT:    andl $16711680, %ecx # imm = 0xFF0000
-; CHECK-NEXT:    movl %eax, %edx
-; CHECK-NEXT:    andl $-16777216, %edx # imm = 0xFF000000
-; CHECK-NEXT:    shll $8, %ecx
-; CHECK-NEXT:    shrl $8, %edx
-; CHECK-NEXT:    orl %ecx, %edx
 ; CHECK-NEXT:    bswapl %eax
-; CHECK-NEXT:    shrl $16, %eax
-; CHECK-NEXT:    orl %edx, %eax
+; CHECK-NEXT:    roll $16, %eax
 ; CHECK-NEXT:    retl
 ;
 ; CHECK64-LABEL: test3:
 ; CHECK64:       # %bb.0:
 ; CHECK64-NEXT:    movl %edi, %eax
-; CHECK64-NEXT:    andl $16711680, %eax # imm = 0xFF0000
-; CHECK64-NEXT:    movl %edi, %ecx
-; CHECK64-NEXT:    andl $-16777216, %ecx # imm = 0xFF000000
-; CHECK64-NEXT:    shll $8, %eax
-; CHECK64-NEXT:    shrl $8, %ecx
-; CHECK64-NEXT:    addl %ecx, %eax
-; CHECK64-NEXT:    bswapl %edi
-; CHECK64-NEXT:    shrl $16, %edi
-; CHECK64-NEXT:    orl %edi, %eax
+; CHECK64-NEXT:    bswapl %eax
+; CHECK64-NEXT:    roll $16, %eax
 ; CHECK64-NEXT:    retq
   %byte2 = and i32 %x, 16711680   ; 0x00ff0000
   %byte3 = and i32 %x, 4278190080 ; 0xff000000
   %1 = shl  i32 %byte2, 8
   %2 = lshr i32 %byte3, 8
   %or = or i32 %1, %2
   %bswap = call i32 @llvm.bswap.i32(i32 %x)
   %3 = lshr i32 %bswap, 16
   %result = or i32 %or, %3
   ret i32 %result
 }