[mlir] Support emit fp16 and bf16 type to cpp #105803

jacquesguan · 2024-08-23T08:58:48Z

No description provided.

llvmbot · 2024-08-23T08:59:19Z

@llvm/pr-subscribers-mlir

@llvm/pr-subscribers-mlir-emitc

Author: Jianjian Guan (jacquesguan)

Changes

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

5 Files Affected:

(modified) mlir/lib/Dialect/EmitC/IR/EmitC.cpp (+1)
(modified) mlir/lib/Target/Cpp/TranslateToCpp.cpp (+5)
(modified) mlir/test/Conversion/ArithToEmitC/arith-to-emitc-unsupported.mlir (-33)
(modified) mlir/test/Conversion/MemRefToEmitC/memref-to-emitc-failed.mlir (-8)
(modified) mlir/test/Target/Cpp/types.mlir (+4)

diff --git a/mlir/lib/Dialect/EmitC/IR/EmitC.cpp b/mlir/lib/Dialect/EmitC/IR/EmitC.cpp
index e6f1618cc26116..8555e82002d56b 100644
--- a/mlir/lib/Dialect/EmitC/IR/EmitC.cpp
+++ b/mlir/lib/Dialect/EmitC/IR/EmitC.cpp
@@ -116,6 +116,7 @@ bool mlir::emitc::isIntegerIndexOrOpaqueType(Type type) {
 bool mlir::emitc::isSupportedFloatType(Type type) {
   if (auto floatType = llvm::dyn_cast<FloatType>(type)) {
     switch (floatType.getWidth()) {
+    case 16:
     case 32:
     case 64:
       return true;
diff --git a/mlir/lib/Target/Cpp/TranslateToCpp.cpp b/mlir/lib/Target/Cpp/TranslateToCpp.cpp
index c043582b7be9c6..aa45e7c9d7f757 100644
--- a/mlir/lib/Target/Cpp/TranslateToCpp.cpp
+++ b/mlir/lib/Target/Cpp/TranslateToCpp.cpp
@@ -1640,6 +1640,11 @@ LogicalResult CppEmitter::emitType(Location loc, Type type) {
   }
   if (auto fType = dyn_cast<FloatType>(type)) {
     switch (fType.getWidth()) {
+    case 16:
+      if (llvm::isa<Float16Type>(type))
+        return (os << "_Float16"), success();
+      else
+        return (os << "__bf16"), success();
     case 32:
       return (os << "float"), success();
     case 64:
diff --git a/mlir/test/Conversion/ArithToEmitC/arith-to-emitc-unsupported.mlir b/mlir/test/Conversion/ArithToEmitC/arith-to-emitc-unsupported.mlir
index ef0e71ee8673b7..b3eebaf8a1ef1e 100644
--- a/mlir/test/Conversion/ArithToEmitC/arith-to-emitc-unsupported.mlir
+++ b/mlir/test/Conversion/ArithToEmitC/arith-to-emitc-unsupported.mlir
@@ -16,39 +16,6 @@ func.func @arith_cast_vector(%arg0: vector<5xf32>) -> vector<5xi32> {
 
 // -----
 
-func.func @arith_cast_bf16(%arg0: bf16) -> i32 {
-  // expected-error @+1 {{failed to legalize operation 'arith.fptosi'}}
-  %t = arith.fptosi %arg0 : bf16 to i32
-  return %t: i32
-}
-
-// -----
-
-func.func @arith_cast_f16(%arg0: f16) -> i32 {
-  // expected-error @+1 {{failed to legalize operation 'arith.fptosi'}}
-  %t = arith.fptosi %arg0 : f16 to i32
-  return %t: i32
-}
-
-
-// -----
-
-func.func @arith_cast_to_bf16(%arg0: i32) -> bf16 {
-  // expected-error @+1 {{failed to legalize operation 'arith.sitofp'}}
-  %t = arith.sitofp %arg0 : i32 to bf16
-  return %t: bf16
-}
-
-// -----
-
-func.func @arith_cast_to_f16(%arg0: i32) -> f16 {
-  // expected-error @+1 {{failed to legalize operation 'arith.sitofp'}}
-  %t = arith.sitofp %arg0 : i32 to f16
-  return %t: f16
-}
-
-// -----
-
 func.func @arith_cast_fptosi_i1(%arg0: f32) -> i1 {
   // expected-error @+1 {{failed to legalize operation 'arith.fptosi'}}
   %t = arith.fptosi %arg0 : f32 to i1
diff --git a/mlir/test/Conversion/MemRefToEmitC/memref-to-emitc-failed.mlir b/mlir/test/Conversion/MemRefToEmitC/memref-to-emitc-failed.mlir
index 836d8aedefc1f0..14977bfb3e2fd9 100644
--- a/mlir/test/Conversion/MemRefToEmitC/memref-to-emitc-failed.mlir
+++ b/mlir/test/Conversion/MemRefToEmitC/memref-to-emitc-failed.mlir
@@ -46,14 +46,6 @@ memref.global "nested" constant @nested_global : memref<3x7xf32>
 
 // -----
 
-func.func @unsupported_type_f16() {
-  // expected-error@+1 {{failed to legalize operation 'memref.alloca'}}
-  %0 = memref.alloca() : memref<4xf16>
-  return
-}
-
-// -----
-
 func.func @unsupported_type_i4() {
   // expected-error@+1 {{failed to legalize operation 'memref.alloca'}}
   %0 = memref.alloca() : memref<4xi4>
diff --git a/mlir/test/Target/Cpp/types.mlir b/mlir/test/Target/Cpp/types.mlir
index deda383b3b0a72..e7f935c7374382 100644
--- a/mlir/test/Target/Cpp/types.mlir
+++ b/mlir/test/Target/Cpp/types.mlir
@@ -22,6 +22,10 @@ func.func @ptr_types() {
   emitc.call_opaque "f"() {template_args = [!emitc.ptr<i32>]} : () -> ()
   // CHECK-NEXT: f<int64_t*>();
   emitc.call_opaque "f"() {template_args = [!emitc.ptr<i64>]} : () -> ()
+  // CHECK-NEXT: f<_Float16*>();
+  emitc.call_opaque "f"() {template_args = [!emitc.ptr<f16>]} : () -> ()
+  // CHECK-NEXT: f<__bf16*>();
+  emitc.call_opaque "f"() {template_args = [!emitc.ptr<bf16>]} : () -> ()
   // CHECK-NEXT: f<float*>();
   emitc.call_opaque "f"() {template_args = [!emitc.ptr<f32>]} : () -> ()
   // CHECK-NEXT: f<double*>();

mgehre-amd · 2024-08-23T09:07:09Z

mlir/lib/Target/Cpp/TranslateToCpp.cpp

+    case 16:
+      if (llvm::isa<Float16Type>(type))
+        return (os << "_Float16"), success();
+      else


Please explicitly check for bfloat16; at the pace of current floating point development, I wouldn't be surprised if we get another 16 bit float type in the future.

mgehre-amd · 2024-08-23T09:08:24Z

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

 bool mlir::emitc::isSupportedFloatType(Type type) {
  if (auto floatType = llvm::dyn_cast<FloatType>(type)) {
    switch (floatType.getWidth()) {
+    case 16:


Could you please explicitly check for Float16Type and BFloat16Type here?

mgehre-amd · 2024-08-23T09:09:52Z

Thanks for your PR! Could you please update mlir/docs/Dialects/emitc.md to explain that f16/bf16 will require certain compiler extensions to consume the generated C++ code?

simon-camp · 2024-08-23T09:45:52Z

Can you please update the attribute emission to print the correct literal suffix: godbolt, clang documentation

simon-camp · 2024-08-23T09:50:47Z

mlir/test/Conversion/ArithToEmitC/arith-to-emitc-unsupported.mlir


 // -----

-func.func @arith_cast_bf16(%arg0: bf16) -> i32 {


Can you keep the tests and replace the types with unsupported floating point types, like f128 and f80 (and tf32 is a thing as well, I think)

jacquesguan · 2024-08-26T06:42:38Z

Can you please update the attribute emission to print the correct literal suffix: godbolt, clang documentation

Added fp16 and bf16 to the literal emit logic.

jacquesguan · 2024-08-26T06:45:07Z

Thanks for your PR! Could you please update mlir/docs/Dialects/emitc.md to explain that f16/bf16 will require certain compiler extensions to consume the generated C++ code?

Add to the doc.

cferry-AMD

One small nit, LGTM

mlir/docs/Dialects/emitc.md

CoTinker · 2024-08-26T06:54:25Z

mlir/test/Conversion/MemRefToEmitC/memref-to-emitc-failed.mlir


 // -----

-func.func @unsupported_type_f16() {


Also, keep the tests and replace the types with unsupported floating point types, like f128. But it's not that important. It's up to you.

marbre

Thanks! Just two minor suggestions.

mlir/test/Conversion/MemRefToEmitC/memref-to-emitc-failed.mlir

marbre · 2024-08-26T06:56:32Z

mlir/docs/Dialects/emitc.md

+*   If `__bf16` is used, the code requires complier that supports it, such as:
+    GCC and Clang.


I would probably rephrase to

Suggested change

* If `__bf16` is used, the code requires complier that supports it, such as:

GCC and Clang.

* If `__bf16` is used, the code requires a complier that supports it, such as

GCC or Clang.

jacquesguan · 2024-08-27T02:12:03Z

Thanks! Just two minor suggestions.

Thanks for comment, all addressed.

CoTinker · 2024-08-27T02:15:51Z

LGTM!

cferry-AMD · 2024-08-27T05:51:22Z

mlir/docs/Dialects/emitc.md

    or any of the C++ headers in which the type is defined.
+*   If `_Float16` is used, the code requires the support of C additional
+    floating types.
+*   If `__bf16` is used, the code requires a complier that supports it, such as 


Suggested change

* If `__bf16` is used, the code requires a complier that supports it, such as

* If `__bf16` is used, the code requires a compiler that supports it, such as

typo slipped through

marbre

Thanks, this look good to me. Please fix the typo as pointed to by @cferry-AMD. Beside this the PR should be ready to land. If you need assistance and someone who hits the Squash and merge button for you, let me know.

TinaAMD

Small comment to the doc only, looks good to me otherwise!

TinaAMD · 2024-08-27T06:44:45Z

mlir/docs/Dialects/emitc.md

+*   If `_Float16` is used, the code requires the support of C additional
+    floating types.


I think usually this documentation specifies the type in the mlir world and then relates it to something in C/C++. Maybe

If f16 types are used, the code requires a compiler that supports _Float16.

Same for bf16 below.

jacquesguan requested review from CoTinker, TinaAMD and cferry-AMD August 23, 2024 08:58

llvmbot added mlir mlir:emitc labels Aug 23, 2024

mgehre-amd reviewed Aug 23, 2024

View reviewed changes

mgehre-amd requested a review from marbre August 23, 2024 09:09

simon-camp reviewed Aug 23, 2024

View reviewed changes

jacquesguan force-pushed the mlir-support-f16-bf16 branch from 19f7d82 to 488bd12 Compare August 26, 2024 06:39

mgehre-amd approved these changes Aug 26, 2024

View reviewed changes

cferry-AMD approved these changes Aug 26, 2024

View reviewed changes

mlir/docs/Dialects/emitc.md Outdated Show resolved Hide resolved

CoTinker approved these changes Aug 26, 2024

View reviewed changes

marbre reviewed Aug 26, 2024

View reviewed changes

jacquesguan force-pushed the mlir-support-f16-bf16 branch from 488bd12 to d0adb5c Compare August 27, 2024 02:11

cferry-AMD reviewed Aug 27, 2024

View reviewed changes

cferry-AMD approved these changes Aug 27, 2024

View reviewed changes

marbre approved these changes Aug 27, 2024

View reviewed changes

TinaAMD approved these changes Aug 27, 2024

View reviewed changes

[mlir] Support emit fp16 and bf16 type to cpp

5cd6285

jacquesguan force-pushed the mlir-support-f16-bf16 branch from d0adb5c to 5cd6285 Compare August 27, 2024 07:23

jacquesguan merged commit d4f97da into llvm:main Aug 27, 2024
8 checks passed

		* If `__bf16` is used, the code requires complier that supports it, such as:
		GCC and Clang.

		* If `_Float16` is used, the code requires the support of C additional
		floating types.

[mlir] Support emit fp16 and bf16 type to cpp #105803

[mlir] Support emit fp16 and bf16 type to cpp #105803

Uh oh!

Conversation

jacquesguan commented Aug 23, 2024

Uh oh!

llvmbot commented Aug 23, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mgehre-amd commented Aug 23, 2024

Uh oh!

simon-camp commented Aug 23, 2024

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jacquesguan commented Aug 26, 2024

Uh oh!

jacquesguan commented Aug 26, 2024

Uh oh!

cferry-AMD left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

marbre left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jacquesguan commented Aug 27, 2024

Uh oh!

CoTinker commented Aug 27, 2024

Uh oh!

Choose a reason for hiding this comment

Uh oh!

marbre left a comment

Choose a reason for hiding this comment

Uh oh!

TinaAMD left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

llvmbot commented Aug 23, 2024 •

edited

Loading

marbre left a comment •

edited

Loading