[mlir:Standard] Remove support for creating a `unit` ConstantOp

[rriddle]

Summary

This is completely unused upstream, and does not really have well defined semantics
on what this is supposed to do/how this fits into the ecosystem. Given that, as part of
splitting up the standard dialect it's best to just remove this behavior, instead of try
to awkwardly fit it somewhere upstream. Downstream users are encouraged to
define their own operations that clearly can define the semantics of this.

This also uncovered several lingering uses of ConstantOp that weren't
updated to use arith::ConstantOp, and worked during conversions because
the constant was removed/converted into something else before
verification.

See https://llvm.discourse.group/t/standard-dialect-the-final-chapter/ for more discussion.

[bondhugula]

mlir/test/Dialect/Standard/invalid.mlir

4

This error message can be improved.

expected symbol ref attribute

?

[lattner]

Narrowing this seems great to me.

[rriddle]

mlir/test/Dialect/Standard/invalid.mlir

4

Agreed. Looks like this is coming from the parser, let me look at improving this in a followup (I see a few other cases in-tree where this is firing).

flang/lib/Optimizer/Builder/Character.cpp

 /// values will have a type `!fir.array<...x!fir.char<N>>` or `!fir.char<N>`.
 LLVM_ATTRIBUTE_UNUSED static bool needToMaterialize(mlir::Value str) {
   return str.getType().isa<fir::SequenceType>() || fir::isa_char(str.getType());
 }
 
 /// Unwrap integer constant from mlir::Value.
 static llvm::Optional<std::int64_t> getIntIfConstant(mlir::Value value) {
   if (auto *definingOp = value.getDefiningOp())
-    if (auto cst = mlir::dyn_cast<mlir::ConstantOp>(definingOp))
+    if (auto cst = mlir::dyn_cast<mlir::arith::ConstantOp>(definingOp))
       if (auto intAttr = cst.getValue().dyn_cast<mlir::IntegerAttr>())
         return intAttr.getInt();
   return {};
 }
 
 /// This is called only if `str` does not reside in memory. Such a bare string
 /// value will be converted into a memory-based temporary and an extended
 /// boxchar value returned.

mlir/include/mlir/Dialect/StandardOps/IR/Ops.td

