[PATCH] [DAGCombiner] Fix wrong folding of AND dag nodes.

[Andrea Di Biagio]

Hi qcolombet, nadav, mkuper,

This patch fixes the logic in the DAGCombiner that folds an AND node according to rule:
  (and (X (load V)), C) -> (X (load V))

An AND between a vector load 'X' and a constant build_vector 'C' can be folded into the load itself only if we can prove that the AND operation is redundant.
The algorithm implemented by 'visitAND' firstly computes the splat value 'S' from C, and then checks if S has the lower 'B' bits set (where B is the size in bits of the vector element type). The algorithm takes into account also the 'undef' bits in the splat mask.

Unfortunately, the algorithm only works under the assumption that the sizeof S is a multiple of the vector element type. This assumption is not always valid. For example:

  %load = load <3 x i8>, <3 x i8>* %ptr
  %and = <3 x i8> %load, <i8 undef, i8 undef, i8 95>

Here, the splat value returned by method 'BuildVectorSDNode::isConstantSplat' is APInt(12b, 1520u, 1520s).
The undef bits of the splat mask are APInt(12b, 15u, 15s).
The unified splat plus undef mask 'SU' is ApInt(12b, 1535u, 1535s).
The size in bits of the vector element type is 8. However, the splat value has a size of 12.

Before this patch, the algorithm would have wrongly truncated SU from 12 to 8 bits. So, the resulting splat value was incorrectly propagated as constant 255. Since 255 is a mask with all-bits set, the AND was considered redundant and therefore wrongly removed.

With this patch, we conservatively avoid folding the AND if the splat bits are not compatible with the vector element type.

Added X86 test and-load-fold.ll

Please let me know if ok to submit.

http://reviews.llvm.org/D8085

Files:
  lib/CodeGen/SelectionDAG/DAGCombiner.cpp
  test/CodeGen/X86/and-load-fold.ll

Index: lib/CodeGen/SelectionDAG/DAGCombiner.cpp
===================================================================
--- lib/CodeGen/SelectionDAG/DAGCombiner.cpp
+++ lib/CodeGen/SelectionDAG/DAGCombiner.cpp
@@ -2808,9 +2808,13 @@
                SplatBitSize = SplatBitSize * 2)
             SplatValue |= SplatValue.shl(SplatBitSize);
 
-        Constant = APInt::getAllOnesValue(BitWidth);
-        for (unsigned i = 0, n = SplatBitSize/BitWidth; i < n; ++i)
-          Constant &= SplatValue.lshr(i*BitWidth).zextOrTrunc(BitWidth);
+        // Make sure that variable 'Constant' is only set if 'SplatBitSize' is a
+        // multiple of 'BitWidth'. Otherwise, we could propagate a wrong value.
+        if (SplatBitSize % BitWidth == 0) {
+          Constant = APInt::getAllOnesValue(BitWidth);
+          for (unsigned i = 0, n = SplatBitSize/BitWidth; i < n; ++i)
+            Constant &= SplatValue.lshr(i*BitWidth).zextOrTrunc(BitWidth);
+        }
       }
     }
 
Index: test/CodeGen/X86/and-load-fold.ll
===================================================================
--- test/CodeGen/X86/and-load-fold.ll
+++ test/CodeGen/X86/and-load-fold.ll
@@ -0,0 +1,15 @@
+; RUN: llc -mtriple=x86_64-unknown-unknown -mcpu=generic < %s | FileCheck %s
+
+; Verify that the DAGCombiner doesn't wrongly remove the 'and' from the dag.
+
+define i8 @foo(<4 x i8>* %V) {
+; CHECK-LABEL: foo:
+; CHECK: pand
+; CHECK: ret
+entry:
+  %Vp = bitcast <4 x i8>* %V to <3 x i8>*
+  %V3i8 = load <3 x i8>, <3 x i8>* %Vp, align 4
+  %0 = and <3 x i8> %V3i8, <i8 undef, i8 undef, i8 95>
+  %1 = extractelement <3 x i8> %0, i64 2
+  ret i8 %1
+}

EMAIL PREFERENCES
  http://reviews.llvm.org/settings/panel/emailpreferences/
-------------- next part --------------
A non-text attachment was scrubbed...
Name: D8085.21291.patch
Type: text/x-patch
Size: 1632 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150305/6f8f085e/attachment.bin>