[mlir] Remove the default isDynamicallyLegal hook

[bkramer]

Summary

This is redundant with the callback variant and untested. Also remove
the callback-less methods for adding a dynamically legal op, as they
are no longer useful.

[rriddle]

Thanks for the cleanup!

mlir/docs/DialectConversion.md

*   Legal
     -   This action signals that every instance of a given operation is legal,
         i.e. any combination of attributes, operands, types, etc. are valid.
 
 *   Dynamic
 
     -   This action signals that only some instances of a given operation are
         legal. This allows for defining fine-tune constraints, e.g. saying that
         `addi` is only legal when operating on 32-bit integers.
-    -   If a specific handler is not provided when setting the action, the
-        target must override the `isDynamicallyLegal` hook provided by
-        `ConversionTarget`.
 
 *   Illegal
 
     -   This action signals that no instance of a given operation is legal.
         Operations marked as "illegal" must always be converted for the
         conversion to be successful. This action also allows for selectively
         marking specific operations as illegal in an otherwise legal dialect.
 
MyTarget(MLIRContext &ctx) : ConversionTarget(ctx) {
     /// Mark `std.constant` op is always legal on this target.
     addLegalOp<ConstantOp>();
 
     //--------------------------------------------------------------------------
     // Marking an operation as dynamically legal.
 
     /// Mark all operations within Affine dialect have dynamic legality
     /// constraints.
-    addDynamicallyLegalDialect<AffineDialect>();
-
-    /// Mark `std.return` as dynamically legal.
-    addDynamicallyLegalOp<ReturnOp>();
+    addDynamicallyLegalDialect<AffineDialect>([](Operation *op) { ... });
 
     /// Mark `std.return` as dynamically legal, but provide a specific legality
     /// callback.
     addDynamicallyLegalOp<ReturnOp>([](ReturnOp op) { ... });
 
     /// Treat unknown operations, i.e. those without a legalization action
     /// directly set, as dynamically legal.
-    markUnknownOpDynamicallyLegal();
     markUnknownOpDynamicallyLegal([](Operation *op) { ... });
 
     //--------------------------------------------------------------------------
     // Marking an operation as illegal.
 
     /// All operations within the GPU dialect are illegal.
     addIllegalDialect<GPUDialect>();
 

mlir/include/mlir/Transforms/DialectConversion.h

struct LegalOpDetails {
     bool isRecursivelyLegal = false;
   };
 
   /// The signature of the callback used to determine if an operation is
   /// dynamically legal on the target.
   using DynamicLegalityCallbackFn = std::function<bool(Operation *)>;
 
   ConversionTarget(MLIRContext &ctx) : ctx(ctx) {}
-  virtual ~ConversionTarget() = default;
 
   //===--------------------------------------------------------------------===//
   // Legality Registration
   //===--------------------------------------------------------------------===//
 
   /// Register a legality action for the given operation.
   void setOpAction(OperationName op, LegalizationAction action);
   template <typename OpT>
   void setOpAction(LegalizationAction action) {
     setOpAction(OperationName(OpT::getOperationName(), &ctx), action);
   }
 
   /// Register the given operations as legal.
   template <typename OpT>
   void addLegalOp() {
     setOpAction<OpT>(LegalizationAction::Legal);
   }
   template <typename OpT, typename OpT2, typename... OpTs>
   void addLegalOp() {
     addLegalOp<OpT>();
     addLegalOp<OpT2, OpTs...>();
   }
 
-  /// Register the given operation as dynamically legal, i.e. requiring custom
-  /// handling by the target via 'isDynamicallyLegal'.
-  template <typename OpT>
-  void addDynamicallyLegalOp() {
-    setOpAction<OpT>(LegalizationAction::Dynamic);
-  }
-  template <typename OpT, typename OpT2, typename... OpTs>
-  void addDynamicallyLegalOp() {
-    addDynamicallyLegalOp<OpT>();
-    addDynamicallyLegalOp<OpT2, OpTs...>();
-  }
-
   /// Register the given operation as dynamically legal and set the dynamic
   /// legalization callback to the one provided.
   template <typename OpT>
   void addDynamicallyLegalOp(const DynamicLegalityCallbackFn &callback) {
     OperationName opName(OpT::getOperationName(), &ctx);
     setOpAction(opName, LegalizationAction::Dynamic);
     setLegalityCallback(opName, callback);
   }
public:
   }
   template <typename... Args>
   void addLegalDialect() {
     SmallVector<StringRef, 2> dialectNames({Args::getDialectNamespace()...});
     setDialectAction(dialectNames, LegalizationAction::Legal);
   }
 
   /// Register the operations of the given dialects as dynamically legal, i.e.
