[llvm-commits] Please Review: ZERO_EXTEND/SIGN_EXTEND/TRUNCATE optimization for AVX2

[Demikhovsky, Elena]

I fixed all and committed the changes.

Thank you for reviewing this.

- Elena
From: Craig Topper [mailto:craig.topper at gmail.com]
Sent: Saturday, April 21, 2012 09:57
To: Chad Rosier
Cc: Demikhovsky, Elena; llvm-commits at cs.uiuc.edu
Subject: Re: [llvm-commits] Please Review: ZERO_EXTEND/SIGN_EXTEND/TRUNCATE optimization for AVX2

The test case is full of windows line endings.

+      // VPERMD
+      int ShufMask[] = {0, 2, 4, 6, -1, -1, -1, -1};

That should probably be "static const int ShufMask"


+      int ShufMask[] = {0,  2,  -1,  -1};

Same here. And only use 1 space after the comma.


+      Op = DAG.getNode(X86ISD::PSHUFB, dl, MVT::v32i8, Op, BV);
+
+      Op = DAG.getNode(ISD::BITCAST, dl, MVT::v4i64, Op);
+
+      int ShufMask[] = {0,  2,  -1,  -1};
+      Op = DAG.getVectorShuffle(MVT::v4i64, dl,  Op, DAG.getUNDEF(MVT::v4i64), &ShufMask[0]);
+
+      Op =  DAG.getNode(ISD::EXTRACT_SUBVECTOR, dl, MVT::v2i64, Op, DAG.getIntPtrConstant(0));
+
+      return DAG.getNode(ISD::BITCAST, dl, VT, Op);

There are some dangling spaces in there. Also need to fit in 80 columns


+      SDValue pshufbConstPool = DAG.getNode(ISD::BUILD_VECTOR, dl,
+                                 MVT::v32i8, &pshufbMask[0], 32);

This looks unused


     if (((VT == MVT::v8i32) && (OpVT == MVT::v8i16))  ||
       ((VT == MVT::v4i64) && (OpVT == MVT::v4i32)))  {

This isn't your code, but can you fix the alignment so that the second line starts after the first line's opening parenthese.


+let Predicates = [HasAVX2] in {
+  let AddedComplexity = 15 in {
+    def : Pat<(v4i64 (X86vzmovly (v4i32 VR128:$src))), (VPMOVZXDQYrr VR128:$src)>;
+    def : Pat<(v8i32 (X86vzmovly (v8i16 VR128:$src))), (VPMOVZXWDYrr VR128:$src)>;
+  }
+}

Why the AddedComplexity? Also 80 columns.


+      AddToWorkList(Op.getNode());
     } else if (Op.getValueType().bitsGT(VT)) {
       Op = DAG.getNode(ISD::TRUNCATE, N->getDebugLoc(), VT, Op);
-    }
+      AddToWorkList(Op.getNode());
+   }

Last bracket needs to be right 1 space.

On Thu, Apr 19, 2012 at 10:50 AM, Craig Topper <craig.topper at gmail.com<mailto:craig.topper at gmail.com>> wrote:
On Thu, Apr 19, 2012 at 10:29 AM, Chad Rosier <mcrosier at apple.com<mailto:mcrosier at apple.com>> wrote:
Mostly stylistic comments:

+      SmallVector<SDValue,32> pshufbMask;
+      for (unsigned i=0; i<2; i++) {

Spacing: for (unsigned i = 0; i < 2; i++)

That should also be "++i" to match coding standards.

I'll try to review the content tonight.

--
~Craig



--
~Craig
---------------------------------------------------------------------
Intel Israel (74) Limited

This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20120422/9b0394f4/attachment.html>