[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