-  /// requiring custom handling by the target via 'isDynamicallyLegal'.
+  /// requiring custom handling by the callback.
   template <typename... Names>
-  void addDynamicallyLegalDialect(StringRef name, Names... names) {
+  void addDynamicallyLegalDialect(DynamicLegalityCallbackFn callback,
+                                  StringRef name, Names... names) {
     SmallVector<StringRef, 2> dialectNames({name, names...});
     setDialectAction(dialectNames, LegalizationAction::Dynamic);
+    setLegalityCallback(dialectNames, std::move(callback));
   }
   template <typename... Args>
-  void addDynamicallyLegalDialect(DynamicLegalityCallbackFn callback = {}) {
-    SmallVector<StringRef, 2> dialectNames({Args::getDialectNamespace()...});
-    setDialectAction(dialectNames, LegalizationAction::Dynamic);
-    if (callback)
-      setLegalityCallback(dialectNames, callback);
+  void addDynamicallyLegalDialect(DynamicLegalityCallbackFn callback) {
+    addDynamicallyLegalDialect(std::move(callback),
+                               Args::getDialectNamespace()...);
   }
 
   /// Register unknown operations as dynamically legal. For operations(and
   /// dialects) that do not have a set legalization action, treat them as
-  /// dynamically legal and invoke the given callback if valid or
-  /// 'isDynamicallyLegal'.
+  /// dynamically legal and invoke the given callback.
   void markUnknownOpDynamicallyLegal(const DynamicLegalityCallbackFn &fn) {
     setLegalityCallback(fn);
   }
-  void markUnknownOpDynamicallyLegal() {
-    setLegalityCallback(
-        [this](Operation *op) { return isDynamicallyLegal(op); });
-  }
 
   /// Register the operations of the given dialects as illegal, i.e.
   /// operations of this dialect are not supported by the target.
   template <typename... Names>
   void addIllegalDialect(StringRef name, Names... names) {
     SmallVector<StringRef, 2> dialectNames({name, names...});
     setDialectAction(dialectNames, LegalizationAction::Illegal);
   }
public:
   /// Get the legality action for the given operation.
   Optional<LegalizationAction> getOpAction(OperationName op) const;
 
   /// If the given operation instance is legal on this target, a structure
   /// containing legality information is returned. If the operation is not
   /// legal, None is returned.
   Optional<LegalOpDetails> isLegal(Operation *op) const;
 
-protected:
-  /// Runs a custom legalization query for the given operation. This should
-  /// return true if the given operation is legal, otherwise false.
-  virtual bool isDynamicallyLegal(Operation *op) const {
-    llvm_unreachable(
-        "targets with custom legalization must override 'isDynamicallyLegal'");
-  }
-
 private:
   /// Set the dynamic legality callback for the given operation.
   void setLegalityCallback(OperationName name,
                            const DynamicLegalityCallbackFn &callback);
 
   /// Set the dynamic legality callback for the given dialects.
   void setLegalityCallback(ArrayRef<StringRef> dialects,
                            const DynamicLegalityCallbackFn &callback);

mlir/lib/Transforms/Utils/DialectConversion.cpp

 auto ConversionTarget::isLegal(Operation *op) const
     -> Optional<LegalOpDetails> {
   Optional<LegalizationInfo> info = getOpInfo(op->getName());
   if (!info)
     return llvm::None;
 
   // Returns true if this operation instance is known to be legal.
   auto isOpLegal = [&] {
-    // Handle dynamic legality either with the provided legality function, or
-    // the default hook on the derived instance.
+    // Handle dynamic legality either with the provided legality function.
     if (info->action == LegalizationAction::Dynamic)
-      return info->legalityFn ? info->legalityFn(op) : isDynamicallyLegal(op);
+      return info->legalityFn(op);
 
     // Otherwise, the operation is only legal if it was marked 'Legal'.
     return info->action == LegalizationAction::Legal;
   };
   if (!isOpLegal())
     return llvm::None;
 
   // This operation is legal, compute any additional legality information.