[Mlir-commits] [mlir] [mlir][EmitC] Disallow string attributes as initial values (PR #75310)
Simon Camphausen
llvmlistbot at llvm.org
Tue Jan 2 07:32:20 PST 2024
https://github.com/simon-camp updated https://github.com/llvm/llvm-project/pull/75310
>From 673d3fbd7c46b5707dc9a9785d7346d3c0d20fd1 Mon Sep 17 00:00:00 2001
From: Simon Camphausen <simon.camphausen at iml.fraunhofer.de>
Date: Tue, 12 Dec 2023 15:44:27 +0000
Subject: [PATCH] [mlir][EmitC] Disallow string attributes as initial values
---
mlir/lib/Dialect/EmitC/IR/EmitC.cpp | 58 ++++++++++++++----------
mlir/test/Dialect/EmitC/invalid_ops.mlir | 22 ++++++---
mlir/test/Target/Cpp/const.mlir | 6 +--
3 files changed, 52 insertions(+), 34 deletions(-)
diff --git a/mlir/lib/Dialect/EmitC/IR/EmitC.cpp b/mlir/lib/Dialect/EmitC/IR/EmitC.cpp
index fd32efe783bcf5..8508d29d2306a3 100644
--- a/mlir/lib/Dialect/EmitC/IR/EmitC.cpp
+++ b/mlir/lib/Dialect/EmitC/IR/EmitC.cpp
@@ -50,6 +50,32 @@ 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.
+static LogicalResult verifyInitializationAttribute(Operation *op,
+ Attribute value) {
+ assert(op->getNumResults() == 1 && "operation must have 1 result");
+
+ 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";
+
+ Type resultType = op->getResult(0).getType();
+ Type attrType = cast<TypedAttr>(value).getType();
+
+ if (resultType != attrType)
+ return op->emitOpError()
+ << "requires attribute to either be an #emitc.opaque attribute or "
+ "it's type ("
+ << attrType << ") to match the op's result type (" << resultType
+ << ")";
+
+ return success();
+}
+
//===----------------------------------------------------------------------===//
// AddOp
//===----------------------------------------------------------------------===//
@@ -169,21 +195,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)))
+ return failure();
+ if (auto opaqueValue = llvm::dyn_cast<emitc::OpaqueAttr>(value)) {
+ if (opaqueValue.getValue().empty())
+ return emitOpError() << "value must not be empty";
+ }
return success();
}
@@ -561,17 +580,8 @@ 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());
}
//===----------------------------------------------------------------------===//
diff --git a/mlir/test/Dialect/EmitC/invalid_ops.mlir b/mlir/test/Dialect/EmitC/invalid_ops.mlir
index 6ad646d7c62f11..46eccb1c24eea2 100644
--- a/mlir/test/Dialect/EmitC/invalid_ops.mlir
+++ b/mlir/test/Dialect/EmitC/invalid_ops.mlir
@@ -1,7 +1,15 @@
// RUN: mlir-opt %s -split-input-file -verify-diagnostics
+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_1() {
- // expected-error @+1 {{'emitc.constant' op requires attribute's type ('i64') to match op's return type ('i32')}}
+ // expected-error @+1 {{'emitc.constant' op requires attribute to either be an #emitc.opaque attribute or it's type ('i64') to match the op's result type ('i32')}}
%c0 = "emitc.constant"(){value = 42: i64} : () -> i32
return
}
@@ -9,8 +17,8 @@ func.func @const_attribute_return_type_1() {
// -----
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">
+ // expected-error @+1 {{'emitc.constant' op attribute 'value' failed to satisfy constraint: An opaque attribute or TypedAttr instance}}
+ %c0 = "emitc.constant"(){value = unit} : () -> i32
return
}
@@ -18,7 +26,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
}
@@ -98,7 +106,7 @@ func.func @illegal_operand() {
// -----
func.func @var_attribute_return_type_1() {
- // expected-error @+1 {{'emitc.variable' op requires attribute's type ('i64') to match op's return type ('i32')}}
+ // expected-error @+1 {{'emitc.variable' op requires attribute to either be an #emitc.opaque attribute or it's type ('i64') to match the op's result type ('i32')}}
%c0 = "emitc.variable"(){value = 42: i64} : () -> i32
return
}
@@ -106,8 +114,8 @@ 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>
+ // expected-error @+1 {{'emitc.variable' op attribute 'value' failed to satisfy constraint: An opaque attribute or TypedAttr instance}}
+ %c0 = "emitc.variable"(){value = unit} : () -> i32
return
}
diff --git a/mlir/test/Target/Cpp/const.mlir b/mlir/test/Target/Cpp/const.mlir
index e6c94732e9f6bc..28a547909a0ac6 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;
More information about the Mlir-commits
mailing list