Expose ScalarizerPass options to C++ (not just commandline)

[Benoit]

Summary

Context: I needed this for https://github.com/google/iree/pull/8474 . I found that TSan instrumentation expects vector sizes to be <= 16, and in my project (IREE) we have tests with higher vector sizes. That left some test functions uninstrumented, resulting in crashes as instrumented code called into them.

[Benoit]

Context: I needed this for https://github.com/google/iree/pull/8474 . I found that TSan instrumentation expects vector sizes to be <= 16, and in my project (IREE) we have tests with higher vector sizes. That left some tests function uninstrumented, resulting in crashes as instrumented called into them.

[Benoit]

fixes

[Benoit]

clang-format

[bjope]

llvm/lib/Transforms/Scalar/Scalarizer.cpp

209

With the current patch you won't be able to override any pass options setup by the pass manager etc.
I would expect that the cmd line value has higher precedence.

[Benoit]

give the cl opts precedence. per review comment

[Benoit]

llvm/lib/Transforms/Scalar/Scalarizer.cpp

209

Thanks, that makes sense. Code updated.

[Benoit]

rebase

[bjope]

Just a few nits. But otherwise this LGTM.

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

38

nit: This pass use CamelCase for variable names.

43

nit: This pass use CamelCase for variable names.

46

nit: This pass use CamelCase for variable names.

[Benoit]

review comments

[Benoit]

I don't have write access. Can someone please push this for me?

[bjope]

In D121182#3372814, @Benoit wrote:

I don't have write access. Can someone please push this for me?

I've now landed this for you.

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

 /// 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/ADT/Optional.h"
 #include "llvm/IR/PassManager.h"
 
 namespace llvm {
 
 class Function;
 class FunctionPass;
 
+struct ScalarizerPassOptions {
+  // These optional booleans correspond 1:1 to cl::opt<bool> options defined in
+  // Scalarizer.cpp. When the cl::opt are specified, they take precedence.
+  // When the cl::opt are not specified, the present optional booleans allow to
+  // override the cl::opt's default values.
+  llvm::Optional<bool> ScalarizeVariableInsertExtract;
+  llvm::Optional<bool> ScalarizeLoadStore;
+};
+
 class ScalarizerPass : public PassInfoMixin<ScalarizerPass> {
+  ScalarizerPassOptions Options;
+
 public:
   PreservedAnalyses run(Function &F, FunctionAnalysisManager &AM);
+
+  void setScalarizeVariableInsertExtract(bool Value) {
+    Options.ScalarizeVariableInsertExtract = Value;
+  }
+  void setScalarizeLoadStore(bool Value) { Options.ScalarizeLoadStore = Value; }
 };
 
 /// Create a legacy pass manager instance of the Scalarizer pass
 FunctionPass *createScalarizerPass();
 
 }
 
 #endif /* LLVM_TRANSFORMS_SCALAR_SCALARIZER_H */

llvm/lib/Transforms/Scalar/Scalarizer.cpp

 #include <iterator>
 #include <map>
 #include <utility>
 
 using namespace llvm;
 
 #define DEBUG_TYPE "scalarizer"
 
-static cl::opt<bool> ScalarizeVariableInsertExtract(
+static cl::opt<bool> ClScalarizeVariableInsertExtract(
     "scalarize-variable-insert-extract", cl::init(true), cl::Hidden,
     cl::desc("Allow the scalarizer pass to scalarize "
              "insertelement/extractelement with variable index"));
 
 // 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"));
+static cl::opt<bool> ClScalarizeLoadStore(
+    "scalarize-load-store", cl::init(false), cl::Hidden,
 
 namespace {
 
 BasicBlock::iterator skipPastPhiNodesAndDbg(BasicBlock::iterator Itr) {
   BasicBlock *BB = Itr->getParent();
   if (isa<PHINode>(Itr))
     Itr = BB->getFirstInsertionPt();
   if (Itr != BB->end())
struct VectorLayout {
 
   // The alignment of the vector.
   Align VecAlign;
 
   // The size of each element.
   uint64_t ElemSize = 0;
 };
 
+template <typename T>
+T getWithDefaultOverride(const cl::opt<T> &ClOption,
+                         const llvm::Optional<T> &DefaultOverride) {
+  return ClOption.getNumOccurrences() ? ClOption
+                                      : DefaultOverride.getValueOr(ClOption);
+}
+
 class ScalarizerVisitor : public InstVisitor<ScalarizerVisitor, bool> {
 public:
-  ScalarizerVisitor(unsigned ParallelLoopAccessMDKind, DominatorTree *DT)
-    : ParallelLoopAccessMDKind(ParallelLoopAccessMDKind), DT(DT) {
+  ScalarizerVisitor(unsigned ParallelLoopAccessMDKind, DominatorTree *DT,
+                    ScalarizerPassOptions Options)
+      : ParallelLoopAccessMDKind(ParallelLoopAccessMDKind), DT(DT),
+        ScalarizeVariableInsertExtract(
+            getWithDefaultOverride(ClScalarizeVariableInsertExtract,
+                                   Options.ScalarizeVariableInsertExtract)),
+        ScalarizeLoadStore(getWithDefaultOverride(ClScalarizeLoadStore,
+                                                  Options.ScalarizeLoadStore)) {
   }
 
   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 visitUnaryOperator(UnaryOperator &UO);
   bool visitBinaryOperator(BinaryOperator &BO);
private:
   ScatterMap Scattered;
   GatherList Gathered;
 
   SmallVector<WeakTrackingVH, 32> PotentiallyDeadInstrs;
 
   unsigned ParallelLoopAccessMDKind;
 
   DominatorTree *DT;
+
+  const bool ScalarizeVariableInsertExtract;
+  const bool ScalarizeLoadStore;
 };
 
 class ScalarizerLegacyPass : public FunctionPass {
 public:
   static char ID;
 
   ScalarizerLegacyPass() : FunctionPass(ID) {
     initializeScalarizerLegacyPassPass(*PassRegistry::getPassRegistry());

 bool ScalarizerLegacyPass::runOnFunction(Function &F) {
   if (skipFunction(F))
     return false;
 
   Module &M = *F.getParent();
   unsigned ParallelLoopAccessMDKind =
       M.getContext().getMDKindID("llvm.mem.parallel_loop_access");
   DominatorTree *DT = &getAnalysis<DominatorTreeWrapperPass>().getDomTree();
-  ScalarizerVisitor Impl(ParallelLoopAccessMDKind, DT);
+  ScalarizerVisitor Impl(ParallelLoopAccessMDKind, DT, ScalarizerPassOptions());
   return Impl.visit(F);
 }
 
 FunctionPass *llvm::createScalarizerPass() {
   return new ScalarizerLegacyPass();
 }
 
 bool ScalarizerVisitor::visit(Function &F) {
bool ScalarizerVisitor::finish() {
   return true;
 }
 
 PreservedAnalyses ScalarizerPass::run(Function &F, FunctionAnalysisManager &AM) {
   Module &M = *F.getParent();
   unsigned ParallelLoopAccessMDKind =
       M.getContext().getMDKindID("llvm.mem.parallel_loop_access");
   DominatorTree *DT = &AM.getResult<DominatorTreeAnalysis>(F);
-  ScalarizerVisitor Impl(ParallelLoopAccessMDKind, DT);
+  ScalarizerVisitor Impl(ParallelLoopAccessMDKind, DT, Options);
   bool Changed = Impl.visit(F);
   PreservedAnalyses PA;
   PA.preserve<DominatorTreeAnalysis>();
   return Changed ? PA : PreservedAnalyses::all();
 }