[PATCH] D8943: Calculate vectorization factor using the narrowest type instead of widest type
Michael Zolotukhin via llvm-commits
llvm-commits at lists.llvm.org
Fri Oct 16 16:45:48 PDT 2015
mzolotukhin added a subscriber: mzolotukhin.
mzolotukhin added a comment.
Hi Cong,
Please find some comments inline.
Michael
================
Comment at: lib/Target/X86/X86TargetTransformInfo.cpp:851-853
@@ -850,4 +850,5 @@
std::pair<int, MVT> LT = TLI->getTypeLegalizationCost(DL, SrcVTy);
+ auto VT = TLI->getValueType(DL, SrcVTy);
int Cost = 0;
- if (LT.second != TLI->getValueType(DL, SrcVTy).getSimpleVT() &&
+ if (VT.isSimple() && LT.second != VT.getSimpleVT() &&
LT.second.getVectorNumElements() == NumElem)
----------------
I believe it's an independent fix from the rest of the patch. Please commit it separately.
================
Comment at: lib/Transforms/Vectorize/LoopVectorize.cpp:1401
@@ -1395,3 +1400,3 @@
- /// \return information about the register usage of the loop.
- RegisterUsage calculateRegisterUsage();
+ /// \return information about the register usages of the loop for the given
+ /// vectorization factors.
----------------
Nitpick: redundant whitespace after `\return`
================
Comment at: lib/Transforms/Vectorize/LoopVectorize.cpp:4605
@@ +4604,3 @@
+ // For each VF calculate its register usage.
+ auto RUs = calculateRegisterUsage(VFs);
+
----------------
I would prefer not changing signature of `calculateRegisterUsage` and build array of `RUs` here instead. I think the original interface (takes a single vectorization factor, return register usage object for it) is more intuitive that the one operating on arrays.
================
Comment at: lib/Transforms/Vectorize/LoopVectorize.cpp:4607
@@ +4606,3 @@
+
+ // Select the largest VF which doen't require more registers than existing
+ // ones.
----------------
Typo: doen't
================
Comment at: lib/Transforms/Vectorize/LoopVectorize.cpp:4994-4995
@@ -4948,3 +4993,4 @@
// Ignore instructions that are never used within the loop.
- if (!Ends.count(I)) continue;
+ if (!Ends.count(I))
+ continue;
----------------
Please commit such formatting fixes separately if you feel that you need them.
================
Comment at: test/Transforms/LoopVectorize/X86/vector_max_bandwidth.ll:1
@@ +1,2 @@
+; RUN: opt -loop-vectorize -vectorizer-maximize-bandwidth -mcpu=corei7-avx -debug-only=loop-vectorize -S < %s 2>&1 | FileCheck %s
+
----------------
You'll need `REQUIRES: asserts` if you scan debug dumps.
http://reviews.llvm.org/D8943
More information about the llvm-commits
mailing list