Revert "[mlir][Linalg] NFC - Reorganize options nesting."

This reverts commit 4703a07.

Didnt' mean to push this yet, sorry about the noise.

Author: nicolasvasilache

mlir/include/mlir/Conversion/VectorToSCF/VectorToSCF.h

@@ -46,29 +46,29 @@ class RewritePatternSet;
 ///
 /// When applying the pattern a second time, the existing alloca() operation
 /// is reused and only a second vector.type_cast is added.
+
 struct VectorTransferToSCFOptions {
-  /// Minimal rank to which vector transfer are lowered.
   unsigned targetRank = 1;
-  VectorTransferToSCFOptions &setTargetRank(unsigned r) {
-    targetRank = r;
-    return *this;
-  }
-  ///
   bool lowerPermutationMaps = false;
-  VectorTransferToSCFOptions &enableLowerPermutationMaps(bool l = true) {
+  bool lowerTensors = false;
+  bool unroll = false;
+
+  VectorTransferToSCFOptions &setLowerPermutationMaps(bool l) {
     lowerPermutationMaps = l;
     return *this;
   }
-  /// Allows vector transfers that operated on tensors to be lowered (this is an
-  /// uncommon alternative).
-  bool lowerTensors = false;
-  VectorTransferToSCFOptions &enableLowerTensors(bool l = true) {
+
+  VectorTransferToSCFOptions &setLowerTensors(bool l) {
     lowerTensors = l;
     return *this;
   }
-  /// Triggers full unrolling (vs iterating with a loop) during transfer to scf.
-  bool unroll = false;
-  VectorTransferToSCFOptions &enableFullUnroll(bool u = true) {
+
+  VectorTransferToSCFOptions &setTargetRank(unsigned r) {
+    targetRank = r;
+    return *this;
+  }
+
+  VectorTransferToSCFOptions &setUnroll(bool u) {
     unroll = u;
     return *this;
   }

mlir/include/mlir/Dialect/Linalg/Transforms/CodegenStrategy.h

@@ -195,16 +195,53 @@ struct CodegenStrategy {
     return b ? vectorize(opName, f) : *this;
     return *this;
   }
-  /// Configure the post staged-patterns late vector lowering options.
+  /// Configure the post staged-patterns late vector transformations.
   CodegenStrategy &
-  setLinalgVectorLoweringOptions(LinalgVectorLoweringOptions options) {
-    lateVectorLoweringOptions = options;
+  setVectorTransformsOptions(vector::VectorTransformsOptions options) {
+    vectorTransformOptions = options;
     return *this;
   }
-  /// Configure the post staged-patterns global enabling passes options.
+  /// Configure the post staged-patterns late vector.transfer to scf
+  /// conversion.
   CodegenStrategy &
-  setVectorTransferToSCFOptions(LinalgEnablingOptions options) {
-    linalgEnablingOptions = options;
+  setVectorTransferToSCFOptions(VectorTransferToSCFOptions options) {
+    vectorToSCFOptions = options;
+    return *this;
+  }
+  ///
+  /// Configure the application of late transformations.
+  ///
+  CodegenStrategy &setEnableLICM(bool val) {
+    this->lateCodegenStrategyOptions.enableLICM = val;
+    return *this;
+  }
+  CodegenStrategy &setEnableHoistRedundantVectorTransfers(bool val) {
+    this->lateCodegenStrategyOptions.enableHoistRedundantVectorTransfers = val;
+    return *this;
+  }
+  CodegenStrategy &setEnableHoistRedundantVectorTransfersOnTensor(bool val) {
+    this->lateCodegenStrategyOptions
+        .enableHoistRedundantVectorTransfersOnTensor = val;
+    return *this;
+  }
+  CodegenStrategy &setMaxTransferRank(int64_t val) {
+    this->lateCodegenStrategyOptions.maxTransferRank = val;
+    return *this;
+  }
+  CodegenStrategy &setEnableVectorTransferLowering(bool val) {
+    this->lateCodegenStrategyOptions.enableVectorTransferLowering = val;
+    return *this;
+  }
+  CodegenStrategy &setEnableVectorTransferPartialRewrite(bool val) {
+    this->lateCodegenStrategyOptions.enableVectorTransferPartialRewrite = val;
+    return *this;
+  }
+  CodegenStrategy &setEnableVectorContractLowering(bool val) {
+    this->lateCodegenStrategyOptions.enableVectorContractLowering = val;
+    return *this;
+  }
+  CodegenStrategy &setEnableVectorToSCFConversion(bool val) {
+    this->lateCodegenStrategyOptions.enableVectorToSCFConversion = val;
     return *this;
   }
 
@@ -215,9 +252,10 @@ struct CodegenStrategy {
 private:
   LogicalResult postPatternTransforms(Operation *func) const;
 
-  LinalgEnablingOptions linalgEnablingOptions;
-  LinalgVectorLoweringOptions lateVectorLoweringOptions;
+  vector::VectorTransformsOptions vectorTransformOptions;
+  VectorTransferToSCFOptions vectorToSCFOptions;
   SmallVector<std::unique_ptr<Transformation>, 4> transformationSequence;
+  LateCodegenStrategyOptions lateCodegenStrategyOptions;
 };
 
 } // namespace linalg

mlir/include/mlir/Dialect/Linalg/Transforms/Transforms.h

@@ -846,80 +846,41 @@ struct LinalgVectorizationPattern : public LinalgBaseVectorizationPattern {
       : LinalgBaseVectorizationPattern(opName, context, filter, benefit) {}
 };
 
+/// Options to control the application of late transformations.
+struct LateCodegenStrategyOptions {
+  /// Hoisting transformations are always deemed beneficial and must disabled
+  /// explicitly.
+  bool enableLICM = true;
+  bool enableHoistRedundantVectorTransfers = true;
+  bool enableHoistRedundantVectorTransfersOnTensor = true;
+  /// Vector lowering operations may result in surprising behavior when
+  /// composing multiple codegen strategies and must be enabled explicitly.
+  int64_t maxTransferRank = 1;
+  bool enableVectorTransferLowering = true;
+  bool enableVectorTransferPartialRewrite = false;
+  bool enableVectorContractLowering = false;
+  bool enableVectorToSCFConversion = false;
+};
+
 /// Options to control the application of enabling transformations.
 /// Hoisting transformations are always deemed beneficial and must be disabled
 /// explicitly.
 struct LinalgEnablingOptions {
-  /// Enable LICM.
-  bool licm = true;
-  LinalgEnablingOptions &enableLICM(bool val = true) {
-    licm = val;
-    return *this;
-  }
-  /// Enable hoisting of redundant vector transfer ops.
-  bool hoistRedundantVectorTransfers = true;
-  LinalgEnablingOptions &enableHoistRedundantVectorTransfers(bool val = true) {
-    hoistRedundantVectorTransfers = val;
-    return *this;
-  }
-  /// Enable hoisting of redundant vector transfer ops on tensor.
-  bool hoistRedundantVectorTransfersOnTensor = true;
-  LinalgEnablingOptions &
-  enableHoistRedundantVectorTransfersOnTensor(bool val = true) {
-    hoistRedundantVectorTransfersOnTensor = val;
-    return *this;
-  }
+  bool enableLICM = true;
+  bool enableHoistRedundantVectorTransfers = true;
+  bool enableHoistRedundantVectorTransfersOnTensor = true;
 };
 
 /// Vector lowering options control how ops are lowered down to 1-D and scf.for
 /// form.
 struct LinalgVectorLoweringOptions {
-  /// Maximal transfer rank under which we do not lower further.
   int64_t maxTransferRank = 1;
-  LinalgVectorLoweringOptions &setMaxTransferRank(int64_t val) {
-    maxTransferRank = val;
-    return *this;
-  }
-  /// Vector lowering operations may result in surprising behavior when
-  /// composing multiple codegen strategies and must be enabled explicitly.
-  bool transferLowering = true;
-  LinalgVectorLoweringOptions &enableTransferLowering(bool val = true) {
-    transferLowering = val;
-    return *this;
-  }
-  /// Trigger full / partial vector.transfer splits.
-  bool transferPartialRewrite = false;
-  LinalgVectorLoweringOptions &enableTransferPartialRewrite(bool val = true) {
-    transferPartialRewrite = val;
-    return *this;
-  }
-  /// Enable lowering of vector.contract.
-  bool contractionLowering = false;
-  LinalgVectorLoweringOptions &enableContractionLowering(bool val = true) {
-    contractionLowering = val;
-    return *this;
-  }
-  /// Enable lowering of vector.transfer to scf.
-  bool transferToSCFConversion = false;
-  LinalgVectorLoweringOptions &enableTransferToSCFConversion(bool val = true) {
-    transferToSCFConversion = val;
-    return *this;
-  }
-  /// Configure late vector transformations.
+  bool enableVectorTransferLowering = true;
+  bool enableVectorTransferPartialRewrite = false;
+  bool enableVectorContractLowering = false;
+  bool enableVectorToSCFConversion = false;
   vector::VectorTransformsOptions vectorTransformOptions;
-  LinalgVectorLoweringOptions &
-  setVectorTransformsOptions(vector::VectorTransformsOptions options) {
-    vectorTransformOptions = options;
-    return *this;
-  }
-  /// Configure the post staged-patterns late vector.transfer to scf
-  /// conversion.
   VectorTransferToSCFOptions vectorTransferToSCFOptions;
-  LinalgVectorLoweringOptions &
-  setVectorTransferToSCFOptions(VectorTransferToSCFOptions options) {
-    vectorTransferToSCFOptions = options;
-    return *this;
-  }
 };
 
 /// Trait to check if T provides a `getOperationName` method.

mlir/lib/Dialect/Linalg/Transforms/CodegenStrategy.cpp

@@ -46,6 +46,19 @@ void mlir::linalg::CodegenStrategy::configurePassPipeline(
     t->addToPassPipeline(pm, filter);
     pm.addPass(createLinalgStrategyEnablePass());
   }
-  pm.addPass(createLinalgStrategyLowerVectorsPass(lateVectorLoweringOptions));
+  LinalgVectorLoweringOptions vectorLoweringOptions;
+  vectorLoweringOptions.maxTransferRank =
+      lateCodegenStrategyOptions.maxTransferRank;
+  vectorLoweringOptions.enableVectorTransferLowering =
+      lateCodegenStrategyOptions.enableVectorTransferLowering;
+  vectorLoweringOptions.enableVectorTransferPartialRewrite =
+      lateCodegenStrategyOptions.enableVectorTransferPartialRewrite;
+  vectorLoweringOptions.enableVectorContractLowering =
+      lateCodegenStrategyOptions.enableVectorContractLowering;
+  vectorLoweringOptions.enableVectorToSCFConversion =
+      lateCodegenStrategyOptions.enableVectorToSCFConversion;
+  vectorLoweringOptions.vectorTransformOptions = vectorTransformOptions;
+  vectorLoweringOptions.vectorTransferToSCFOptions = vectorToSCFOptions;
+  pm.addPass(createLinalgStrategyLowerVectorsPass(vectorLoweringOptions));
   pm.addPass(createLinalgStrategyRemoveMarkersPass());
 }

mlir/lib/Dialect/Linalg/Transforms/LinalgStrategyPasses.cpp

@@ -224,7 +224,7 @@ struct LinalgStrategyEnablePass
     if (failed(applyPatternsAndFoldGreedily(funcOp, std::move(patterns))))
       return signalPassFailure();
 
-    if (options.licm) {
+    if (options.enableLICM) {
       if (funcOp
               ->walk([&](LoopLikeOpInterface loopLike) {
                 if (failed(moveLoopInvariantCode(loopLike)))
@@ -236,10 +236,10 @@ struct LinalgStrategyEnablePass
     }
 
     promoteSingleIterationLoops(funcOp);
-    if (options.hoistRedundantVectorTransfers)
+    if (options.enableHoistRedundantVectorTransfers)
       hoistRedundantVectorTransfers(funcOp);
 
-    if (options.hoistRedundantVectorTransfersOnTensor)
+    if (options.enableHoistRedundantVectorTransfersOnTensor)
       hoistRedundantVectorTransfersOnTensor(funcOp);
   }
 
@@ -263,21 +263,21 @@ struct LinalgStrategyLowerVectorsPass
 
     MLIRContext *context = funcOp.getContext();
     RewritePatternSet patterns(context);
-    if (options.transferLowering) {
+    if (options.enableVectorTransferLowering) {
       vector::populateVectorTransferLoweringPatterns(patterns,
                                                      options.maxTransferRank);
     }
-    if (options.transferPartialRewrite) {
+    if (options.enableVectorTransferPartialRewrite) {
       patterns.add<vector::VectorTransferFullPartialRewriter>(
           context, options.vectorTransformOptions);
     }
-    if (options.contractionLowering) {
+    if (options.enableVectorContractLowering) {
       patterns.add<ContractionOpToOuterProductOpLowering,
                    ContractionOpToMatmulOpLowering, ContractionOpLowering>(
           options.vectorTransformOptions, context);
       vector::populateVectorTransferPermutationMapLoweringPatterns(patterns);
     }
-    if (options.transferToSCFConversion) {
+    if (options.enableVectorToSCFConversion) {
       populateVectorToSCFConversionPatterns(patterns,
                                             options.vectorTransferToSCFOptions);
     }

mlir/test/lib/Dialect/Linalg/TestLinalgCodegenStrategy.cpp

@@ -153,18 +153,15 @@ void TestLinalgCodegenStrategy::runStrategy(
       .generalizeIf(generalize, anchorOpName)
       .interchangeIf(!iteratorInterchange.empty(), iteratorInterchange)
       .vectorizeIf(vectorize, generalize ? genericOpName : anchorOpName)
-      .setLinalgVectorLoweringOptions(
-          LinalgVectorLoweringOptions()
-              .setVectorTransformsOptions(
-                  vector::VectorTransformsOptions()
-                      .setVectorTransformsOptions(vectorContractLowering)
-                      .setVectorTransferSplit(vectorTransferSplit))
-              .setVectorTransferToSCFOptions(
-                  VectorTransferToSCFOptions().enableFullUnroll(
-                      unrollVectorTransfers))
-              .enableTransferPartialRewrite()
-              .enableContractionLowering()
-              .enableTransferToSCFConversion());
+      .setEnableVectorTransferPartialRewrite(true)
+      .setEnableVectorContractLowering(true)
+      .setEnableVectorToSCFConversion(true)
+      .setVectorTransformsOptions(
+          vector::VectorTransformsOptions()
+              .setVectorTransformsOptions(vectorContractLowering)
+              .setVectorTransferSplit(vectorTransferSplit))
+      .setVectorTransferToSCFOptions(
+          VectorTransferToSCFOptions().setUnroll(unrollVectorTransfers));
 
   // Created a nested OpPassManager and run.
   FuncOp funcOp = getFunction();