def ConstantOp : Std_Op<"constant",
   let summary = "constant";
   let description = [{
     Syntax:
 
     ```
     operation ::= ssa-id `=` `std.constant` attribute-value `:` type
     ```
 
-    The `constant` operation produces an SSA value equal to some constant
-    specified by an attribute. This is the way that MLIR uses to form simple
-    integer and floating point constants, as well as more exotic things like
-    references to functions and tensor/vector constants.
+    The `constant` operation produces an SSA value from a symbol reference to a
+    `builtin.func` operation
 
     Example:
 
     ```mlir
-    // Complex constant
-    %1 = constant [1.0 : f32, 1.0 : f32] : complex<f32>
-
     // Reference to function @myfn.
     %2 = constant @myfn : (tensor<16xf32>, f32) -> tensor<16xf32>
 
     // Equivalent generic forms
-    %1 = "std.constant"() {value = [1.0 : f32, 1.0 : f32] : complex<f32>}
-       : () -> complex<f32>
     %2 = "std.constant"() {value = @myfn}
        : () -> ((tensor<16xf32>, f32) -> tensor<16xf32>)
     ```
 
     MLIR does not allow direct references to functions in SSA operands because
     the compiler is multithreaded, and disallowing SSA values to directly
     reference a function simplifies this
     ([rationale](../Rationale/Rationale.md#multithreading-the-compiler)).
   }];
 
-  let arguments = (ins AnyAttr:$value);
+  let arguments = (ins FlatSymbolRefAttr:$value);
   let results = (outs AnyType);
-
-  let builders = [
-    OpBuilder<(ins "Attribute":$value),
-    [{ build($_builder, $_state, value.getType(), value); }]>,
-    OpBuilder<(ins "Attribute":$value, "Type":$type),
-    [{ build($_builder, $_state, type, value); }]>,
-  ];
+  let assemblyFormat = "attr-dict $value `:` type(results)";
 
   let extraClassDeclaration = [{
     /// Returns true if a constant operation can be built with the given value
     /// and result type.
     static bool isBuildableWith(Attribute value, Type type);
   }];
 
   let hasFolder = 1;

mlir/lib/Conversion/StandardToLLVM/StandardToLLVM.cpp

 };
 
 struct ConstantOpLowering : public ConvertOpToLLVMPattern<ConstantOp> {
   using ConvertOpToLLVMPattern<ConstantOp>::ConvertOpToLLVMPattern;
 
   LogicalResult
   matchAndRewrite(ConstantOp op, OpAdaptor adaptor,
                   ConversionPatternRewriter &rewriter) const override {
-    // If constant refers to a function, convert it to "addressof".
-    if (auto symbolRef = op.getValue().dyn_cast<FlatSymbolRefAttr>()) {
-      auto type = typeConverter->convertType(op.getResult().getType());
-      if (!type || !LLVM::isCompatibleType(type))
-        return rewriter.notifyMatchFailure(op, "failed to convert result type");
 
-      auto newOp = rewriter.create<LLVM::AddressOfOp>(op.getLoc(), type,
-                                                      symbolRef.getValue());
-      for (const NamedAttribute &attr : op->getAttrs()) {
-        if (attr.getName().strref() == "value")
-          continue;
-        newOp->setAttr(attr.getName(), attr.getValue());
-      }
-      rewriter.replaceOp(op, newOp->getResults());
-      return success();
-    }
-
-    // Calling into other scopes (non-flat reference) is not supported in LLVM.
-    if (op.getValue().isa<SymbolRefAttr>())
-      return rewriter.notifyMatchFailure(
-          op, "referring to a symbol outside of the current module");
-
-    return LLVM::detail::oneToOneRewrite(
-        op, LLVM::ConstantOp::getOperationName(), adaptor.getOperands(),
-        *getTypeConverter(), rewriter);
-  }
+    auto newOp =
+        rewriter.create<LLVM::AddressOfOp>(op.getLoc(), type, op.getValue());
 };
 
 // A CallOp automatically promotes MemRefType to a sequence of alloca/store and
 // passes the pointer to the MemRef across function boundaries.
 template <typename CallOpType>
 struct CallOpInterfaceLowering : public ConvertOpToLLVMPattern<CallOpType> {
   using ConvertOpToLLVMPattern<CallOpType>::ConvertOpToLLVMPattern;
   using Super = CallOpInterfaceLowering<CallOpType>;

mlir/lib/Dialect/Async/Transforms/AsyncParallelFor.cpp

static ParallelComputeFunction createParallelComputeFunction(
   Value c0 = b.create<arith::ConstantIndexOp>(0);
   Value c1 = b.create<arith::ConstantIndexOp>(1);
 
   // Materialize known constants as constant operation in the function body.
   auto values = [&](ArrayRef<BlockArgument> args, ArrayRef<IntegerAttr> attrs) {
     return llvm::to_vector(
         llvm::map_range(llvm::zip(args, attrs), [&](auto tuple) -> Value {
           if (IntegerAttr attr = std::get<1>(tuple))
-            return b.create<ConstantOp>(attr);
+            return b.create<arith::ConstantOp>(attr);
           return std::get<0>(tuple);
         }));
   };
 
   // Multi-dimensional parallel iteration space defined by the loop trip counts.
   auto tripCounts = values(args.tripCounts(), bounds.tripCounts);
 
   // Parallel operation lower bound and step.

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

if (isFloat) {
         intOutputValues[dstLinearIndex] = *computeFn(computeFnInputs).apInt;
       }
     }
 
     DenseElementsAttr outputAttr =
         isFloat ? DenseElementsAttr::get(outputType, fpOutputValues)
                 : DenseElementsAttr::get(outputType, intOutputValues);
 
-    rewriter.replaceOpWithNewOp<ConstantOp>(genericOp, outputAttr);
+    rewriter.replaceOpWithNewOp<arith::ConstantOp>(genericOp, outputAttr);
     return success();
   }
 
 private:
   ControlElementwiseOpsFusionFn controlFn;
 };
 
 // Folds linalg.generic ops that are actually transposes on constant values.

mlir/lib/Dialect/Math/Transforms/PolynomialApproximation.cpp

for (const auto &tuple : llvm::enumerate(expandedOperands))
           builder.create<vector::ExtractOp>(tuple.value(), offsets);
 
     results[i] = compute(extracted);
   }
 
   // Stitch results together into one large vector.
   Type resultEltType = results[0].getType().cast<VectorType>().getElementType();
   Type resultExpandedType = VectorType::get(expandedShape, resultEltType);
