[PATCH] D17932: [X86][SSE] Simplify vector LOAD + EXTEND on pre-SSE41 hardware

[Quentin Colombet via llvm-commits]

qcolombet added inline comments.

================
Comment at: lib/CodeGen/SelectionDAG/LegalizeVectorTypes.cpp:938
@@ +937,3 @@
+
+  // *_EXTEND_VECTOR_INREG instructions extends the lowest lanes of the input
+  // vector. We don't need InHi, instead we access the InLo lanes just after
----------------
extend

================
Comment at: lib/CodeGen/SelectionDAG/LegalizeVectorTypes.cpp:940
@@ +939,3 @@
+  // vector. We don't need InHi, instead we access the InLo lanes just after
+  // the lanes used for Lo - so we shuffle those lanes in place.
+  EVT InLoVT = InLo.getValueType();
----------------
Could you rephrase, I admit I understood the comment after I understood the code :).

================
Comment at: lib/CodeGen/SelectionDAG/LegalizeVectorTypes.cpp:944
@@ +943,3 @@
+  for (unsigned i = 0, e = HiVT.getVectorNumElements(); i != e; ++i)
+    SplitHi[i] = i + LoVT.getVectorNumElements();
+
----------------
Something saying that we build the mask that takes the high element of the low input perhaps?

What through me off for the understanding, at first, here was the name of the variable LoVT. It is not obvious that LoVT is the type of the destination LoVT, not the type of the input LoVT. I guess it does not hurt to use DestLoVT as opposed to InLoVT, instead of LoVT vs. InLoVT.

That being said, up to you, I am fine with the current version, but try to improve the comment :).


Repository:
  rL LLVM

http://reviews.llvm.org/D17932