[mlir][llvm] Fix verifier for const float

[rikhuijzer]

Fixes one of the cases of #56962.

This PR basically moves some code from mlir::LLVM::detail::getLLVMConstant (source) over to the verifier of LLVM::ConstantOp. For now, I focused just on the case where the attribute is a float and ignored the integer case of #56962. Note that without this patch, both added tests will crash inside getLLVMConstant during mlir-translate -mlir-to-llvmir.

[llvmbot]

@llvm/pr-subscribers-mlir

@llvm/pr-subscribers-mlir-llvm

Author: Rik Huijzer (rikhuijzer)

Changes

Fixes one of the cases of #56962.

This PR basically moves some code from mlir::LLVM::detail::getLLVMConstant (source) over to the verifier of LLVM::ConstantOp. For now, I focused just on the case where the attribute is a float and ignored the integer case of #56962. Note that without this patch, both added tests will crash inside getLLVMConstant during mlir-translate -mlir-to-llvmir.

---

Full diff: https://github.com/llvm/llvm-project/pull/74247.diff

3 Files Affected:

• (modified) mlir/lib/Dialect/LLVMIR/IR/LLVMDialect.cpp (+14)
• (modified) mlir/test/Target/LLVMIR/llvmir-invalid.mlir (+16)
• (modified) mlir/test/Target/LLVMIR/llvmir.mlir (+1-1)

diff --git a/mlir/lib/Dialect/LLVMIR/IR/LLVMDialect.cpp b/mlir/lib/Dialect/LLVMIR/IR/LLVMDialect.cpp
index f6c8f388732c3..71860363b8ea5 100644
--- a/mlir/lib/Dialect/LLVMIR/IR/LLVMDialect.cpp
+++ b/mlir/lib/Dialect/LLVMIR/IR/LLVMDialect.cpp
@@ -2539,6 +2539,20 @@ LogicalResult LLVM::ConstantOp::verify() {
   if (!llvm::isa<IntegerAttr, ArrayAttr, FloatAttr, ElementsAttr>(getValue()))
     return emitOpError()
            << "only supports integer, float, string or elements attributes";
+  if (auto floatAttr = dyn_cast<FloatAttr>(getValue())) {
+    const llvm::fltSemantics &sem = floatAttr.getValue().getSemantics();
+    unsigned floatWidth = APFloat::getSizeInBits(sem);
+    if (auto floatTy = dyn_cast<FloatType>(getType())) {
+      if (floatTy.getWidth() != floatWidth) {
+        return emitOpError() << "expected float type of width " << floatWidth;
+      }
+    }
+    // See the comment for getLLVMConstant for more details about why 8-bit
+    // floats can be represented by integers.
+    if (getType().isa<IntegerType>() && !getType().isInteger(floatWidth)) {
+      return emitOpError() << "expected integer type of width " << floatWidth;
+    }
+  }
   return success();
 }
 
diff --git a/mlir/test/Target/LLVMIR/llvmir-invalid.mlir b/mlir/test/Target/LLVMIR/llvmir-invalid.mlir
index e531c3cb4e240..0def5895fb330 100644
--- a/mlir/test/Target/LLVMIR/llvmir-invalid.mlir
+++ b/mlir/test/Target/LLVMIR/llvmir-invalid.mlir
@@ -31,6 +31,22 @@ llvm.func @struct_wrong_attribute_element_type() -> !llvm.struct<(f64, f64)> {
 
 // -----
 
+llvm.func @incompatible_float_attribute_type() -> f32 {
+  // expected-error @below{{expected float type of width 64}}
+  %cst = llvm.mlir.constant(1.0 : f64) : f32
+  llvm.return %cst : f32
+}
+
+// -----
+
+llvm.func @incompatible_integer_type_for_float_attr() -> i32 {
+  // expected-error @below{{expected integer type of width 16}}
+  %cst = llvm.mlir.constant(1.0 : f16) : i32
+  llvm.return %cst : i32
+}
+
+// -----
+
 // expected-error @below{{unsupported constant value}}
 llvm.mlir.global internal constant @test([2.5, 7.4]) : !llvm.array<2 x f64>
 
diff --git a/mlir/test/Target/LLVMIR/llvmir.mlir b/mlir/test/Target/LLVMIR/llvmir.mlir
index ab8506ff163ef..3f84f9dc5a9b8 100644
--- a/mlir/test/Target/LLVMIR/llvmir.mlir
+++ b/mlir/test/Target/LLVMIR/llvmir.mlir
@@ -66,7 +66,7 @@ llvm.mlir.global external @explicit_undef() : i32 {
 // CHECK: @int_gep = internal constant ptr getelementptr (i32, ptr @i32_global, i32 2)
 llvm.mlir.global internal constant @int_gep() : !llvm.ptr {
   %addr = llvm.mlir.addressof @i32_global : !llvm.ptr
-  %_c0 = llvm.mlir.constant(2: i32) :i32
+  %_c0 = llvm.mlir.constant(2: i32) : i32
   %gepinit = llvm.getelementptr %addr[%_c0] : (!llvm.ptr, i32) -> !llvm.ptr, i32
   llvm.return %gepinit : !llvm.ptr
 }

[joker-eph]

Thanks for the fix!