[PATCH] [AArch64]Fix an assertion failure caused by v1i64 in DAGCombiner shrink

[Hao Liu]

Hi t.p.northover,

Hi Tim and other reviewers,

An assertion fails in TargetLoweringOpt::ShrinkDemandedOp() when trying to create a truncate from v1i64 to i32. This patch fixes this problem. As isZExtFree() and isTrancateFree() are not only used in ShrinkDemandedOp(), this patch also modifies isZExtFree() and isTrancateFree() to make the code more robust. 

This case exposes that Type::isIntegerTy() and EVT::isInteger() have different semantics, which is quite confusing. EVT::isInteger() is currently like isIntOrIntVectorTy(). There is on function can check if an EVT is an scalar integer. Anyway, as I don't know whether it is proper to rename a function and add a function in EVT, and this will affect a lot of code, I don't do this in the patch.

Thanks,
-Hao

http://reviews.llvm.org/D3907

Files:
  lib/CodeGen/SelectionDAG/TargetLowering.cpp
  lib/Target/AArch64/AArch64ISelLowering.cpp
  test/CodeGen/AArch64/2014-05-16-shrink-v1i64.ll

Index: lib/CodeGen/SelectionDAG/TargetLowering.cpp
===================================================================
--- lib/CodeGen/SelectionDAG/TargetLowering.cpp
+++ lib/CodeGen/SelectionDAG/TargetLowering.cpp
@@ -327,6 +327,10 @@
   assert(Op.getNode()->getNumValues() == 1 &&
          "ShrinkDemandedOp only supports nodes with one result!");
 
+  // Early return, as this function cannot handle vector types.
+  if (Op.getValueType().isVector())
+    return false;
+
   // Don't do this if the node has another user, which may require the
   // full value.
   if (!Op.getNode()->hasOneUse())
Index: lib/Target/AArch64/AArch64ISelLowering.cpp
===================================================================
--- lib/Target/AArch64/AArch64ISelLowering.cpp
+++ lib/Target/AArch64/AArch64ISelLowering.cpp
@@ -6036,18 +6036,14 @@
     return false;
   unsigned NumBits1 = Ty1->getPrimitiveSizeInBits();
   unsigned NumBits2 = Ty2->getPrimitiveSizeInBits();
-  if (NumBits1 <= NumBits2)
-    return false;
-  return true;
+  return NumBits1 > NumBits2;
 }
 bool AArch64TargetLowering::isTruncateFree(EVT VT1, EVT VT2) const {
-  if (!VT1.isInteger() || !VT2.isInteger())
+  if (VT1.isVector() || VT2.isVector() || !VT1.isInteger() || !VT2.isInteger())
     return false;
   unsigned NumBits1 = VT1.getSizeInBits();
   unsigned NumBits2 = VT2.getSizeInBits();
-  if (NumBits1 <= NumBits2)
-    return false;
-  return true;
+  return NumBits1 > NumBits2;
 }
 
 // All 32-bit GPR operations implicitly zero the high-half of the corresponding
@@ -6057,18 +6053,14 @@
     return false;
   unsigned NumBits1 = Ty1->getPrimitiveSizeInBits();
   unsigned NumBits2 = Ty2->getPrimitiveSizeInBits();
-  if (NumBits1 == 32 && NumBits2 == 64)
-    return true;
-  return false;
+  return NumBits1 == 32 && NumBits2 == 64;
 }
 bool AArch64TargetLowering::isZExtFree(EVT VT1, EVT VT2) const {
-  if (!VT1.isInteger() || !VT2.isInteger())
+  if (VT1.isVector() || VT2.isVector() || !VT1.isInteger() || !VT2.isInteger())
     return false;
   unsigned NumBits1 = VT1.getSizeInBits();
   unsigned NumBits2 = VT2.getSizeInBits();
-  if (NumBits1 == 32 && NumBits2 == 64)
-    return true;
-  return false;
+  return NumBits1 == 32 && NumBits2 == 64;
 }
 
 bool AArch64TargetLowering::isZExtFree(SDValue Val, EVT VT2) const {
@@ -6082,7 +6074,7 @@
 
   // 8-, 16-, and 32-bit integer loads all implicitly zero-extend.
   return (VT1.isSimple() && VT1.isInteger() && VT2.isSimple() &&
-          VT2.isInteger() && VT1.getSizeInBits() <= 32);
+          !VT2.isVector() && VT2.isInteger() && VT1.getSizeInBits() <= 32);
 }
 
 bool AArch64TargetLowering::hasPairedLoad(Type *LoadedType,
Index: test/CodeGen/AArch64/2014-05-16-shrink-v1i64.ll
===================================================================
--- /dev/null
+++ test/CodeGen/AArch64/2014-05-16-shrink-v1i64.ll
@@ -0,0 +1,14 @@
+; RUN: llc -march=arm64 < %s
+
+; The DAGCombiner tries to do following shrink:
+;     Convert x+y to (VT)((SmallVT)x+(SmallVT)y)
+; But currently it can't handle vector type and will trigger an assertion failure
+; when it tries to generate an add mixed using vector type and scaler type.
+; This test checks that such assertion failur should not happen.
+define <1 x i64> @dotest(<1 x i64> %in0) {
+entry:
+  %0 = add <1 x i64> %in0, %in0
+  %vshl_n = shl <1 x i64> %0, <i64 32>
+  %vsra_n = ashr <1 x i64> %vshl_n, <i64 32>
+  ret <1 x i64> %vsra_n
+}
-------------- next part --------------
A non-text attachment was scrubbed...
Name: D3907.9797.patch
Type: text/x-patch
Size: 3435 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20140526/61ef60e6/attachment.bin>