Skip to content

Commit 4ded2c6

Browse files
mgehre-amdmgehre-amd
committed
Address comments
1 parent d46d2b2 commit 4ded2c6

File tree

10 files changed

+146
-15
lines changed

10 files changed

+146
-15
lines changed

‎mlir/lib/Dialect/EmitC/IR/EmitC.cpp‎

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -140,6 +140,8 @@ LogicalResult emitc::AssignOp::verify() {
140140
return emitOpError() << "requires value's type (" << value.getType()
141141
<< ") to match variable's type (" << variable.getType()
142142
<< ")";
143+
if (isa<ArrayType>(variable.getType()))
144+
return emitOpError() << "cannot assign to array type";
143145
return success();
144146
}
145147

@@ -191,6 +193,11 @@ LogicalResult emitc::CallOpaqueOp::verify() {
191193
}
192194
}
193195

196+
if (llvm::any_of(getResultTypes(),
197+
[](Type type) { return isa<ArrayType>(type); })) {
198+
return emitOpError() << "cannot return array type";
199+
}
200+
194201
return success();
195202
}
196203

@@ -455,6 +462,9 @@ LogicalResult FuncOp::verify() {
455462
return emitOpError("requires zero or exactly one result, but has ")
456463
<< getNumResults();
457464

465+
if (getNumResults() == 1 && isa<ArrayType>(getResultTypes()[0]))
466+
return emitOpError("cannot return array type");
467+
458468
return success();
459469
}
460470

