Skip to content

Commit 13da9a5

Browse files
authored
[mlir][llvm] Fix verifier for const int and dense (#74340)
Continuation of #74247 to fix #56962. Fixes verifier for (Integer Attr): ```mlir llvm.mlir.constant(1 : index) : f32 ``` and (Dense Attr): ```mlir llvm.mlir.constant(dense<100.0> : vector<1xf64>) : f32 ``` ## Integer Attr The addition that this PR makes to `LLVM::ConstantOp::verify` is meant to be exactly verifying the code in `mlir::LLVM::detail::getLLVMConstant`: https://github.com/llvm/llvm-project/blob/9f78edbd20ed922cced9482f7791deb9899a6d82/mlir/lib/Target/LLVMIR/ModuleTranslation.cpp#L350-L353 One failure mode is when the `type` (`llvm.mlir.constant(<value>) : <type>`) is not an `Integer`, because then the `cast` in `getIntegerBitWidth` will crash: https://github.com/llvm/llvm-project/blob/dca432cb7b1c282f5dc861095813c4f40f109619/llvm/include/llvm/IR/DerivedTypes.h#L97-L99 So that's now caught in the verifier. Apart from that, I don't see anything we could check for. `sextOrTrunc` means "Sign extend or truncate to width" and that one is quite permissive. For example, the following doesn't have to be caught in the verifier as it doesn't crash during `mlir-translate -mlir-to-llvmir`: ```mlir llvm.func @main() -> f32 { %cst = llvm.mlir.constant(100 : i64) : f32 llvm.return %cst : f32 } ``` ## Dense Attr Crash if not either a MLIR Vector type or one of these: https://github.com/llvm/llvm-project/blob/9f78edbd20ed922cced9482f7791deb9899a6d82/mlir/lib/Target/LLVMIR/ModuleTranslation.cpp#L375-L391
1 parent 17de468 commit 13da9a5

File tree

2 files changed

+26
-0
lines changed

2 files changed

+26
-0
lines changed

mlir/lib/Dialect/LLVMIR/IR/LLVMDialect.cpp

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2544,6 +2544,10 @@ LogicalResult LLVM::ConstantOp::verify() {
25442544
if (!llvm::isa<IntegerAttr, ArrayAttr, FloatAttr, ElementsAttr>(getValue()))
25452545
return emitOpError()
25462546
<< "only supports integer, float, string or elements attributes";
2547+
if (auto intAttr = dyn_cast<IntegerAttr>(getValue())) {
2548+
if (!llvm::isa<IntegerType>(getType()))
2549+
return emitOpError() << "expected integer type";
2550+
}
25472551
if (auto floatAttr = dyn_cast<FloatAttr>(getValue())) {
25482552
const llvm::fltSemantics &sem = floatAttr.getValue().getSemantics();
25492553
unsigned floatWidth = APFloat::getSizeInBits(sem);
@@ -2558,6 +2562,12 @@ LogicalResult LLVM::ConstantOp::verify() {
25582562
return emitOpError() << "expected integer type of width " << floatWidth;
25592563
}
25602564
}
2565+
if (auto splatAttr = dyn_cast<SplatElementsAttr>(getValue())) {
2566+
if (!getType().isa<VectorType>() && !getType().isa<LLVM::LLVMArrayType>() &&
2567+
!getType().isa<LLVM::LLVMFixedVectorType>() &&
2568+
!getType().isa<LLVM::LLVMScalableVectorType>())
2569+
return emitOpError() << "expected vector or array type";
2570+
}
25612571
return success();
25622572
}
25632573

mlir/test/Target/LLVMIR/llvmir-invalid.mlir

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,14 @@ func.func @foo() {
77

88
// -----
99

10+
llvm.func @vector_with_non_vector_type() -> f32 {
11+
// expected-error @below{{expected vector or array type}}
12+
%cst = llvm.mlir.constant(dense<100.0> : vector<1xf64>) : f32
13+
llvm.return %cst : f32
14+
}
15+
16+
// -----
17+
1018
llvm.func @no_non_complex_struct() -> !llvm.array<2 x array<2 x array<2 x struct<(i32)>>>> {
1119
// expected-error @below{{expected struct type to be a complex number}}
1220
%0 = llvm.mlir.constant(dense<[[[1, 2], [3, 4]], [[42, 43], [44, 45]]]> : tensor<2x2x2xi32>) : !llvm.array<2 x array<2 x array<2 x struct<(i32)>>>>
@@ -31,6 +39,14 @@ llvm.func @struct_wrong_attribute_element_type() -> !llvm.struct<(f64, f64)> {
3139

3240
// -----
3341

42+
llvm.func @integer_with_float_type() -> f32 {
43+
// expected-error @+1 {{expected integer type}}
44+
%0 = llvm.mlir.constant(1 : index) : f32
45+
llvm.return %0 : f32
46+
}
47+
48+
// -----
49+
3450
llvm.func @incompatible_float_attribute_type() -> f32 {
3551
// expected-error @below{{expected float type of width 64}}
3652
%cst = llvm.mlir.constant(1.0 : f64) : f32

0 commit comments

Comments
 (0)