[PATCH] [X86][SSE] Shuffle mask decode support for zero extend, scalar float/double moves and integer load instructions

Quentin Colombet qcolombet at apple.com
Wed Jan 28 17:33:33 PST 2015


Comment at: lib/Target/X86/InstPrinter/X86InstComments.cpp:24
@@ -23,1 +23,3 @@
+static void getZeroExtensionTypes(const MCInst *MI, MVT &SrcVT, MVT &DstVT) {
+  switch (MI->getOpcode()) {
Instead of returning SrcVT, I think, based on the uses of that function, that it would be better to return what you called the scale.
If you want to stick with SrcVT, see my next comment.

Comment at: lib/Target/X86/InstPrinter/X86InstComments.cpp:34
@@ +33,3 @@
+    SrcVT = MVT::v16i8;
+    DstVT = MVT::v8i16;
+    break;
The SrcVT is confusing to me.
The used input type is v8i8, not v16i8.
But indeed, the “register” type is v16i8.
If you want to use SrcVT, please clarify what is the expected output of this function.

Note: I like the use of SrcVT, instead of scale, but I found the types set here confusing.

So my suggestion is do either of:
1. Return Scale.
2. Add a comment to explain the return SrcVT is the content of the whole register, not just that is used.
3. Set SrcVT to what is actually used.

Comment at: lib/Target/X86/Utils/X86ShuffleDecode.cpp:428
@@ +427,3 @@
+  // first element comes from first element of second source
+  // load zero extends / move uses elements from first source
+  unsigned NumElts = VT.getVectorNumElements();
Format of the comments. Capital letter and period.



More information about the llvm-commits mailing list