[Mlir-commits] [mlir] [mlir][EmitC] Disallow string attributes as initial values (PR #75310)
llvmlistbot at llvm.org
llvmlistbot at llvm.org
Wed Dec 13 02:08:24 PST 2023
llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-mlir
Author: Simon Camphausen (simon-camp)
<details>
<summary>Changes</summary>
---
Full diff: https://github.com/llvm/llvm-project/pull/75310.diff
3 Files Affected:
- (modified) mlir/lib/Dialect/EmitC/IR/EmitC.cpp (+29-24)
- (modified) mlir/test/Dialect/EmitC/invalid_ops.mlir (+8-16)
- (modified) mlir/test/Target/Cpp/const.mlir (+3-3)
``````````diff
diff --git a/mlir/lib/Dialect/EmitC/IR/EmitC.cpp b/mlir/lib/Dialect/EmitC/IR/EmitC.cpp
index e8ea4da0b089c..e9379d5b711d9 100644
--- a/mlir/lib/Dialect/EmitC/IR/EmitC.cpp
+++ b/mlir/lib/Dialect/EmitC/IR/EmitC.cpp
@@ -50,6 +50,26 @@ void mlir::emitc::buildTerminatedBody(OpBuilder &builder, Location loc) {
builder.create<emitc::YieldOp>(loc);
}
+/// Check that the type of the initial value is compatible with the operations
+/// result type.
+LogicalResult verifyInitializationAttribute(Operation *op, Attribute value,
+ Type expectedType) {
+ if (llvm::isa<emitc::OpaqueAttr>(value))
+ return success();
+
+ if (llvm::isa<StringAttr>(value))
+ return op->emitOpError()
+ << "string attributes are not supported, use #emitc.opaque instead";
+
+ auto typedValue = cast<TypedAttr>(value);
+ if (!llvm::isa<NoneType>(typedValue.getType()) &&
+ expectedType != typedValue.getType())
+ return op->emitOpError()
+ << "requires attribute's type (" << typedValue.getType()
+ << ") to match op's return type (" << expectedType << ")";
+ return success();
+}
+
//===----------------------------------------------------------------------===//
// AddOp
//===----------------------------------------------------------------------===//
@@ -169,21 +189,14 @@ LogicalResult emitc::CallOpaqueOp::verify() {
// ConstantOp
//===----------------------------------------------------------------------===//
-/// The constant op requires that the attribute's type matches the return type.
LogicalResult emitc::ConstantOp::verify() {
- if (llvm::isa<emitc::OpaqueAttr>(getValueAttr()))
- return success();
-
- // Value must not be empty
- StringAttr strAttr = llvm::dyn_cast<StringAttr>(getValueAttr());
- if (strAttr && strAttr.empty())
- return emitOpError() << "value must not be empty";
-
- auto value = cast<TypedAttr>(getValueAttr());
- Type type = getType();
- if (!llvm::isa<NoneType>(value.getType()) && type != value.getType())
- return emitOpError() << "requires attribute's type (" << value.getType()
- << ") to match op's return type (" << type << ")";
+ Attribute value = getValueAttr();
+ if (failed(verifyInitializationAttribute(getOperation(), value, getType())))
+ return failure();
+ if (auto opaqueValue = llvm::dyn_cast<emitc::OpaqueAttr>(value)) {
+ if (opaqueValue.getValue().empty())
+ return emitOpError() << "value must not be empty";
+ }
return success();
}
@@ -517,17 +530,9 @@ LogicalResult SubOp::verify() {
// VariableOp
//===----------------------------------------------------------------------===//
-/// The variable op requires that the attribute's type matches the return type.
LogicalResult emitc::VariableOp::verify() {
- if (llvm::isa<emitc::OpaqueAttr>(getValueAttr()))
- return success();
-
- auto value = cast<TypedAttr>(getValueAttr());
- Type type = getType();
- if (!llvm::isa<NoneType>(value.getType()) && type != value.getType())
- return emitOpError() << "requires attribute's type (" << value.getType()
- << ") to match op's return type (" << type << ")";
- return success();
+ return verifyInitializationAttribute(getOperation(), getValueAttr(),
+ getType());
}
//===----------------------------------------------------------------------===//
diff --git a/mlir/test/Dialect/EmitC/invalid_ops.mlir b/mlir/test/Dialect/EmitC/invalid_ops.mlir
index 49efb962dfa25..0814e1d42de82 100644
--- a/mlir/test/Dialect/EmitC/invalid_ops.mlir
+++ b/mlir/test/Dialect/EmitC/invalid_ops.mlir
@@ -1,16 +1,16 @@
// RUN: mlir-opt %s -split-input-file -verify-diagnostics
-func.func @const_attribute_return_type_1() {
- // expected-error @+1 {{'emitc.constant' op requires attribute's type ('i64') to match op's return type ('i32')}}
- %c0 = "emitc.constant"(){value = 42: i64} : () -> i32
+func.func @const_attribute_str() {
+ // expected-error @+1 {{'emitc.constant' op string attributes are not supported, use #emitc.opaque instead}}
+ %c0 = "emitc.constant"(){value = "NULL"} : () -> !emitc.ptr<i32>
return
}
// -----
-func.func @const_attribute_return_type_2() {
- // expected-error @+1 {{'emitc.constant' op requires attribute's type ('!emitc.opaque<"char">') to match op's return type ('!emitc.opaque<"mychar">')}}
- %c0 = "emitc.constant"(){value = "CHAR_MIN" : !emitc.opaque<"char">} : () -> !emitc.opaque<"mychar">
+func.func @const_attribute_return_type() {
+ // expected-error @+1 {{'emitc.constant' op requires attribute's type ('i64') to match op's return type ('i32')}}
+ %c0 = "emitc.constant"(){value = 42: i64} : () -> i32
return
}
@@ -18,7 +18,7 @@ func.func @const_attribute_return_type_2() {
func.func @empty_constant() {
// expected-error @+1 {{'emitc.constant' op value must not be empty}}
- %c0 = "emitc.constant"(){value = ""} : () -> i32
+ %c0 = "emitc.constant"(){value = #emitc.opaque<"">} : () -> i32
return
}
@@ -97,7 +97,7 @@ func.func @illegal_operand() {
// -----
-func.func @var_attribute_return_type_1() {
+func.func @var_attribute_return_type() {
// expected-error @+1 {{'emitc.variable' op requires attribute's type ('i64') to match op's return type ('i32')}}
%c0 = "emitc.variable"(){value = 42: i64} : () -> i32
return
@@ -105,14 +105,6 @@ func.func @var_attribute_return_type_1() {
// -----
-func.func @var_attribute_return_type_2() {
- // expected-error @+1 {{'emitc.variable' op requires attribute's type ('!emitc.ptr<i64>') to match op's return type ('!emitc.ptr<i32>')}}
- %c0 = "emitc.variable"(){value = "nullptr" : !emitc.ptr<i64>} : () -> !emitc.ptr<i32>
- return
-}
-
-// -----
-
func.func @cast_tensor(%arg : tensor<f32>) {
// expected-error @+1 {{'emitc.cast' op operand type 'tensor<f32>' and result type 'tensor<f32>' are cast incompatible}}
%1 = emitc.cast %arg: tensor<f32> to tensor<f32>
diff --git a/mlir/test/Target/Cpp/const.mlir b/mlir/test/Target/Cpp/const.mlir
index e6c94732e9f6b..28a547909a0ac 100644
--- a/mlir/test/Target/Cpp/const.mlir
+++ b/mlir/test/Target/Cpp/const.mlir
@@ -2,7 +2,7 @@
// RUN: mlir-translate -mlir-to-cpp -declare-variables-at-top %s | FileCheck %s -check-prefix=CPP-DECLTOP
func.func @emitc_constant() {
- %c0 = "emitc.constant"(){value = #emitc.opaque<"">} : () -> i32
+ %c0 = "emitc.constant"(){value = #emitc.opaque<"INT_MAX">} : () -> i32
%c1 = "emitc.constant"(){value = 42 : i32} : () -> i32
%c2 = "emitc.constant"(){value = -1 : i32} : () -> i32
%c3 = "emitc.constant"(){value = -1 : si8} : () -> si8
@@ -11,7 +11,7 @@ func.func @emitc_constant() {
return
}
// CPP-DEFAULT: void emitc_constant() {
-// CPP-DEFAULT-NEXT: int32_t [[V0:[^ ]*]];
+// CPP-DEFAULT-NEXT: int32_t [[V0:[^ ]*]] = INT_MAX;
// CPP-DEFAULT-NEXT: int32_t [[V1:[^ ]*]] = 42;
// CPP-DEFAULT-NEXT: int32_t [[V2:[^ ]*]] = -1;
// CPP-DEFAULT-NEXT: int8_t [[V3:[^ ]*]] = -1;
@@ -25,7 +25,7 @@ func.func @emitc_constant() {
// CPP-DECLTOP-NEXT: int8_t [[V3:[^ ]*]];
// CPP-DECLTOP-NEXT: uint8_t [[V4:[^ ]*]];
// CPP-DECLTOP-NEXT: char [[V5:[^ ]*]];
-// CPP-DECLTOP-NEXT: ;
+// CPP-DECLTOP-NEXT: [[V0]] = INT_MAX;
// CPP-DECLTOP-NEXT: [[V1]] = 42;
// CPP-DECLTOP-NEXT: [[V2]] = -1;
// CPP-DECLTOP-NEXT: [[V3]] = -1;
``````````
</details>
https://github.com/llvm/llvm-project/pull/75310
More information about the Mlir-commits
mailing list