-  Value result = builder.create<ConstantOp>(
+  Value result = builder.create<arith::ConstantOp>(
       resultExpandedType, builder.getZeroAttr(resultExpandedType));
 
   for (int64_t i = 0; i < maxLinearIndex; ++i)
     result = builder.create<vector::InsertOp>(results[i], result,
                                               delinearize(strides, i));
 
   // Reshape back to the original vector shape.
   return builder.create<vector::ShapeCastOp>(

mlir/lib/Dialect/StandardOps/IR/Ops.cpp

 
 /// Materialize a single constant operation from a given attribute value with
 /// the desired resultant type.
 Operation *StandardOpsDialect::materializeConstant(OpBuilder &builder,
                                                    Attribute value, Type type,
                                                    Location loc) {
   if (arith::ConstantOp::isBuildableWith(value, type))
     return builder.create<arith::ConstantOp>(loc, type, value);
-  return builder.create<ConstantOp>(loc, type, value);
+  if (ConstantOp::isBuildableWith(value, type))
+    return builder.create<ConstantOp>(loc, type,
+                                      value.cast<FlatSymbolRefAttr>());
+  return nullptr;
 }
 
 //===----------------------------------------------------------------------===//
 // AssertOp
 //===----------------------------------------------------------------------===//
 
 LogicalResult AssertOp::canonicalize(AssertOp op, PatternRewriter &rewriter) {
   // Erase assertion if argument is constant true.
if (IntegerAttr condAttr = operands.front().dyn_cast_or_null<IntegerAttr>())
     return condAttr.getValue().isOneValue() ? getTrueDest() : getFalseDest();
   return nullptr;
 }
 
 //===----------------------------------------------------------------------===//
 // ConstantOp
 //===----------------------------------------------------------------------===//
 
-static void print(OpAsmPrinter &p, ConstantOp &op) {
-  p << " ";
-  p.printOptionalAttrDict(op->getAttrs(), /*elidedAttrs=*/{"value"});
-
-  if (op->getAttrs().size() > 1)
-    p << ' ';
-  p << op.getValue();
-
-  // If the value is a symbol reference, print a trailing type.
-  if (op.getValue().isa<SymbolRefAttr>())
-    p << " : " << op.getType();
-}
-
-static ParseResult parseConstantOp(OpAsmParser &parser,
-                                   OperationState &result) {
-  Attribute valueAttr;
-  if (parser.parseOptionalAttrDict(result.attributes) ||
-      parser.parseAttribute(valueAttr, "value", result.attributes))
-    return failure();
-
-  // If the attribute is a symbol reference, then we expect a trailing type.
-  Type type;
-  if (!valueAttr.isa<SymbolRefAttr>())
-    type = valueAttr.getType();
-  else if (parser.parseColonType(type))
-    return failure();
-
-  // Add the attribute type to the list.
-  return parser.addTypeToList(type, result.types);
-}
-
-/// The constant op requires an attribute, and furthermore requires that it
-/// matches the return type.
 LogicalResult ConstantOp::verify() {
-  auto value = getValue();
-  if (!value)
-    return emitOpError("requires a 'value' attribute");
-
+  StringRef fnName = getValue();
   Type type = getType();
-  if (!value.getType().isa<NoneType>() && type != value.getType())
-    return emitOpError() << "requires attribute's type (" << value.getType()
-                         << ") to match op's return type (" << type << ")";
-
-  if (type.isa<FunctionType>()) {
-    auto fnAttr = value.dyn_cast<FlatSymbolRefAttr>();
-    if (!fnAttr)
-      return emitOpError("requires 'value' to be a function reference");
 
-    // Try to find the referenced function.
-    auto fn = (*this)->getParentOfType<ModuleOp>().lookupSymbol<FuncOp>(
-        fnAttr.getValue());
-    if (!fn)
-      return emitOpError() << "reference to undefined function '"
-                           << fnAttr.getValue() << "'";
+  auto fn = (*this)->getParentOfType<ModuleOp>().lookupSymbol<FuncOp>(fnName);
+    return emitOpError() << "reference to undefined function '" << fnName
+                         << "'";
 
-    // Check that the referenced function has the correct type.
-    if (fn.getType() != type)
-      return emitOpError("reference to function with mismatched type");
 
-    return success();
-  }
 
-  if (type.isa<NoneType>() && value.isa<UnitAttr>())
-    return success();
-
-  return emitOpError("unsupported 'value' attribute: ") << value;
-}
-
 OpFoldResult ConstantOp::fold(ArrayRef<Attribute> operands) {
   assert(operands.empty() && "constant has no operands");
-  return getValue();
+  return getValueAttr();
 }
 
 void ConstantOp::getAsmResultNames(
     function_ref<void(Value, StringRef)> setNameFn) {
-  Type type = getType();
-  if (type.isa<FunctionType>()) {
-    setNameFn(getResult(), "f");
-  } else {
-    setNameFn(getResult(), "cst");
-  }
 }
 
-/// Returns true if a constant operation can be built with the given value and
-/// result type.
 bool ConstantOp::isBuildableWith(Attribute value, Type type) {
-  // SymbolRefAttr can only be used with a function type.
-  if (value.isa<SymbolRefAttr>())
-    return type.isa<FunctionType>();
-  // Otherwise, this must be a UnitAttr.
-  return value.isa<UnitAttr>() && type.isa<NoneType>();
+  return value.isa<FlatSymbolRefAttr>() && type.isa<FunctionType>();
 }
 
 //===----------------------------------------------------------------------===//
 // ReturnOp
 //===----------------------------------------------------------------------===//
 
 LogicalResult ReturnOp::verify() {
   auto function = cast<FuncOp>((*this)->getParentOp());

mlir/lib/Dialect/Vector/Transforms/VectorMultiDimReductionTransforms.cpp

LogicalResult matchAndRewrite(vector::MultiDimReductionOp multiReductionOp,
     auto srcRank = multiReductionOp.getSourceVectorType().getRank();
     if (srcRank != 2)
       return failure();
 
     if (multiReductionOp.isReducedDim(0) || !multiReductionOp.isReducedDim(1))
       return failure();
 
     auto loc = multiReductionOp.getLoc();
-    Value result = rewriter.create<ConstantOp>(
+    Value result = rewriter.create<arith::ConstantOp>(
         loc, multiReductionOp.getDestType(),
         rewriter.getZeroAttr(multiReductionOp.getDestType()));
     int outerDim = multiReductionOp.getSourceVectorType().getShape()[0];
 
     // TODO: Add vector::CombiningKind attribute instead of string to
     // vector.reduction.
     auto getKindStr = [](vector::CombiningKind kind) {
       switch (kind) {

mlir/lib/Target/Cpp/TranslateToCpp.cpp

static LogicalResult printOperation(CppEmitter &emitter,
   Attribute value = constantOp.getValue();
 
   return printConstantOp(emitter, operation, value);
 }
 
 static LogicalResult printOperation(CppEmitter &emitter,
                                     mlir::ConstantOp constantOp) {
   Operation *operation = constantOp.getOperation();
-  Attribute value = constantOp.getValue();
+  Attribute value = constantOp.getValueAttr();
 
   return printConstantOp(emitter, operation, value);
 }
 
 static LogicalResult printOperation(CppEmitter &emitter, BranchOp branchOp) {
   raw_ostream &os = emitter.ostream();
   Block &successor = *branchOp.getSuccessor();
 

mlir/test/Dialect/Standard/invalid.mlir

 // RUN: mlir-opt -split-input-file %s -verify-diagnostics
 
 func @unsupported_attribute() {
-  // expected-error @+1 {{unsupported 'value' attribute: "" : index}}
+  // expected-error @+1 {{invalid kind of attribute specified}}
   %0 = constant "" : index
   return
 }
 
 // -----
 
 func @return_i32_f32() -> (i32, f32) {
   %0 = arith.constant 1 : i32

mlir/test/IR/core-ops.mlir

^bb42(%t: tensor<4x4x?xf32>, %f: f32, %i: i32, %idx : index, %j: i64, %half: f16):
   %68 = "arith.cmpf"(%vcf32, %vcf32) {predicate = 1} : (vector<4 x f32>, vector<4 x f32>) -> vector<4 x i1>
 
   // CHECK: %{{.*}} = arith.cmpf oeq, %{{.*}}, %{{.*}}: tensor<42xf32>
   %69 = arith.cmpf oeq, %tcf32, %tcf32 : tensor<42 x f32>
 
   // CHECK: %{{.*}} = arith.cmpf oeq, %{{.*}}, %{{.*}}: vector<4xf32>
   %70 = arith.cmpf oeq, %vcf32, %vcf32 : vector<4 x f32>
 
-  // CHECK: = constant unit
-  %73 = constant unit
-
   // CHECK: arith.constant true
   %74 = arith.constant true
 
   // CHECK: arith.constant false
   %75 = arith.constant false
 
   // CHECK: %{{.*}} = math.abs %arg1 : f32
   %100 = "math.abs"(%f) : (f32) -> f32

mlir/test/lib/Dialect/Test/TestPatterns.cpp

 // This pattern replaces explicitly illegal op with explicitly legal op,
 // but in addition creates unregistered operation.
 struct TestCreateUnregisteredOp : public OpRewritePattern<ILLegalOpG> {
   using OpRewritePattern<ILLegalOpG>::OpRewritePattern;
 
   LogicalResult matchAndRewrite(ILLegalOpG op,
                                 PatternRewriter &rewriter) const final {
     IntegerAttr attr = rewriter.getI32IntegerAttr(0);
-    Value val = rewriter.create<ConstantOp>(op->getLoc(), attr);
+    Value val = rewriter.create<arith::ConstantOp>(op->getLoc(), attr);
     rewriter.replaceOpWithNewOp<LegalOpC>(op, val);
     return success();
   };
 };
 } // namespace
 
 namespace {
 struct TestTypeConverter : public TypeConverter {