[PATCH] D45552: [NFC][LV][LoopUtil] Move LoopVectorizationLegality to Analysis tree

[Diego Caballero via Phabricator via llvm-commits]

dcaballe added a comment.

Some minor comments inline. Since Renato already approved this patch I can give you LGTM after that.

> I thought about simply eliminating asserts in Legal that are based on "EnableVPlanNativePath",

These asserts guarantee that code in the VPlan native path is not executed when such a path is not enabled. They helped me caught some issues when doing stress testing mode so I think it's a good idea to keep them.

Thanks,
Diego



================
Comment at: include/llvm/Transforms/Vectorize/LoopVectorizationLegality.h:238
+  /// loop, only that it is legal to do so.
+  bool canVectorize(bool UseVPlanNativePath);
+
----------------
hsaito wrote:
> New parameter added.
Could you please document the flag here and in the other two places below?


================
Comment at: lib/Transforms/Vectorize/LoopVectorize.cpp:7319
                     "attribute is used.\n");
-    ORE->emit(createMissedAnalysis(Hints.vectorizeAnalysisPassName(),
-                                   "NoImplicitFloat", L)
+    ORE->emit(createLVMissedAnalysis(Hints.vectorizeAnalysisPassName(),
+                                     "NoImplicitFloat", L)
----------------
I'm probably missing something but why this renaming is necessary?


https://reviews.llvm.org/D45552