[PATCH] Using Masked Load / Store intrinsics in Loop Vectorizer

Arnold Schwaighofer aschwaighofer at apple.com
Thu Dec 4 08:35:42 PST 2014


Very nice! I only see minor fixes.

Thanks!

================
Comment at: lib/Transforms/Vectorize/LoopVectorize.cpp:778
@@ +777,3 @@
+  }
+  bool setMaskedOp(const Instruction* I) {
+    return (MaskedOp.find(I) != MaskedOp.end());
----------------
This is a predicate. Could you rename it to isMaskedOperation?

================
Comment at: lib/Transforms/Vectorize/LoopVectorize.cpp:889
@@ -874,1 +888,3 @@
+  /// call to an appropriate masked intrinsic
+  std::set<const Instruction*> MaskedOp;
 };
----------------
Should this be a SmallPtrSet?

================
Comment at: lib/Transforms/Vectorize/LoopVectorize.cpp:5457
@@ -5389,3 +5456,3 @@
 
-  assert(MaxVectorSize <= 32 && "Did not expect to pack so many elements"
+  assert(MaxVectorSize <= 64 && "Did not expect to pack so many elements"
          " into one vector!");
----------------
Should this be in a separate patch? This seems like an unrelated change.

================
Comment at: test/Transforms/LoopVectorize/X86/mask1.ll:1
@@ +1,2 @@
+; RUN: opt < %s  -O3 -mcpu=corei7-avx -S | FileCheck %s -check-prefix=AVX1
+; RUN: opt < %s  -O3 -mcpu=core-avx2 -S | FileCheck %s -check-prefix=AVX2
----------------
Could the mask1.ll, mask2.ll, etc be in one file? We try to not create to many .ll files when test cases are related.

Could you use

CHECK-LABEL: @function_name
CHECK: ... // rest of the test for function_name test

CHECK-LABEL: @function_name2
CHECK: ... // rest of the test for function_name2 test

Could you modify the test so that it tests more of the expected IR?

http://reviews.llvm.org/D6527






More information about the llvm-commits mailing list