[PM] Port Scalarizer to the new pass manager.

[fedor.sergeev]

To me it would be a cleaner solution if you could split legacy FunctionPass interface implementation into a separate wrapper pass (e.g. ScalarizerLegacyPass),
similar to how new-pm wrapper ScalarizerPass implements all the new-pm interfaces.
That would allow to separate actual transformation (Scalarizer) from pass-manager details (ScalarizerPass/ScalarizerLegacyPass).

Say, you would not need to perform legacy initializeScalarizerPass when initializing Scalarizer object for new-pm operations.

lib/Transforms/Scalar/Scalarizer.cpp

159 ↗

(On Diff #174581)

Can this friend declaration be removed?

[markus]

Agree that splitting would give a cleaner solution. Will give that a go.

lib/Transforms/Scalar/Scalarizer.cpp

159 ↗

(On Diff #174581)

If I do the splitting suggested above then yes, most likely it can.

[markus]

Split into one wrapper class for the legacy PM and one wrapper class for the new PM.

[chandlerc]

Bunch of somewhat minor API cleanups that seem reasonable to do when touching these specific bits of the code...

(Please, feel free to continue / finish the review Fedor, just wanted to throw these out while I was looking at the code...)

include/llvm/Transforms/Scalar/Scalarizer.h

9–14 ↗

(On Diff #174726)

Make this a file-level doxygen comment? Or better yet, move it to be a class-level doxygen comment on the class below?

27 ↗

(On Diff #174726)

I've also tried to move the create... functions to build the legacy pass into these headers when introducing them as I think that makes things much cleaner.

lib/Transforms/Scalar/Scalarizer.cpp

164 ↗

(On Diff #174726)

Naming suggestion (but minor): you could call this ScalarizerVisitor and then name the variables below Visitor which seems to give a bit more information.

167 ↗

(On Diff #174726)

While here, I would suggest removing the do from the name as it doesn't add any information. transform is already a great verb for this function.

Actually, I might suggest naming this visit to explicitly shadow the InstVisitors visit methods as it would be an error for a user to accidentally call one of those on this class (there is clearly very precise logic for this one that needs to be used).

167 ↗

(On Diff #174726)

Since this is never called with a null dominator tree, how about accepting it by reference?

Personally, I'd move everything but the function to the constructor so that the main interface to this visitor mirrors that of the underlying InstVisitor with just top-level visit methods.

[fedor.sergeev]

Overall it looks really good!

lib/Transforms/Scalar/Scalarizer.cpp

167 ↗

(On Diff #174726)

Agreed, moving DT and metadata initialization to constructor seems to be a very logical step forward.

test/Transforms/Scalarizer/basic.ll

2 ↗

(On Diff #174726)

There are more scalarizer tests under test/Tranforms/Scalarizer.
Why not converting them all?

[markus]

Hopefully this patch solves most of the comments I have received so far.

You may notice that some stuff about dominator trees have simply disappeared from the patch and this is because I previously, by accident, managed to include some downstream local changes that are note relevant for upstreaming.

[fedor.sergeev]

Looks nearly ready to go.

lib/Transforms/Scalar/Scalarizer.cpp

301 ↗

(On Diff #174754)

Would be nice to move this into the constructor (and thus pass ParallelLoopAccessMDKind parameter to that constructor and not to transform).

[markus]

Moved ParallelLoopAccessMDKind into constructor.

[fedor.sergeev]

LGTM, but feel free to address Chandlers' note if you dont have strong feelings against it.

lib/Transforms/Scalar/Scalarizer.cpp

169 ↗

(On Diff #174762)

Chandler suggestion was to rename this method to 'visit'.
I dont have strong preference here, though his reasoning about shadowing InstVisitor's methods sorta makes sense.

[markus]

I do not have a strong opinion about renaming 'transform' to 'visit' and would be happy to comply but simply doing a rename does not compile giving errors such as

../lib/Transforms/Scalar/Scalarizer.cpp:311:25: error: non-const lvalue reference to type 'llvm::Function' cannot bind to a value of unrelated type 'llvm::Instruction *'
      bool Done = visit(I);
                        ^
../lib/Transforms/Scalar/Scalarizer.cpp:302:41: note: passing argument to parameter 'F' here
bool ScalarizerVisitor::visit(Function &F) {
                                        ^

so it would appear to require some additional changes to make it work.

[fedor.sergeev]

In D54695#1305054, @markus wrote:

../lib/Transforms/Scalar/Scalarizer.cpp:311:25: error: non-const lvalue reference to type 'llvm::Function' cannot bind to a value of unrelated type 'llvm::Instruction *'

bool Done = visit(I);

You can use explicit qualification here:

bool Done = InstVisitor::visit(I);

[fedor.sergeev]

Looks ok, feel free to push. Thanks!

[uabelho]

I submitted this for markus since he doesn't have commit access yet.

[fedor.sergeev]

In D54695#1305258, @uabelho wrote:

I submitted this for markus since he doesn't have commit access yet.

Ah, sure, thanks!

[bjope]

llvm/trunk/lib/Transforms/Scalar/Scalarizer.cpp

822

(sorry for post commit comments, but I ran into minor trouble when trying to merge this with our downstream branch)

Not sure if it matters really, but why are we saying that we preserve "none" here?
If Impl.visit returns false, it means that we didn't do any changes. And then I think it would be more appropriate to return PreservedAnalyses::all().

Isn't that what happens for the legacy version. When runOnFunction returns that nothing was modified it will implicitly also say that it preserve all analyses, even if getAnalysisUsage doesn't say so (or maybe I remember wrong about that).

So there might be a slight difference about preserved analyses here, when the pass isn't doing any rewrites at all.

[fedor.sergeev]

@bjope:
Ugh... thanks for noting this, no idea how I managed to miss that during review.
Will patch it myself.

[fedor.sergeev]

return value for Scalarizer::run fixed in rL347432

[bjope]

In D54695#1305771, @fedor.sergeev wrote:

return value for Scalarizer::run fixed in rL347432

Thanks! Looks nice!

llvm/trunk/include/llvm/InitializePasses.h

 void initializeSLPVectorizerPass(PassRegistry&);
 void initializeSROALegacyPassPass(PassRegistry&);
 void initializeSafeStackLegacyPassPass(PassRegistry&);
 void initializeSafepointIRVerifierPass(PassRegistry&);
 void initializeSampleProfileLoaderLegacyPassPass(PassRegistry&);
 void initializeSanitizerCoverageModulePass(PassRegistry&);
 void initializeScalarEvolutionWrapperPassPass(PassRegistry&);
 void initializeScalarizeMaskedMemIntrinPass(PassRegistry&);
-void initializeScalarizerPass(PassRegistry&);
+void initializeScalarizerLegacyPassPass(PassRegistry&);
 void initializeScavengerTestPass(PassRegistry&);
 void initializeScopedNoAliasAAWrapperPassPass(PassRegistry&);
 void initializeSeparateConstOffsetFromGEPPass(PassRegistry&);
 void initializeShadowStackGCLoweringPass(PassRegistry&);
 void initializeShrinkWrapPass(PassRegistry&);
 void initializeSimpleInlinerPass(PassRegistry&);
 void initializeSimpleLoopUnswitchLegacyPassPass(PassRegistry&);
 void initializeSingleLoopExtractorPass(PassRegistry&);

llvm/trunk/include/llvm/LinkAllPasses.h

 #include "llvm/Transforms/IPO/FunctionAttrs.h"
 #include "llvm/Transforms/InstCombine/InstCombine.h"
 #include "llvm/Transforms/Instrumentation.h"
 #include "llvm/Transforms/Instrumentation/BoundsChecking.h"
 #include "llvm/Transforms/ObjCARC.h"
 #include "llvm/Transforms/Scalar.h"
 #include "llvm/Transforms/Scalar/GVN.h"
 #include "llvm/Transforms/Scalar/InstSimplifyPass.h"
+#include "llvm/Transforms/Scalar/Scalarizer.h"
 #include "llvm/Transforms/Utils.h"
 #include "llvm/Transforms/Utils/SymbolRewriter.h"
 #include "llvm/Transforms/Utils/UnifyFunctionExitNodes.h"
 #include "llvm/Transforms/Vectorize.h"
 #include <cstdlib>
 
 namespace {
   struct ForcePassLinking {

llvm/trunk/include/llvm/Transforms/Scalar.h

 //
 // PartiallyInlineLibCalls - Tries to inline the fast path of library
 // calls such as sqrt.
 //
 FunctionPass *createPartiallyInlineLibCallsPass();
 
 //===----------------------------------------------------------------------===//
 //
-// ScalarizerPass - Converts vector operations into scalar operations
-//
-FunctionPass *createScalarizerPass();
-
-//===----------------------------------------------------------------------===//
-//
 // SeparateConstOffsetFromGEP - Split GEPs for better CSE
 //
 FunctionPass *createSeparateConstOffsetFromGEPPass(bool LowerGEP = false);
 
 //===----------------------------------------------------------------------===//
 //
 // SpeculativeExecution - Aggressively hoist instructions to enable
 // speculative execution on targets where branches are expensive.

llvm/trunk/include/llvm/Transforms/Scalar/Scalarizer.h

+//===- Scalarizer.h --- Scalarize vector operations -----------------------===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+//
+/// \file
+/// This pass converts vector operations into scalar operations, in order
+/// to expose optimization opportunities on the individual scalar operations.
+/// It is mainly intended for targets that do not have vector units, but it
+/// may also be useful for revectorizing code to different vector widths.
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_TRANSFORMS_SCALAR_SCALARIZER_H
+#define LLVM_TRANSFORMS_SCALAR_SCALARIZER_H
+
+#include "llvm/IR/PassManager.h"
+
+namespace llvm {
+
+class ScalarizerPass : public PassInfoMixin<ScalarizerPass> {
+public:
+  PreservedAnalyses run(Function &F, FunctionAnalysisManager &AM);
+};
+
+/// Create a legacy pass manager instance of the Scalarizer pass
+FunctionPass *createScalarizerPass();
+
+}
+
+#endif /* LLVM_TRANSFORMS_SCALAR_SCALARIZER_H */

llvm/trunk/lib/Passes/PassBuilder.cpp

 #include "llvm/Transforms/Scalar/MergedLoadStoreMotion.h"
 #include "llvm/Transforms/Scalar/NaryReassociate.h"
 #include "llvm/Transforms/Scalar/NewGVN.h"
 #include "llvm/Transforms/Scalar/PartiallyInlineLibCalls.h"
 #include "llvm/Transforms/Scalar/Reassociate.h"
 #include "llvm/Transforms/Scalar/RewriteStatepointsForGC.h"
 #include "llvm/Transforms/Scalar/SCCP.h"
 #include "llvm/Transforms/Scalar/SROA.h"
+#include "llvm/Transforms/Scalar/Scalarizer.h"
 #include "llvm/Transforms/Scalar/SimpleLoopUnswitch.h"
 #include "llvm/Transforms/Scalar/SimplifyCFG.h"
 #include "llvm/Transforms/Scalar/Sink.h"
 #include "llvm/Transforms/Scalar/SpeculateAroundPHIs.h"
 #include "llvm/Transforms/Scalar/SpeculativeExecution.h"
 #include "llvm/Transforms/Scalar/TailRecursionElimination.h"
 #include "llvm/Transforms/Utils/AddDiscriminators.h"
 #include "llvm/Transforms/Utils/BreakCriticalEdges.h"

llvm/trunk/lib/Passes/PassRegistry.def

 FUNCTION_PASS("print<demanded-bits>", DemandedBitsPrinterPass(dbgs()))
 FUNCTION_PASS("print<domfrontier>", DominanceFrontierPrinterPass(dbgs()))
 FUNCTION_PASS("print<loops>", LoopPrinterPass(dbgs()))
 FUNCTION_PASS("print<memoryssa>", MemorySSAPrinterPass(dbgs()))
 FUNCTION_PASS("print<phi-values>", PhiValuesPrinterPass(dbgs()))
 FUNCTION_PASS("print<regions>", RegionInfoPrinterPass(dbgs()))
 FUNCTION_PASS("print<scalar-evolution>", ScalarEvolutionPrinterPass(dbgs()))
 FUNCTION_PASS("reassociate", ReassociatePass())
+FUNCTION_PASS("scalarizer", ScalarizerPass())
 FUNCTION_PASS("sccp", SCCPPass())
 FUNCTION_PASS("simplify-cfg", SimplifyCFGPass())
 FUNCTION_PASS("sink", SinkingPass())
 FUNCTION_PASS("slp-vectorizer", SLPVectorizerPass())
 FUNCTION_PASS("speculative-execution", SpeculativeExecutionPass())
 FUNCTION_PASS("spec-phis", SpeculateAroundPHIsPass())
 FUNCTION_PASS("sroa", SROA())
 FUNCTION_PASS("tailcallelim", TailCallElimPass())

llvm/trunk/lib/Transforms/Scalar/Scalar.cpp

 #include "llvm/Analysis/Passes.h"
 #include "llvm/Analysis/ScopedNoAliasAA.h"
 #include "llvm/Analysis/TypeBasedAliasAnalysis.h"
 #include "llvm/IR/DataLayout.h"
 #include "llvm/IR/LegacyPassManager.h"
 #include "llvm/IR/Verifier.h"
 #include "llvm/InitializePasses.h"
 #include "llvm/Transforms/Scalar/GVN.h"
+#include "llvm/Transforms/Scalar/Scalarizer.h"
 #include "llvm/Transforms/Scalar/SimpleLoopUnswitch.h"
 #include "llvm/Transforms/Utils/UnifyFunctionExitNodes.h"
 
 using namespace llvm;
 
 /// initializeScalarOptsPasses - Initialize all passes linked into the
 /// ScalarOpts library.
 void llvm::initializeScalarOpts(PassRegistry &Registry) {
   initializeADCELegacyPassPass(Registry);
   initializeBDCELegacyPassPass(Registry);
   initializeAlignmentFromAssumptionsPass(Registry);
   initializeCallSiteSplittingLegacyPassPass(Registry);
   initializeConstantHoistingLegacyPassPass(Registry);
   initializeConstantPropagationPass(Registry);
   initializeCorrelatedValuePropagationPass(Registry);
   initializeDCELegacyPassPass(Registry);
   initializeDeadInstEliminationPass(Registry);
   initializeDivRemPairsLegacyPassPass(Registry);
-  initializeScalarizerPass(Registry);
+  initializeScalarizerLegacyPassPass(Registry);
   initializeDSELegacyPassPass(Registry);
   initializeGuardWideningLegacyPassPass(Registry);
   initializeLoopGuardWideningLegacyPassPass(Registry);
   initializeGVNLegacyPassPass(Registry);
   initializeNewGVNLegacyPassPass(Registry);
   initializeEarlyCSELegacyPassPass(Registry);
   initializeEarlyCSEMemSSALegacyPassPass(Registry);
   initializeGVNHoistLegacyPassPass(Registry);

llvm/trunk/lib/Transforms/Scalar/Scalarizer.cpp

 #include "llvm/IR/Module.h"
 #include "llvm/IR/Type.h"
 #include "llvm/IR/Value.h"
 #include "llvm/Pass.h"
 #include "llvm/Support/Casting.h"
 #include "llvm/Support/MathExtras.h"
 #include "llvm/Support/Options.h"
 #include "llvm/Transforms/Scalar.h"
+#include "llvm/Transforms/Scalar/Scalarizer.h"
 #include <cassert>
 #include <cstdint>
 #include <iterator>
 #include <map>
 #include <utility>
 
 using namespace llvm;
 
 #define DEBUG_TYPE "scalarizer"
 
+// This is disabled by default because having separate loads and stores
+// makes it more likely that the -combiner-alias-analysis limits will be
+// reached.
+static cl::opt<bool>
+    ScalarizeLoadStore("scalarize-load-store", cl::init(false), cl::Hidden,
+                       cl::desc("Allow the scalarizer pass to scalarize loads and store"));
+
 namespace {
 
 // Used to store the scattered form of a vector.
 using ValueVector = SmallVector<Value *, 8>;
 
 // Used to map a vector Value to its scattered form.  We use std::map
 // because we want iterators to persist across insertion and because the
 // values are relatively large.
struct VectorLayout {
 
   // The alignment of the vector.
   uint64_t VecAlign = 0;
 
   // The size of each element.
   uint64_t ElemSize = 0;
 };
 
-class Scalarizer : public FunctionPass,
-                   public InstVisitor<Scalarizer, bool> {
+class ScalarizerVisitor : public InstVisitor<ScalarizerVisitor, bool> {
 public:
-  static char ID;
-
-  Scalarizer() : FunctionPass(ID) {
-    initializeScalarizerPass(*PassRegistry::getPassRegistry());
+  ScalarizerVisitor(unsigned ParallelLoopAccessMDKind)
+    : ParallelLoopAccessMDKind(ParallelLoopAccessMDKind) {
   }
 
-  bool doInitialization(Module &M) override;
-  bool runOnFunction(Function &F) override;
+  bool visit(Function &F);
 
   // InstVisitor methods.  They return true if the instruction was scalarized,
   // false if nothing changed.
   bool visitInstruction(Instruction &I) { return false; }
   bool visitSelectInst(SelectInst &SI);
   bool visitICmpInst(ICmpInst &ICI);
   bool visitFCmpInst(FCmpInst &FCI);
   bool visitBinaryOperator(BinaryOperator &BO);
   bool visitGetElementPtrInst(GetElementPtrInst &GEPI);
   bool visitCastInst(CastInst &CI);
   bool visitBitCastInst(BitCastInst &BCI);
   bool visitShuffleVectorInst(ShuffleVectorInst &SVI);
   bool visitPHINode(PHINode &PHI);
   bool visitLoadInst(LoadInst &LI);
   bool visitStoreInst(StoreInst &SI);
   bool visitCallInst(CallInst &ICI);
 
-  static void registerOptions() {
-    // This is disabled by default because having separate loads and stores
-    // makes it more likely that the -combiner-alias-analysis limits will be
-    // reached.
-    OptionRegistry::registerOption<bool, Scalarizer,
-                                 &Scalarizer::ScalarizeLoadStore>(
-        "scalarize-load-store",
-        "Allow the scalarizer pass to scalarize loads and store", false);
-  }
-
 private:
   Scatterer scatter(Instruction *Point, Value *V);
   void gather(Instruction *Op, const ValueVector &CV);
   bool canTransferMetadata(unsigned Kind);
   void transferMetadata(Instruction *Op, const ValueVector &CV);
   bool getVectorLayout(Type *Ty, unsigned Alignment, VectorLayout &Layout,
                        const DataLayout &DL);
   bool finish();
 
   template<typename T> bool splitBinary(Instruction &, const T &);
 
   bool splitCall(CallInst &CI);
 
   ScatterMap Scattered;
   GatherList Gathered;
+
   unsigned ParallelLoopAccessMDKind;
-  bool ScalarizeLoadStore;
 };
 
-} // end anonymous namespace
+class ScalarizerLegacyPass : public FunctionPass {
+public:
+  static char ID;
+
+  ScalarizerLegacyPass() : FunctionPass(ID) {
+    initializeScalarizerLegacyPassPass(*PassRegistry::getPassRegistry());
+  }
 
-char Scalarizer::ID = 0;
+  bool runOnFunction(Function &F) override;
+};
+
+} // end anonymous namespace
 
-INITIALIZE_PASS_WITH_OPTIONS(Scalarizer, "scalarizer",
-                             "Scalarize vector operations", false, false)
+char ScalarizerLegacyPass::ID = 0;
+INITIALIZE_PASS_BEGIN(ScalarizerLegacyPass, "scalarizer",
+                      "Scalarize vector operations", false, false)
+INITIALIZE_PASS_END(ScalarizerLegacyPass, "scalarizer",
 
 Scatterer::Scatterer(BasicBlock *bb, BasicBlock::iterator bbi, Value *v,
                      ValueVector *cachePtr)
   : BB(bb), BBI(bbi), V(v), CachePtr(cachePtr) {
   Type *Ty = V->getType();
   PtrTy = dyn_cast<PointerType>(Ty);
   if (PtrTy)
     Ty = PtrTy->getElementType();
while (true) {
       }
     }
     CV[I] = Builder.CreateExtractElement(V, Builder.getInt32(I),
                                          V->getName() + ".i" + Twine(I));
   }
   return CV[I];
 }
 
-bool Scalarizer::doInitialization(Module &M) {
-  ParallelLoopAccessMDKind =
-      M.getContext().getMDKindID("llvm.mem.parallel_loop_access");
-  ScalarizeLoadStore =
-      M.getContext().getOption<bool, Scalarizer, &Scalarizer::ScalarizeLoadStore>();
+bool ScalarizerLegacyPass::runOnFunction(Function &F) {
+  if (skipFunction(F))
     return false;
+
+  Module &M = *F.getParent();
+  unsigned ParallelLoopAccessMDKind =
+      M.getContext().getMDKindID("llvm.mem.parallel_loop_access");
+  ScalarizerVisitor Impl(ParallelLoopAccessMDKind);
+  return Impl.visit(F);
 }
 
-bool Scalarizer::runOnFunction(Function &F) {
-  if (skipFunction(F))
-    return false;
+FunctionPass *llvm::createScalarizerPass() {
+  return new ScalarizerLegacyPass();
+}
+
+bool ScalarizerVisitor::visit(Function &F) {
   assert(Gathered.empty() && Scattered.empty());
 
   // To ensure we replace gathered components correctly we need to do an ordered
   // traversal of the basic blocks in the function.
   ReversePostOrderTraversal<BasicBlock *> RPOT(&F.getEntryBlock());
   for (BasicBlock *BB : RPOT) {
     for (BasicBlock::iterator II = BB->begin(), IE = BB->end(); II != IE;) {
       Instruction *I = &*II;
-      bool Done = visit(I);
+      bool Done = InstVisitor::visit(I);
       ++II;
       if (Done && I->getType()->isVoidTy())
         I->eraseFromParent();
     }
   }
   return finish();
 }
 
 // Return a scattered form of V that can be accessed by Point.  V must be a
 // vector or a pointer to a vector.
-Scatterer Scalarizer::scatter(Instruction *Point, Value *V) {
+Scatterer ScalarizerVisitor::scatter(Instruction *Point, Value *V) {
   if (Argument *VArg = dyn_cast<Argument>(V)) {
     // Put the scattered form of arguments in the entry block,
     // so that it can be used everywhere.
     Function *F = VArg->getParent();
     BasicBlock *BB = &F->getEntryBlock();
     return Scatterer(BB, BB->begin(), V, &Scattered[V]);
   }
   if (Instruction *VOp = dyn_cast<Instruction>(V)) {
     // Put the scattered form of an instruction directly after the
     // instruction.
     BasicBlock *BB = VOp->getParent();
     return Scatterer(BB, std::next(BasicBlock::iterator(VOp)),
                      V, &Scattered[V]);
   }
   // In the fallback case, just put the scattered before Point and
   // keep the result local to Point.
   return Scatterer(Point->getParent(), Point->getIterator(), V);
 }
 
 // Replace Op with the gathered form of the components in CV.  Defer the
 // deletion of Op and creation of the gathered form to the end of the pass,
 // so that we can avoid creating the gathered form if all uses of Op are
 // replaced with uses of CV.
-void Scalarizer::gather(Instruction *Op, const ValueVector &CV) {
+void ScalarizerVisitor::gather(Instruction *Op, const ValueVector &CV) {
   // Since we're not deleting Op yet, stub out its operands, so that it
   // doesn't make anything live unnecessarily.
   for (unsigned I = 0, E = Op->getNumOperands(); I != E; ++I)
     Op->setOperand(I, UndefValue::get(Op->getOperand(I)->getType()));
 
   transferMetadata(Op, CV);
 
   // If we already have a scattered form of Op (created from ExtractElements
if (!SV.empty()) {
     }
   }
   SV = CV;
   Gathered.push_back(GatherList::value_type(Op, &SV));
 }
 
 // Return true if it is safe to transfer the given metadata tag from
 // vector to scalar instructions.
-bool Scalarizer::canTransferMetadata(unsigned Tag) {
+bool ScalarizerVisitor::canTransferMetadata(unsigned Tag) {
   return (Tag == LLVMContext::MD_tbaa
           || Tag == LLVMContext::MD_fpmath
           || Tag == LLVMContext::MD_tbaa_struct
           || Tag == LLVMContext::MD_invariant_load
           || Tag == LLVMContext::MD_alias_scope
           || Tag == LLVMContext::MD_noalias
           || Tag == ParallelLoopAccessMDKind);
 }
 
 // Transfer metadata from Op to the instructions in CV if it is known
 // to be safe to do so.
-void Scalarizer::transferMetadata(Instruction *Op, const ValueVector &CV) {
+void ScalarizerVisitor::transferMetadata(Instruction *Op, const ValueVector &CV) {
   SmallVector<std::pair<unsigned, MDNode *>, 4> MDs;
   Op->getAllMetadataOtherThanDebugLoc(MDs);
   for (unsigned I = 0, E = CV.size(); I != E; ++I) {
     if (Instruction *New = dyn_cast<Instruction>(CV[I])) {
       for (const auto &MD : MDs)
         if (canTransferMetadata(MD.first))
           New->setMetadata(MD.first, MD.second);
       if (Op->getDebugLoc() && !New->getDebugLoc())
         New->setDebugLoc(Op->getDebugLoc());
     }
   }
 }
 
 // Try to fill in Layout from Ty, returning true on success.  Alignment is
 // the alignment of the vector, or 0 if the ABI default should be used.
-bool Scalarizer::getVectorLayout(Type *Ty, unsigned Alignment,
+bool ScalarizerVisitor::getVectorLayout(Type *Ty, unsigned Alignment,
                                  VectorLayout &Layout, const DataLayout &DL) {
   // Make sure we're dealing with a vector.
   Layout.VecTy = dyn_cast<VectorType>(Ty);
   if (!Layout.VecTy)
     return false;
 
   // Check that we're dealing with full-byte elements.
   Layout.ElemTy = Layout.VecTy->getElementType();
   if (DL.getTypeSizeInBits(Layout.ElemTy) !=
       DL.getTypeStoreSizeInBits(Layout.ElemTy))
     return false;
 
   if (Alignment)
     Layout.VecAlign = Alignment;
   else
     Layout.VecAlign = DL.getABITypeAlignment(Layout.VecTy);
   Layout.ElemSize = DL.getTypeStoreSize(Layout.ElemTy);
   return true;
 }
 
 // Scalarize two-operand instruction I, using Split(Builder, X, Y, Name)
 // to create an instruction like I with operands X and Y and name Name.
 template<typename Splitter>
-bool Scalarizer::splitBinary(Instruction &I, const Splitter &Split) {
+bool ScalarizerVisitor::splitBinary(Instruction &I, const Splitter &Split) {
   VectorType *VT = dyn_cast<VectorType>(I.getType());
   if (!VT)
     return false;
 
   unsigned NumElems = VT->getNumElements();
   IRBuilder<> Builder(&I);
   Scatterer Op0 = scatter(&I, I.getOperand(0));
   Scatterer Op1 = scatter(&I, I.getOperand(1));

 static Function *getScalarIntrinsicDeclaration(Module *M,
                                                Intrinsic::ID ID,
                                                VectorType *Ty) {
   return Intrinsic::getDeclaration(M, ID, { Ty->getScalarType() });
 }
 
 /// If a call to a vector typed intrinsic function, split into a scalar call per
 /// element if possible for the intrinsic.
-bool Scalarizer::splitCall(CallInst &CI) {
+bool ScalarizerVisitor::splitCall(CallInst &CI) {
   VectorType *VT = dyn_cast<VectorType>(CI.getType());
   if (!VT)
     return false;
 
   Function *F = CI.getCalledFunction();
   if (!F)
     return false;
 
for (unsigned Elem = 0; Elem < NumElems; ++Elem) {
     Res[Elem] = Builder.CreateCall(NewIntrin, ScalarCallOps,
                                    CI.getName() + ".i" + Twine(Elem));
   }
 
   gather(&CI, Res);
   return true;
 }
 
-bool Scalarizer::visitSelectInst(SelectInst &SI) {
+bool ScalarizerVisitor::visitSelectInst(SelectInst &SI) {
   VectorType *VT = dyn_cast<VectorType>(SI.getType());
   if (!VT)
     return false;
 
   unsigned NumElems = VT->getNumElements();
   IRBuilder<> Builder(&SI);
   Scatterer Op1 = scatter(&SI, SI.getOperand(1));
   Scatterer Op2 = scatter(&SI, SI.getOperand(2));
if (SI.getOperand(0)->getType()->isVectorTy()) {
     for (unsigned I = 0; I < NumElems; ++I)
       Res[I] = Builder.CreateSelect(Op0, Op1[I], Op2[I],
                                     SI.getName() + ".i" + Twine(I));
   }
   gather(&SI, Res);
   return true;
 }
 
-bool Scalarizer::visitICmpInst(ICmpInst &ICI) {
+bool ScalarizerVisitor::visitICmpInst(ICmpInst &ICI) {
   return splitBinary(ICI, ICmpSplitter(ICI));
 }
 
-bool Scalarizer::visitFCmpInst(FCmpInst &FCI) {
+bool ScalarizerVisitor::visitFCmpInst(FCmpInst &FCI) {
   return splitBinary(FCI, FCmpSplitter(FCI));
 }
 
-bool Scalarizer::visitBinaryOperator(BinaryOperator &BO) {
+bool ScalarizerVisitor::visitBinaryOperator(BinaryOperator &BO) {
   return splitBinary(BO, BinarySplitter(BO));
 }
 
-bool Scalarizer::visitGetElementPtrInst(GetElementPtrInst &GEPI) {
+bool ScalarizerVisitor::visitGetElementPtrInst(GetElementPtrInst &GEPI) {
   VectorType *VT = dyn_cast<VectorType>(GEPI.getType());
   if (!VT)
     return false;
 
   IRBuilder<> Builder(&GEPI);
   unsigned NumElems = VT->getNumElements();
   unsigned NumIndices = GEPI.getNumIndices();
 
for (unsigned I = 0; I < NumElems; ++I) {
     if (GEPI.isInBounds())
       if (GetElementPtrInst *NewGEPI = dyn_cast<GetElementPtrInst>(Res[I]))
         NewGEPI->setIsInBounds();
   }
   gather(&GEPI, Res);
   return true;
 }
 
-bool Scalarizer::visitCastInst(CastInst &CI) {
+bool ScalarizerVisitor::visitCastInst(CastInst &CI) {
   VectorType *VT = dyn_cast<VectorType>(CI.getDestTy());
   if (!VT)
     return false;
 
   unsigned NumElems = VT->getNumElements();
   IRBuilder<> Builder(&CI);
   Scatterer Op0 = scatter(&CI, CI.getOperand(0));
   assert(Op0.size() == NumElems && "Mismatched cast");
   ValueVector Res;
   Res.resize(NumElems);
   for (unsigned I = 0; I < NumElems; ++I)
     Res[I] = Builder.CreateCast(CI.getOpcode(), Op0[I], VT->getElementType(),
                                 CI.getName() + ".i" + Twine(I));
   gather(&CI, Res);
   return true;
 }
 
-bool Scalarizer::visitBitCastInst(BitCastInst &BCI) {
+bool ScalarizerVisitor::visitBitCastInst(BitCastInst &BCI) {
   VectorType *DstVT = dyn_cast<VectorType>(BCI.getDestTy());
   VectorType *SrcVT = dyn_cast<VectorType>(BCI.getSrcTy());
   if (!DstVT || !SrcVT)
     return false;
 
   unsigned DstNumElems = DstVT->getNumElements();
   unsigned SrcNumElems = SrcVT->getNumElements();
   IRBuilder<> Builder(&BCI);
for (unsigned ResI = 0; ResI < DstNumElems; ++ResI) {
       Res[ResI] = Builder.CreateBitCast(V, DstVT->getElementType(),
                                         BCI.getName() + ".i" + Twine(ResI));
     }
   }
   gather(&BCI, Res);
   return true;
 }
 
-bool Scalarizer::visitShuffleVectorInst(ShuffleVectorInst &SVI) {
+bool ScalarizerVisitor::visitShuffleVectorInst(ShuffleVectorInst &SVI) {
   VectorType *VT = dyn_cast<VectorType>(SVI.getType());
   if (!VT)
     return false;
 
   unsigned NumElems = VT->getNumElements();
   Scatterer Op0 = scatter(&SVI, SVI.getOperand(0));
   Scatterer Op1 = scatter(&SVI, SVI.getOperand(1));
   ValueVector Res;
   Res.resize(NumElems);
 
   for (unsigned I = 0; I < NumElems; ++I) {
     int Selector = SVI.getMaskValue(I);
     if (Selector < 0)
       Res[I] = UndefValue::get(VT->getElementType());
     else if (unsigned(Selector) < Op0.size())
       Res[I] = Op0[Selector];
     else
       Res[I] = Op1[Selector - Op0.size()];
   }
   gather(&SVI, Res);
   return true;
 }
 
-bool Scalarizer::visitPHINode(PHINode &PHI) {
+bool ScalarizerVisitor::visitPHINode(PHINode &PHI) {
   VectorType *VT = dyn_cast<VectorType>(PHI.getType());
   if (!VT)
     return false;
 
   unsigned NumElems = VT->getNumElements();
   IRBuilder<> Builder(&PHI);
   ValueVector Res;
   Res.resize(NumElems);
 
   unsigned NumOps = PHI.getNumOperands();
   for (unsigned I = 0; I < NumElems; ++I)
     Res[I] = Builder.CreatePHI(VT->getElementType(), NumOps,
                                PHI.getName() + ".i" + Twine(I));
 
   for (unsigned I = 0; I < NumOps; ++I) {
     Scatterer Op = scatter(&PHI, PHI.getIncomingValue(I));
     BasicBlock *IncomingBlock = PHI.getIncomingBlock(I);
     for (unsigned J = 0; J < NumElems; ++J)
       cast<PHINode>(Res[J])->addIncoming(Op[J], IncomingBlock);
   }
   gather(&PHI, Res);
   return true;
 }
 
-bool Scalarizer::visitLoadInst(LoadInst &LI) {
+bool ScalarizerVisitor::visitLoadInst(LoadInst &LI) {
   if (!ScalarizeLoadStore)
     return false;
   if (!LI.isSimple())
     return false;
 
   VectorLayout Layout;
   if (!getVectorLayout(LI.getType(), LI.getAlignment(), Layout,
                        LI.getModule()->getDataLayout()))
     return false;
 
   unsigned NumElems = Layout.VecTy->getNumElements();
   IRBuilder<> Builder(&LI);
   Scatterer Ptr = scatter(&LI, LI.getPointerOperand());
   ValueVector Res;
   Res.resize(NumElems);
 
   for (unsigned I = 0; I < NumElems; ++I)
     Res[I] = Builder.CreateAlignedLoad(Ptr[I], Layout.getElemAlign(I),
                                        LI.getName() + ".i" + Twine(I));
   gather(&LI, Res);
   return true;
 }
 
-bool Scalarizer::visitStoreInst(StoreInst &SI) {
+bool ScalarizerVisitor::visitStoreInst(StoreInst &SI) {
   if (!ScalarizeLoadStore)
     return false;
   if (!SI.isSimple())
     return false;
 
   VectorLayout Layout;
   Value *FullValue = SI.getValueOperand();
   if (!getVectorLayout(FullValue->getType(), SI.getAlignment(), Layout,
bool ScalarizerVisitor::visitStoreInst(StoreInst &SI) {
   for (unsigned I = 0; I < NumElems; ++I) {
     unsigned Align = Layout.getElemAlign(I);
     Stores[I] = Builder.CreateAlignedStore(Val[I], Ptr[I], Align);
   }
   transferMetadata(&SI, Stores);
   return true;
 }
 
-bool Scalarizer::visitCallInst(CallInst &CI) {
+bool ScalarizerVisitor::visitCallInst(CallInst &CI) {
   return splitCall(CI);
 }
 
 // Delete the instructions that we scalarized.  If a full vector result
 // is still needed, recreate it using InsertElements.
-bool Scalarizer::finish() {
+bool ScalarizerVisitor::finish() {
   // The presence of data in Gathered or Scattered indicates changes
   // made to the Function.
   if (Gathered.empty() && Scattered.empty())
     return false;
   for (const auto &GMI : Gathered) {
     Instruction *Op = GMI.first;
     ValueVector &CV = *GMI.second;
     if (!Op->use_empty()) {
for (const auto &GMI : Gathered) {
     }
     Op->eraseFromParent();
   }
   Gathered.clear();
   Scattered.clear();
   return true;
 }
 
-FunctionPass *llvm::createScalarizerPass() {
-  return new Scalarizer();
+PreservedAnalyses ScalarizerPass::run(Function &F, FunctionAnalysisManager &AM) {
+  Module &M = *F.getParent();
+  unsigned ParallelLoopAccessMDKind =
+      M.getContext().getMDKindID("llvm.mem.parallel_loop_access");
+  ScalarizerVisitor Impl(ParallelLoopAccessMDKind);
+  (void)Impl.visit(F);
+  return PreservedAnalyses::none();
 }

llvm/trunk/test/Transforms/Scalarizer/basic.ll

 ; RUN: opt %s -scalarizer -scalarize-load-store -dce -S | FileCheck %s
+; RUN: opt %s -passes='function(scalarizer,dce)' -scalarize-load-store -S | FileCheck %s
 target datalayout = "e-p:64:64:64-i1:8:8-i8:8:8-i16:16:16-i32:32:32-i64:64:64-f32:32:32-f64:64:64-v64:64:64-v128:128:128-a0:0:64-s0:64:64-f80:128:128-n8:16:32:64-S128"
 
 declare <4 x float> @ext(<4 x float>)
 @g = global <4 x float> zeroinitializer
 
 define void @f1(<4 x float> %init, <4 x float> *%base, i32 %count) {
 ; CHECK-LABEL: @f1(
 ; CHECK: entry:

llvm/trunk/test/Transforms/Scalarizer/cache-bug.ll

 ; RUN: opt -scalarizer -S < %s | FileCheck %s
+; RUN: opt -passes='function(scalarizer)' -S < %s | FileCheck %s
 target datalayout = "e-p:64:64:64-i1:8:8-i8:8:8-i16:16:16-i32:32:32-i64:64:64-f32:32:32-f64:64:64-v64:64:64-v128:128:128-a0:0:64-s0:64:64-f80:128:128-n8:16:32:64-S128"
 
 
 ; Check that vector element 1 is scalarized correctly from a chain of
 ; insertelement instructions
 define void @func(i32 %x) {
 ; CHECK-LABEL: @func(
 ; CHECK-NOT: phi i32 [ %x, %entry ], [ %inc.pos.y, %loop ]

llvm/trunk/test/Transforms/Scalarizer/crash-bug.ll

 ; RUN: opt %s -scalarizer -S -o - | FileCheck %s
+; RUN: opt %s -passes='function(scalarizer)' -S -o - | FileCheck %s
 
 ; Don't crash
 
 define void @foo() {
   br label %bb1
 
 bb2:                                        ; preds = %bb1
   %bb2_vec = shufflevector <2 x i16> <i16 0, i16 10000>,

llvm/trunk/test/Transforms/Scalarizer/dbginfo.ll

 ; RUN: opt %s -scalarizer -scalarize-load-store -S | FileCheck %s
+; RUN: opt %s -passes='function(scalarizer)' -scalarize-load-store -S | FileCheck %s
 target datalayout = "e-p:64:64:64-i1:8:8-i8:8:8-i16:16:16-i32:32:32-i64:64:64-f32:32:32-f64:64:64-v64:64:64-v128:128:128-a0:0:64-s0:64:64-f80:128:128-n8:16:32:64-S128"
 
 ; Function Attrs: nounwind uwtable
 define void @f1(<4 x i32>* nocapture %a, <4 x i32>* nocapture readonly %b, <4 x i32>* nocapture readonly %c) #0 !dbg !4 {
 ; CHECK: @f1(
 ; CHECK: %a.i0 = bitcast <4 x i32>* %a to i32*
 ; CHECK: %a.i1 = getelementptr i32, i32* %a.i0, i32 1
 ; CHECK: %a.i2 = getelementptr i32, i32* %a.i0, i32 2

llvm/trunk/test/Transforms/Scalarizer/dbgloc-bug.ll

 ; RUN: opt -S -march=x86 -scalarizer %s | FileCheck %s
+; RUN: opt -S -march=x86 -passes='function(scalarizer)' %s | FileCheck %s
 
 ; Reproducer for pr27938
 ; https://llvm.org/bugs/show_bug.cgi?id=27938
 
 define i16 @f1() !dbg !5 {
   ret i16 undef, !dbg !9
 }
 

llvm/trunk/test/Transforms/Scalarizer/intrinsics.ll

 ; RUN: opt -S -scalarizer %s | FileCheck %s
+; RUN: opt -S -passes='function(scalarizer)' %s | FileCheck %s
 
 ; Unary fp
 declare <2 x float> @llvm.sqrt.v2f32(<2 x float>)
 
 ; Binary fp
 declare <2 x float> @llvm.minnum.v2f32(<2 x float>, <2 x float>)
 declare <2 x float> @llvm.minimum.v2f32(<2 x float>, <2 x float>)
 declare <2 x float> @llvm.maximum.v2f32(<2 x float>, <2 x float>)

llvm/trunk/test/Transforms/Scalarizer/order-bug.ll

 ; RUN: opt %s -scalarizer -S -o - | FileCheck %s
+; RUN: opt %s -passes='function(scalarizer)' -S -o - | FileCheck %s
 
 ; This input caused the scalarizer to replace & erase gathered results when 
 ; future gathered results depended on them being alive
 
 define dllexport spir_func <4 x i32> @main(float %a) {
 entry:
   %i = insertelement <4 x float> undef, float %a, i32 0
   br label %z

llvm/trunk/test/Transforms/Scalarizer/phi-bug.ll

 ; RUN: opt %s -scalarizer -verify -S -o - | FileCheck %s
+; RUN: opt %s -passes='function(scalarizer,verify)' -S -o - | FileCheck %s
 
 define void @f3() local_unnamed_addr {
 bb1:
   br label %bb2
 
 bb3:
 ; CHECK-LABEL: bb3:
 ; CHECK-NEXT: br label %bb4

llvm/trunk/test/Transforms/Scalarizer/store-bug.ll

 ; RUN: opt -scalarizer -scalarize-load-store -S < %s | FileCheck %s
+; RUN: opt -passes='function(scalarizer)' -scalarize-load-store -S < %s | FileCheck %s
 target datalayout = "e-p:64:64:64-i1:8:8-i8:8:8-i16:16:16-i32:32:32-i64:64:64-f32:32:32-f64:64:64-v64:64:64-v128:128:128-a0:0:64-s0:64:64-f80:128:128-n8:16:32:64-S128"
 
 ; This input caused the scalarizer not to clear cached results
 ; properly.
 ;
 ; Any regressions should trigger an assert in the scalarizer.
 
 define void @func(<4 x float> %val, <4 x float> *%ptr) {

llvm/trunk/test/Transforms/Scalarizer/vector-gep.ll

 ; RUN: opt -S -scalarizer %s | FileCheck %s
+; RUN: opt -S -passes='function(scalarizer)' %s | FileCheck %s
 
 ; Check that the scalarizer can handle vector GEPs with scalar indices
 
 @vec = global <4 x i16*> <i16* null, i16* null, i16* null, i16* null>
 @index = global i16 1
 @ptr = global [4 x i16] [i16 1, i16 2, i16 3, i16 4]
 @ptrptr = global i16* null