@@ -780,7 +790,7 @@ Type emitc::ArrayType::parse(AsmParser &parser) {
780790
if (parser.parseType(elementType))
781791
return Type();
782792

783-
// Check that memref is formed from allowed types.
793+
// Check that array is formed from allowed types.
784794
if (!isValidElementType(elementType))
785795
return parser.emitError(typeLoc, "invalid array element type"), Type();
786796
if (parser.parseGreater())

‎mlir/lib/Target/Cpp/TranslateToCpp.cpp‎

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -904,6 +904,9 @@ static LogicalResult printFunctionBody(CppEmitter &emitter,
904904
if (emitter.hasValueInScope(arg))
905905
return functionOp->emitOpError(" block argument #")
906906
<< arg.getArgNumber() << " is out of scope";
907+
if (isa<ArrayType>(arg.getType()))
908+
return functionOp->emitOpError("cannot emit block argument #")
909+
<< arg.getArgNumber() << " with array type";
907910
if (failed(
908911
emitter.emitType(block.getParentOp()->getLoc(), arg.getType()))) {
909912
return failure();
@@ -947,6 +950,11 @@ static LogicalResult printOperation(CppEmitter &emitter,
947950
"with multiple blocks needs variables declared at top");
948951
}
949952

953+
if (llvm::any_of(functionOp.getResultTypes(),
954+
[](Type type) { return isa<ArrayType>(type); })) {
955+
return functionOp.emitOpError() << "cannot emit array type as result type";
956+
}
957+
950958
CppEmitter::Scope scope(emitter);
951959
raw_indented_ostream &os = emitter.ostream();
952960
if (failed(emitter.emitTypes(functionOp.getLoc(),
@@ -1445,6 +1453,8 @@ LogicalResult CppEmitter::emitType(Location loc, Type type) {
14451453
if (!tType.hasStaticShape())
14461454
return emitError(loc, "cannot emit tensor type with non static shape");
14471455
os << "Tensor<";
1456+
if (isa<ArrayType>(tType.getElementType()))
1457+
return emitError(loc, "cannot emit tensor of array type ") << type;
14481458
if (failed(emitType(loc, tType.getElementType())))
14491459
return failure();
14501460
auto shape = tType.getShape();
@@ -1461,7 +1471,16 @@ LogicalResult CppEmitter::emitType(Location loc, Type type) {
14611471
os << oType.getValue();
14621472
return success();
14631473
}
1474+
if (auto aType = dyn_cast<emitc::ArrayType>(type)) {
1475+
if (failed(emitType(loc, aType.getElementType())))
1476+
return failure();
1477+
for (auto dim : aType.getShape())
1478+
os << "[" << dim << "]";
1479+
return success();
1480+
}
14641481
if (auto pType = dyn_cast<emitc::PointerType>(type)) {
1482+
if (isa<ArrayType>(pType.getPointee()))
1483+
return emitError(loc, "cannot emit pointer to array type ") << type;
14651484
if (failed(emitType(loc, pType.getPointee())))
14661485
return failure();
14671486
os << "*";
@@ -1483,6 +1502,9 @@ LogicalResult CppEmitter::emitTypes(Location loc, ArrayRef<Type> types) {
14831502
}
14841503

14851504
LogicalResult CppEmitter::emitTupleType(Location loc, ArrayRef<Type> types) {
1505+
if (llvm::any_of(types, [](Type type) { return isa<ArrayType>(type); })) {
1506+
return emitError(loc, "cannot emit tuple of array type");
1507+
}
14861508
os << "std::tuple<";
14871509
if (failed(interleaveCommaWithError(
14881510
types, os, [&](Type type) { return emitType(loc, type); })))

‎mlir/test/Dialect/EmitC/invalid_ops.mlir‎

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -80,6 +80,14 @@ func.func @dense_template_argument(%arg : i32) {
8080

8181
// -----
8282

83+
func.func @array_result() {
84+
// expected-error @+1 {{'emitc.call_opaque' op cannot return array type}}
85+
emitc.call_opaque "array_result"() : () -> !emitc.array<4xi32>
86+
return
87+
}
88+
89+
// -----
90+
8391
func.func @empty_operator(%arg : i32) {
8492
// expected-error @+1 {{'emitc.apply' op applicable operator must not be empty}}
8593
%2 = emitc.apply ""(%arg) : (i32) -> !emitc.ptr<i32>
@@ -129,6 +137,14 @@ func.func @cast_tensor(%arg : tensor<f32>) {
129137

130138
// -----
131139

140+
func.func @cast_array(%arg : !emitc.array<4xf32>) {
141+
// expected-error @+1 {{'emitc.cast' op operand type '!emitc.array<4xf32>' and result type '!emitc.array<4xf32>' are cast incompatible}}
142+
%1 = emitc.cast %arg: !emitc.array<4xf32> to !emitc.array<4xf32>
143+
return
144+
}
145+
146+
// -----
147+
132148
func.func @add_two_pointers(%arg0: !emitc.ptr<f32>, %arg1: !emitc.ptr<f32>) {
133149
// expected-error @+1 {{'emitc.add' op requires that at most one operand is a pointer}}
134150
%1 = "emitc.add" (%arg0, %arg1) : (!emitc.ptr<f32>, !emitc.ptr<f32>) -> !emitc.ptr<f32>
@@ -235,6 +251,15 @@ func.func @test_assign_type_mismatch(%arg1: f32) {
235251

236252
// -----
237253

254+
func.func @test_assign_to_array(%arg1: !emitc.array<4xi32>) {
255+
%v = "emitc.variable"() <{value = #emitc.opaque<"">}> : () -> !emitc.array<4xi32>
256+
// expected-error @+1 {{'emitc.assign' op cannot assign to array type}}
257+
emitc.assign %arg1 : !emitc.array<4xi32> to %v : !emitc.array<4xi32>
258+
return
259+
}
260+
261+
// -----
262+
238263
func.func @test_expression_no_yield() -> i32 {
239264
// expected-error @+1 {{'emitc.expression' op must yield a value at termination}}
240265
%r = emitc.expression : i32 {
@@ -313,6 +338,13 @@ emitc.func @return_type_mismatch() -> i32 {
313338

314339
// -----
315340

341+
// expected-error@+1 {{'emitc.func' op cannot return array type}}
342+
emitc.func @return_type_array(%arg : !emitc.array<4xi32>) -> !emitc.array<4xi32> {
343+
emitc.return %arg : !emitc.array<4xi32>
344+
}
345+
346+
// -----
347+
316348
func.func @return_inside_func.func(%0: i32) -> (i32) {
317349
// expected-error@+1 {{'emitc.return' op expects parent op 'emitc.func'}}
318350
emitc.return %0 : i32

‎mlir/test/Dialect/EmitC/invalid_types.mlir‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -65,3 +65,19 @@ func.func @illegal_array_unranked(
6565
%arg0: !emitc.array<*xi32>
6666
) {
6767
}
68+
69+
// -----
70+
71+
func.func @illegal_array_with_array_element_type(
72+
// expected-error @+1 {{invalid array element type}}
73+
%arg0: !emitc.array<4x!emitc.array<4xi32>>
74+
) {
75+
}
76+
77+
// -----
78+
79+
func.func @illegal_array_with_tensor_element_type(
80+
// expected-error @+1 {{invalid array element type}}
81+
%arg0: !emitc.array<4xtensor<4xi32>>
82+
) {
83+
}

‎mlir/test/Dialect/EmitC/types.mlir‎

Lines changed: 14 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,20 @@
22
// check parser
33
// RUN: mlir-opt -verify-diagnostics %s | mlir-opt -verify-diagnostics | FileCheck %s
44

5+
// CHECK-LABEL: func @array_types(
6+
func.func @array_types(
7+
// CHECK-SAME: !emitc.array<1xf32>,
8+
%arg0: !emitc.array<1xf32>,
9+
// CHECK-SAME: !emitc.array<10x20x30xi32>,
10+
%arg1: !emitc.array<10x20x30xi32>,
11+
// CHECK-SAME: !emitc.array<30x!emitc.ptr<i32>>,
12+
%arg2: !emitc.array<30x!emitc.ptr<i32>>,
13+
// CHECK-SAME: !emitc.array<30x!emitc.opaque<"int">>
14+
%arg3: !emitc.array<30x!emitc.opaque<"int">>
15+
) {
16+
return
17+
}
18+
519
// CHECK-LABEL: func @opaque_types() {
620
func.func @opaque_types() {
721
// CHECK-NEXT: !emitc.opaque<"int">
@@ -39,17 +53,3 @@ func.func @pointer_types() {
3953

4054
return
4155
}
42-
43-
// CHECK-LABEL: func @array_types(
44-
func.func @array_types(
45-
// CHECK-SAME: !emitc.array<1xf32>,
46-
%arg0: !emitc.array<1xf32>,
47-
// CHECK-SAME: !emitc.array<10x20x30xi32>,
48-
%arg1: !emitc.array<10x20x30xi32>,
49-
// CHECK-SAME: !emitc.array<30x!emitc.ptr<i32>>,
50-
%arg2: !emitc.array<30x!emitc.ptr<i32>>,
51-
// CHECK-SAME: !emitc.array<30x!emitc.opaque<"int">>
52-
%arg3: !emitc.array<30x!emitc.opaque<"int">>
53-
) {
54-
return
55-
}

‎mlir/test/Target/Cpp/declare_func.mlir‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,3 +14,11 @@ emitc.declare_func @foo
1414
emitc.func @foo(%arg0: i32) -> i32 attributes {specifiers = ["static","inline"]} {
1515
emitc.return %arg0 : i32
1616
}
17+
18+
19+
// CHECK: void array_arg(int32_t [[V2:[^ ]*]][3]);
20+
emitc.declare_func @array_arg
21+
// CHECK: void array_arg(int32_t [[V2:[^ ]*]][3]) {
22+
emitc.func @array_arg(%arg0: !emitc.array<3xi32>) {
23+
emitc.return
24+
}

‎mlir/test/Target/Cpp/func.mlir‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,3 +40,6 @@ emitc.func @emitc_call() -> i32 {
4040

4141
emitc.func private @extern_func(i32) attributes {specifiers = ["extern"]}
4242
// CPP-DEFAULT: extern void extern_func(int32_t);
43+
44+
emitc.func private @array_arg(!emitc.array<3xi32>) attributes {specifiers = ["extern"]}
45+
// CPP-DEFAULT: extern void array_arg(int32_t[3]);

‎mlir/test/Target/Cpp/invalid.mlir‎

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -57,3 +57,31 @@ func.func @non_static_shape(%arg0 : tensor<?xf32>) {
5757
func.func @unranked_tensor(%arg0 : tensor<*xf32>) {
5858
return
5959
}
60+
61+
// -----
62+
63+
// expected-error@+1 {{cannot emit tensor of array type}}
64+
func.func @tensor_of_array(%arg0 : tensor<4x!emitc.array<4xf32>>) {
65+
return
66+
}
67+
68+
// -----
69+
70+
// expected-error@+1 {{cannot emit pointer to array type}}
71+
func.func @tensor_of_array(%arg0 : !emitc.ptr<!emitc.array<4xf32>>) {
72+
return
73+
}
74+
75+
// -----
76+
77+
// expected-error@+1 {{cannot emit array type as result type}}
78+
func.func @array_as_result(%arg: !emitc.array<4xi8>) -> (!emitc.array<4xi8>) {
79+
return %arg : !emitc.array<4xi8>
80+
}
81+
82+
// -----
83+
func.func @ptr_to_array() {
84+
// expected-error@+1 {{cannot emit pointer to array type '!emitc.ptr<!emitc.array<9xi16>>'}}
85+
%v = "emitc.variable"(){value = #emitc.opaque<"NULL">} : () -> !emitc.ptr<!emitc.array<9xi16>>
86+
return
87+
}

‎mlir/test/Target/Cpp/invalid_declare_variables_at_top.mlir‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
// RUN: mlir-translate -split-input-file -declare-variables-at-top -mlir-to-cpp -verify-diagnostics %s
2+
3+
// expected-error@+1 {{'func.func' op cannot emit block argument #0 with array type}}
4+
func.func @array_as_block_argument(!emitc.array<4xi8>) {
5+
^bb0(%arg0 : !emitc.array<4xi8>):
6+
cf.br ^bb1(%arg0 : !emitc.array<4xi8>)
7+
^bb1(%a : !emitc.array<4xi8>):
8+
return
9+
}

‎mlir/test/Target/Cpp/variable.mlir‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ func.func @emitc_variable() {
1010
%c5 = "emitc.variable"(){value = #emitc.opaque<"">} : () -> !emitc.ptr<i32>
1111
%c6 = "emitc.variable"(){value = #emitc.opaque<"NULL">} : () -> !emitc.ptr<i32>
1212
%c7 = "emitc.variable"(){value = #emitc.opaque<"">} : () -> !emitc.array<3x7xi32>
13+
%c8 = "emitc.variable"(){value = #emitc.opaque<"">} : () -> !emitc.array<5x!emitc.ptr<i8>>
1314
return
1415
}
1516
// CPP-DEFAULT: void emitc_variable() {
@@ -21,6 +22,7 @@ func.func @emitc_variable() {
2122
// CPP-DEFAULT-NEXT: int32_t* [[V5:[^ ]*]];
2223
// CPP-DEFAULT-NEXT: int32_t* [[V6:[^ ]*]] = NULL;
2324
// CPP-DEFAULT-NEXT: int32_t [[V7:[^ ]*]][3][7];
25+
// CPP-DEFAULT-NEXT: int8_t* [[V8:[^ ]*]][5];
2426

2527
// CPP-DECLTOP: void emitc_variable() {
2628
// CPP-DECLTOP-NEXT: int32_t [[V0:[^ ]*]];
@@ -31,6 +33,7 @@ func.func @emitc_variable() {
3133
// CPP-DECLTOP-NEXT: int32_t* [[V5:[^ ]*]];
3234
// CPP-DECLTOP-NEXT: int32_t* [[V6:[^ ]*]];
3335
// CPP-DECLTOP-NEXT: int32_t [[V7:[^ ]*]][3][7];
36+
// CPP-DECLTOP-NEXT: int8_t* [[V8:[^ ]*]][5];
3437
// CPP-DECLTOP-NEXT: ;
3538
// CPP-DECLTOP-NEXT: [[V1]] = 42;
3639
// CPP-DECLTOP-NEXT: [[V2]] = -1;

0 commit comments

Comments
 (0)