[PATCH] D21454: [PM] Port MergedLoadStoreMotion to the new pass manager, take two.

[Sean Silva via llvm-commits]

silvas added a subscriber: silvas.
silvas added a comment.

A couple comments.


================
Comment at: include/llvm/Transforms/Scalar/MergedLoadStoreMotion.h:95
@@ +94,3 @@
+  // and the new pass manager.
+  MergedLoadStoreMotion Impl;
+};
----------------
davidxl wrote:
> Any reason we need to make it a member field, instead of just being created as a local instance at the time it is run? With that there is no need to hoist the class into the header.
In this case it does not matter. In other cases, it is preferable to `.clear()` maps/vectors between calls to `run` instead of reallocating from scratch (to save malloc traffic). E.g. SLPVectorizer in r272766.

I have not done measurement to see if this optimization helps, but it is the existing state of things.

================
Comment at: lib/Transforms/Scalar/MergedLoadStoreMotion.cpp:530
@@ +529,3 @@
+  }
+  MergedLoadStoreMotion Impl;
+};
----------------
Use the new-PM class as the Impl (or just make the Impl local to `runOnFunction`). See e.g. r272757


http://reviews.llvm.org/D21454