[llvm] r212915 - [DAGCombiner] Fix a crash caused by a missing check for legal type when trying to fold shuffles.

[Andrea Di Biagio]

Author: adibiagio
Date: Sun Jul 13 16:02:14 2014
New Revision: 212915

URL: http://llvm.org/viewvc/llvm-project?rev=212915&view=rev
Log:
[DAGCombiner] Fix a crash caused by a missing check for legal type when trying to fold shuffles.

Verify that DAGCombiner does not crash when trying to fold a pair of shuffles
according to rule (added at r212539):
  (shuffle (shuffle A, Undef, M0), Undef, M1) -> (shuffle A, Undef, M2)

The DAGCombiner avoids folding shuffles if the resulting shuffle dag node
is not legal for the target. That means, the resulting shuffle must have
legal type and legal mask.

Before, the DAGCombiner only called method
'TargetLowering::isShuffleMaskLegal' to check if it was "safe" to fold according
to the above-mentioned rule. However, this caused a crash in the x86 backend
since method 'isShuffleMaskLegal' always expects to be called on a
legal vector type.


Added:
    llvm/trunk/test/CodeGen/X86/shuffle-combine-crash.ll
Modified:
    llvm/trunk/lib/CodeGen/SelectionDAG/DAGCombiner.cpp

Modified: llvm/trunk/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/SelectionDAG/DAGCombiner.cpp?rev=212915&r1=212914&r2=212915&view=diff
==============================================================================
--- llvm/trunk/lib/CodeGen/SelectionDAG/DAGCombiner.cpp (original)
+++ llvm/trunk/lib/CodeGen/SelectionDAG/DAGCombiner.cpp Sun Jul 13 16:02:14 2014
@@ -10764,7 +10764,7 @@ SDValue DAGCombiner::visitVECTOR_SHUFFLE
 
     // It may still be beneficial to combine the two shuffles if the
     // resulting shuffle is legal.
-    if (TLI.isShuffleMaskLegal(Mask, VT)) {
+    if (TLI.isTypeLegal(VT) && TLI.isShuffleMaskLegal(Mask, VT)) {
       if (!CommuteOperands)
         // shuffle(shuffle(x, undef, M1), undef, M2) -> shuffle(x, undef, M3).
         // shuffle(shuffle(x, y, M1), undef, M2) -> shuffle(x, undef, M3)

Added: llvm/trunk/test/CodeGen/X86/shuffle-combine-crash.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/X86/shuffle-combine-crash.ll?rev=212915&view=auto
==============================================================================
--- llvm/trunk/test/CodeGen/X86/shuffle-combine-crash.ll (added)
+++ llvm/trunk/test/CodeGen/X86/shuffle-combine-crash.ll Sun Jul 13 16:02:14 2014
@@ -0,0 +1,30 @@
+; RUN: llc < %s -mtriple=x86_64-unknown-linux-gnu -mcpu=corei7 
+
+; Verify that DAGCombiner does not crash when checking if it is
+; safe to fold the shuffles in function @sample_test according to rule
+;  (shuffle (shuffle A, Undef, M0), Undef, M1) -> (shuffle A, Undef, M2)
+;
+; The DAGCombiner avoids folding shuffles if
+; the resulting shuffle dag node is not legal for the target.
+; That means, the shuffle must have legal type and legal mask.
+;
+; Before, the DAGCombiner forgot to check if the resulting shuffle
+; was legal. It instead just called method
+; 'X86TargetLowering::isShuffleMaskLegal'; however, that was not enough since
+; that method always expect to have a valid vector type in input.
+; As a consequence, compiling the function below would have caused a crash.
+
+define void @sample_test() {
+  br i1 undef, label %5, label %1
+
+; <label>:1                                       ; preds = %0
+  %2 = load <4 x i8>* undef
+  %3 = shufflevector <4 x i8> %2, <4 x i8> undef, <4 x i32> <i32 2, i32 2, i32 0, i32 0>
+  %4 = shufflevector <4 x i8> %3, <4 x i8> undef, <4 x i32> <i32 2, i32 3, i32 0, i32 1>
+  store <4 x i8> %4, <4 x i8>* undef
+  br label %5
+
+; <label>:5                                       ; preds = %1, %0
+  ret void
+}
+