[mlir] Fix ConstantOp verifier

[marbre]

Summary

This restricts the attributes to integers for constants of type
IndexType. So far an attribute like StringAttr as in

%c1 = constant "" : index

is valid.

[marbre]

Updating D98216: [mlir] Fix ConstantOp verifier

[marbre]

Updating D98216: [mlir] Fix ConstantOp verifier

[marbre]

Updating D98216: [mlir] Fix ConstantOp verifier

[mehdi_amini]

Can you add a test?

[rriddle]

Please add a test.

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

1208

Can you just move this under the IntegerAttr case below?

[marbre]

Add test

[marbre]

In D98216#2617349, @mehdi_amini wrote:

Can you add a test?

Done :)

[marbre]

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

1208

Do you propose to swap if (type.isa<IndexType>()) with if (!value.isa<IntegerAttr>()) or to integrate it into if (auto intAttr = value.dyn_cast<IntegerAttr>()) (line 1217) check? I think the latter is not possible.

[rriddle]

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

1207–1208

This could also move into the IntegerAttr check, BoolAttr is now just a special case of IntegerAttr(it used to be different).

1208

Why can't you just do:

if (auto intAttr = value.dyn_cast<IntegerAttr>()) {
  if (type.isa<IndexType>())
    return success();

  ...
}

[marbre]

Address River's comment

[marbre]

All done.Address River's comment

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

1208

Got your pint!

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

static LogicalResult verify(ConstantOp &op) {
   auto value = op.getValue();
   if (!value)
     return op.emitOpError("requires a 'value' attribute");
 
   Type type = op.getType();
   if (!value.getType().isa<NoneType>() && type != value.getType())
     return op.emitOpError() << "requires attribute's type (" << value.getType()
                             << ") to match op's return type (" << type << ")";
 
+  if (auto intAttr = value.dyn_cast<IntegerAttr>()) {
     if (type.isa<IndexType>() || value.isa<BoolAttr>())
       return success();
-
-  if (auto intAttr = value.dyn_cast<IntegerAttr>()) {
     IntegerType intType = type.cast<IntegerType>();
     if (!intType.isSignless())
       return op.emitOpError("requires integer result types to be signless");
 
     // If the type has a known bitwidth we verify that the value can be
     // represented with the given bitwidth.
     unsigned bitwidth = intType.getWidth();
     APInt intVal = intAttr.getValue();

mlir/test/Dialect/Standard/invalid.mlir

 
 // -----
 
 func @non_signless_constant() {
   // expected-error @+1 {{requires integer result types to be signless}}
   %0 = constant 0 : si32
   return
 }
+
+// -----
+
+func @unsupported_attribute() {
+  // expected-error @+1 {{unsupported 'value' attribute: "" : index}}
+  %0 = constant "" : index
+  return
+}