[PATCH] Small refactor on VectorizerHint for deduplication
Arch D. Robison
arch.robison at intel.com
Fri Aug 15 12:30:16 PDT 2014
Minor editorial comments. I'm not an LLVM expert.
================
Comment at: lib/Transforms/Vectorize/LoopVectorize.cpp:1117
@@ +1116,3 @@
+ bool matchesHintMetadataName(MDNode *Node, std::vector<HintType> &HintTypes) {
+ MDString* Name = cast<MDString>(Node->getOperand(0));
+ if (!Name)
----------------
Should the cast be dyn_cast?
================
Comment at: lib/Transforms/Vectorize/LoopVectorize.cpp:1137
@@ +1136,3 @@
+ for (unsigned i = 1, ie = LoopID->getNumOperands(); i < ie; ++i) {
+ MDNode *Node = cast<MDNode>(LoopID->getOperand(i));
+ // If node in update list, ignore old value.
----------------
Is it safe to assume that the operands are all MDNode and not other kinds of values? If not, consider not casting here, and instead pass a Value* to "matchesHintMetadataName" and do the check for MDNode-ness there.
================
Comment at: lib/Transforms/Vectorize/LoopVectorize.cpp:1139
@@ +1138,3 @@
+ // If node in update list, ignore old value.
+ if (matchesHintMetadataName(Node, HintTypes))
+ continue;
----------------
The if/continue/else seems a bit wordy. It would be shorter to write:
if (!matchesHintMetadataNameNode, HintTypes))
Vals.push_back(Node);
================
Comment at: lib/Transforms/Vectorize/LoopVectorize.cpp:1182
@@ -1133,2 +1181,3 @@
- MDNode *LoopID;
+ /// The loop this hints belong to.
+ const Loop *TheLoop;
----------------
"this" should be "these".
http://reviews.llvm.org/D4913
More information about the llvm-commits
mailing list