<p dir="ltr">Hi Bill, this is the last patch to fix all 3.3 rc1 problems, which should give us a clean rc2 on ARM. </p>
<p dir="ltr">Cheers, <br>
Renato </p>
<div class="gmail_quote">On 15 May 2013 01:31, "Arnold Schwaighofer" <<a href="mailto:aschwaighofer@apple.com">aschwaighofer@apple.com</a>> wrote:<br type="attribution"><blockquote class="quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
Author: arnolds<br>
Date: Tue May 14 17:33:24 2013<br>
New Revision: 181842<br>
<br>
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=181842&view=rev" target="_blank">http://llvm.org/viewvc/llvm-project?rev=181842&view=rev</a><br>
Log:<br>
ARM ISel: Don't create illegal types during LowerMUL<br>
<br>
The transformation happening here is that we want to turn a<br>
"mul(ext(X), ext(X))" into a "vmull(X, X)", stripping off the extension. We have<br>
to make sure that X still has a valid vector type - possibly recreate an<br>
extension to a smaller type. In case of a extload of a memory type smaller than<br>
64 bit we used create a ext(load()). The problem with doing this - instead of<br>
recreating an extload - is that an illegal type is exposed.<br>
<br>
This patch fixes this by creating extloads instead of ext(load()) sequences.<br>
<br>
Fixes PR15970.<br>
<br>
radar://13871383<br>
<br>
Modified:<br>
    llvm/trunk/lib/Target/ARM/ARMISelLowering.cpp<br>
    llvm/trunk/test/CodeGen/ARM/vmul.ll<br>
<br>
Modified: llvm/trunk/lib/Target/ARM/ARMISelLowering.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/ARM/ARMISelLowering.cpp?rev=181842&r1=181841&r2=181842&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/ARM/ARMISelLowering.cpp?rev=181842&r1=181841&r2=181842&view=diff</a><br>

==============================================================================<br>
--- llvm/trunk/lib/Target/ARM/ARMISelLowering.cpp (original)<br>
+++ llvm/trunk/lib/Target/ARM/ARMISelLowering.cpp Tue May 14 17:33:24 2013<br>
@@ -5257,6 +5257,23 @@ static bool isZeroExtended(SDNode *N, Se<br>
   return false;<br>
 }<br>
<br>
+static EVT getExtensionTo64Bits(const EVT &OrigVT) {<br>
+  if (OrigVT.getSizeInBits() >= 64)<br>
+    return OrigVT;<br>
+<br>
+  assert(OrigVT.isSimple() && "Expecting a simple value type");<br>
+<br>
+  MVT::SimpleValueType OrigSimpleTy = OrigVT.getSimpleVT().SimpleTy;<br>
+  switch (OrigSimpleTy) {<br>
+  default: llvm_unreachable("Unexpected Vector Type");<br>
+  case MVT::v2i8:<br>
+  case MVT::v2i16:<br>
+     return MVT::v2i32;<br>
+  case MVT::v4i8:<br>
+    return  MVT::v4i16;<br>
+  }<br>
+}<br>
+<br>
 /// AddRequiredExtensionForVMULL - Add a sign/zero extension to extend the total<br>
 /// value size to 64 bits. We need a 64-bit D register as an operand to VMULL.<br>
 /// We insert the required extension here to get the vector to fill a D register.<br>
@@ -5272,18 +5289,8 @@ static SDValue AddRequiredExtensionForVM<br>
     return N;<br>
<br>
   // Must extend size to at least 64 bits to be used as an operand for VMULL.<br>
-  MVT::SimpleValueType OrigSimpleTy = OrigTy.getSimpleVT().SimpleTy;<br>
-  EVT NewVT;<br>
-  switch (OrigSimpleTy) {<br>
-  default: llvm_unreachable("Unexpected Orig Vector Type");<br>
-  case MVT::v2i8:<br>
-  case MVT::v2i16:<br>
-    NewVT = MVT::v2i32;<br>
-    break;<br>
-  case MVT::v4i8:<br>
-    NewVT = MVT::v4i16;<br>
-    break;<br>
-  }<br>
+  EVT NewVT = getExtensionTo64Bits(OrigTy);<br>
+<br>
   return DAG.getNode(ExtOpcode, N->getDebugLoc(), NewVT, N);<br>
 }<br>
