[PATCH] [mips] Correct and improve special-case shuffle instructions.
Vasileios Kalintiris
Vasileios.Kalintiris at imgtec.com
Thu May 14 04:21:31 PDT 2015
LGTM with a couple nits/suggestions.
================
Comment at: lib/Target/Mips/MipsSEISelLowering.cpp:2468-2469
@@ -2467,1 +2467,4 @@
+/// Determine whether a range fits a regular pattern of values.
+/// This function accounts for the possibility of jumping over the End iterator.
+template <typename ValType>
----------------
Did you intend to use 3 slashes instead of 2?
================
Comment at: lib/Target/Mips/MipsSEISelLowering.cpp:2470-2475
@@ +2469,8 @@
+/// This function accounts for the possibility of jumping over the End iterator.
+template <typename ValType>
+static bool
+fitsRegularPattern(typename SmallVectorImpl<ValType>::const_iterator Begin,
+ unsigned CheckStride,
+ typename SmallVectorImpl<ValType>::const_iterator End,
+ ValType ExpectedIndex, unsigned ExpectedIndexStride) {
+ auto &I = Begin;
----------------
- Every instantiation of the template is using an `int`. There's no need for it.
- Just a personal preference but can we swap the 2nd argument with the 3rd, so that `End` comes right after `Begin`?
- Also, can we document a little bit the role of each argument and find a more descriptive name for ExpectedIndex?
================
Comment at: test/CodeGen/Mips/msa/shuffle.ll:4
@@ -3,2 +3,3 @@
define void @vshf_v16i8_0(<16 x i8>* %c, <16 x i8>* %a, <16 x i8>* %b) nounwind {
+ ; CHECK-LABEL: vshf_v16i8_0:
----------------
The 3rd argument is unused in this and some of the following tests. Can we remove it from those function definitions while we're at it?
http://reviews.llvm.org/D9660
EMAIL PREFERENCES
http://reviews.llvm.org/settings/panel/emailpreferences/
More information about the llvm-commits
mailing list