[PATCH] Small refactor on VectorizerHint for deduplication

[Renato Golin]

Thanks for the review, Arch.

Sending a new patch with the updates.

cheers,
--renato

================
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)
----------------
Arch D. Robison wrote:
> Should the cast be dyn_cast?
Yes, thanks!

================
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.
----------------
Arch D. Robison wrote:
> 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.
It should, since this is the only class touching that part. But I agree it's not the safest way to go. One needs to revise the metadata handling in the loop vectorizer as a whole, I believe. Maybe not for this patch, though.

================
Comment at: lib/Transforms/Vectorize/LoopVectorize.cpp:1139
@@ +1138,3 @@
+        // If node in update list, ignore old value.
+        if (matchesHintMetadataName(Node, HintTypes))
+          continue;
----------------
Arch D. Robison wrote:
> The if/continue/else seems a bit wordy.  It would be shorter to write:
> 
> if (!matchesHintMetadataNameNode, HintTypes))
>     Vals.push_back(Node);
That's the result of a different piece of code. Updated, thanks!

================
Comment at: lib/Transforms/Vectorize/LoopVectorize.cpp:1182
@@ -1133,2 +1181,3 @@
 
-  MDNode *LoopID;
+  /// The loop this hints belong to.
+  const Loop *TheLoop;
----------------
Arch D. Robison wrote:
> "this" should be "these".
Thanks!

http://reviews.llvm.org/D4913