<br>
@@ -5293,22 +5300,22 @@ static SDValue AddRequiredExtensionForVM<br>
 /// reach a total size of 64 bits. We have to add the extension separately<br>
 /// because ARM does not have a sign/zero extending load for vectors.<br>
 static SDValue SkipLoadExtensionForVMULL(LoadSDNode *LD, SelectionDAG& DAG) {<br>
-  SDValue NonExtendingLoad =<br>
-    DAG.getLoad(LD->getMemoryVT(), LD->getDebugLoc(), LD->getChain(),<br>
+  EVT ExtendedTy = getExtensionTo64Bits(LD->getMemoryVT());<br>
+<br>
+  // The load already has the right type.<br>
+  if (ExtendedTy == LD->getMemoryVT())<br>
+    return DAG.getLoad(LD->getMemoryVT(), LD->getDebugLoc(), LD->getChain(),<br>
                 LD->getBasePtr(), LD->getPointerInfo(), LD->isVolatile(),<br>
                 LD->isNonTemporal(), LD->isInvariant(),<br>
                 LD->getAlignment());<br>
-  unsigned ExtOp = 0;<br>
-  switch (LD->getExtensionType()) {<br>
-  default: llvm_unreachable("Unexpected LoadExtType");<br>
-  case ISD::EXTLOAD:<br>
-  case ISD::SEXTLOAD: ExtOp = ISD::SIGN_EXTEND; break;<br>
-  case ISD::ZEXTLOAD: ExtOp = ISD::ZERO_EXTEND; break;<br>
-  }<br>
-  MVT::SimpleValueType MemType = LD->getMemoryVT().getSimpleVT().SimpleTy;<br>
-  MVT::SimpleValueType ExtType = LD->getValueType(0).getSimpleVT().SimpleTy;<br>
-  return AddRequiredExtensionForVMULL(NonExtendingLoad, DAG,<br>
-                                      MemType, ExtType, ExtOp);<br>
+<br>
+  // We need to create a zextload/sextload. We cannot just create a load<br>
+  // followed by a zext/zext node because LowerMUL is also run during normal<br>
+  // operation legalization where we can't create illegal types.<br>
+  return DAG.getExtLoad(LD->getExtensionType(), LD->getDebugLoc(), ExtendedTy,<br>
+                        LD->getChain(), LD->getBasePtr(), LD->getPointerInfo(),<br>
+                        LD->getMemoryVT(), LD->isVolatile(),<br>
+                        LD->isNonTemporal(), LD->getAlignment());<br>
 }<br>
<br>
 /// SkipExtensionForVMULL - For a node that is a SIGN_EXTEND, ZERO_EXTEND,<br>
<br>
Modified: llvm/trunk/test/CodeGen/ARM/vmul.ll<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/ARM/vmul.ll?rev=181842&r1=181841&r2=181842&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/ARM/vmul.ll?rev=181842&r1=181841&r2=181842&view=diff</a><br>

==============================================================================<br>
--- llvm/trunk/test/CodeGen/ARM/vmul.ll (original)<br>
+++ llvm/trunk/test/CodeGen/ARM/vmul.ll Tue May 14 17:33:24 2013<br>
@@ -599,3 +599,27 @@ for.end179:<br>
 declare <8 x i16> @llvm.arm.neon.vrshiftu.v8i16(<8 x i16>, <8 x i16>) nounwind readnone<br>
 declare <8 x i16> @llvm.arm.neon.vqsubu.v8i16(<8 x i16>, <8 x i16>) nounwind readnone<br>
 declare <8 x i8> @llvm.arm.neon.vqmovnu.v8i8(<8 x i16>) nounwind readnone<br>
+<br>
+; vmull lowering would create a zext(v4i8 load()) instead of a zextload(v4i8),<br>
+; creating an illegal type during legalization and causing an assert.<br>
+; PR15970<br>
+define void @no_illegal_types_vmull_sext(<4 x i32> %a) {<br>
+entry:<br>
+  %wide.load283.i = load <4 x i8>* undef, align 1<br>
+  %0 = sext <4 x i8> %wide.load283.i to <4 x i32><br>
+  %1 = sub nsw <4 x i32> %0, %a<br>
+  %2 = mul nsw <4 x i32> %1, %1<br>
+  %predphi290.v.i = select <4 x i1> undef, <4 x i32> undef, <4 x i32> %2<br>
+  store <4 x i32> %predphi290.v.i, <4 x i32>* undef, align 4<br>
+  ret void<br>
+}<br>
+define void @no_illegal_types_vmull_zext(<4 x i32> %a) {<br>
+entry:<br>
+  %wide.load283.i = load <4 x i8>* undef, align 1<br>
+  %0 = zext <4 x i8> %wide.load283.i to <4 x i32><br>
+  %1 = sub nsw <4 x i32> %0, %a<br>
+  %2 = mul nsw <4 x i32> %1, %1<br>
+  %predphi290.v.i = select <4 x i1> undef, <4 x i32> undef, <4 x i32> %2<br>
+  store <4 x i32> %predphi290.v.i, <4 x i32>* undef, align 4<br>
+  ret void<br>
+}<br>
<br>
<br>
_______________________________________________<br>
llvm-commits mailing list<br>
<a href="mailto:llvm-commits@cs.uiuc.edu">llvm-commits@cs.uiuc.edu</a><br>
<a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits</a><br>
</blockquote></div>