Skip to content

Conversation

@TGMM
Copy link
Contributor

@TGMM TGMM commented Dec 12, 2024

Added C API functions for the EmitC dialect types.

@llvmbot llvmbot added the mlir label Dec 12, 2024
@llvmbot
Copy link
Member

llvmbot commented Dec 12, 2024

@llvm/pr-subscribers-mlir-emitc

@llvm/pr-subscribers-mlir

Author: Eliud de León (TGMM)

Changes

Added C API functions for the EmitC dialect types.


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

2 Files Affected:

  • (modified) mlir/include/mlir-c/Dialect/EmitC.h (+62)
  • (modified) mlir/lib/CAPI/Dialect/EmitC.cpp (+100)
diff --git a/mlir/include/mlir-c/Dialect/EmitC.h b/mlir/include/mlir-c/Dialect/EmitC.h
index 82e698344bf1e7..384e695e429ef0 100644
--- a/mlir/include/mlir-c/Dialect/EmitC.h
+++ b/mlir/include/mlir-c/Dialect/EmitC.h
@@ -19,6 +19,68 @@ extern "C" {
 
 MLIR_DECLARE_CAPI_DIALECT_REGISTRATION(EmitC, emitc);
 
+//===---------------------------------------------------------------------===//
+// ArrayType
+//===---------------------------------------------------------------------===//
+
+MLIR_CAPI_EXPORTED bool mlirTypeIsAEmitCArrayType(MlirType type);
+
+MLIR_CAPI_EXPORTED MlirTypeID mlirEmitCArrayTypeGetTypeID(void);
+
+MLIR_CAPI_EXPORTED MlirType mlirEmitCArrayTypeGet(intptr_t nDims,
+                                                  int64_t *shape,
+                                                  MlirType elementType);
+//===---------------------------------------------------------------------===//
+// OpaqueType
+//===---------------------------------------------------------------------===//
+
+MLIR_CAPI_EXPORTED bool mlirTypeIsAEmitCOpaqueType(MlirType type);
+
+MLIR_CAPI_EXPORTED MlirTypeID mlirEmitCOpaqueTypeGetTypeID(void);
+
+MLIR_CAPI_EXPORTED MlirType mlirEmitCOpaqueTypeGet(MlirContext ctx,
+                                                   MlirStringRef value);
+
+//===---------------------------------------------------------------------===//
+// PointerType
+//===---------------------------------------------------------------------===//
+
+MLIR_CAPI_EXPORTED bool mlirTypeIsAEmitCPointerType(MlirType type);
+
+MLIR_CAPI_EXPORTED MlirTypeID mlirEmitCPointerTypeGetTypeID(void);
+
+MLIR_CAPI_EXPORTED MlirType mlirEmitCPointerTypeGet(MlirType pointee);
+
+//===---------------------------------------------------------------------===//
+// PtrDiffTType
+//===---------------------------------------------------------------------===//
+
+MLIR_CAPI_EXPORTED bool mlirTypeIsAEmitCPtrDiffTType(MlirType type);
+
+MLIR_CAPI_EXPORTED MlirTypeID mlirEmitCPtrDiffTTypeGetTypeID(void);
+
+MLIR_CAPI_EXPORTED MlirType mlirEmitCPtrDiffTTypeGet(MlirContext ctx);
+
+//===---------------------------------------------------------------------===//
+// SignedSizeTType
+//===---------------------------------------------------------------------===//
+
+MLIR_CAPI_EXPORTED bool mlirTypeIsAEmitCSignedSizeTType(MlirType type);
+
+MLIR_CAPI_EXPORTED MlirTypeID mlirEmitCSignedSizeTTypeGetTypeID(void);
+
+MLIR_CAPI_EXPORTED MlirType mlirEmitCSignedSizeTTypeGet(MlirContext ctx);
+
+//===---------------------------------------------------------------------===//
+// SizeTType
+//===---------------------------------------------------------------------===//
+
+MLIR_CAPI_EXPORTED bool mlirTypeIsAEmitCSizeTType(MlirType type);
+
+MLIR_CAPI_EXPORTED MlirTypeID mlirEmitCSizeTTypeGetTypeID(void);
+
+MLIR_CAPI_EXPORTED MlirType mlirEmitCSizeTTypeGet(MlirContext ctx);
+
 #ifdef __cplusplus
 }
 #endif
diff --git a/mlir/lib/CAPI/Dialect/EmitC.cpp b/mlir/lib/CAPI/Dialect/EmitC.cpp
index 3dcb7038a57981..57619ff776843d 100644
--- a/mlir/lib/CAPI/Dialect/EmitC.cpp
+++ b/mlir/lib/CAPI/Dialect/EmitC.cpp
@@ -10,4 +10,104 @@
 #include "mlir/CAPI/Registration.h"
 #include "mlir/Dialect/EmitC/IR/EmitC.h"
 
+using namespace mlir;
+
 MLIR_DEFINE_CAPI_DIALECT_REGISTRATION(EmitC, emitc, mlir::emitc::EmitCDialect)
+
+//===---------------------------------------------------------------------===//
+// ArrayType
+//===---------------------------------------------------------------------===//
+
+bool mlirTypeIsAEmitCArrayType(MlirType type) {
+  return isa<emitc::ArrayType>(unwrap(type));
+}
+
+MlirTypeID mlirEmitCArrayTypeGetTypeID(void) {
+  return wrap(emitc::ArrayType::getTypeID());
+}
+
+MlirType mlirEmitCArrayTypeGet(intptr_t nDims, int64_t *shape,
+                               MlirType elementType) {
+  return wrap(
+      emitc::ArrayType::get(llvm::ArrayRef(shape, nDims), unwrap(elementType)));
+}
+
+//===---------------------------------------------------------------------===//
+// OpaqueType
+//===---------------------------------------------------------------------===//
+
+bool mlirTypeIsAEmitCOpaqueType(MlirType type) {
+  return isa<emitc::OpaqueType>(unwrap(type));
+}
+
+MlirTypeID mlirEmitCOpaqueTypeGetTypeID(void) {
+  return wrap(emitc::OpaqueType::getTypeID());
+}
+
+MlirType mlirEmitCOpaqueTypeGet(MlirContext ctx, MlirStringRef value) {
+  return wrap(emitc::OpaqueType::get(unwrap(ctx), unwrap(value)));
+}
+
+//===---------------------------------------------------------------------===//
+// PointerType
+//===---------------------------------------------------------------------===//
+
+bool mlirTypeIsAEmitCPointerType(MlirType type) {
+  return isa<emitc::PointerType>(unwrap(type));
+}
+
+MlirTypeID mlirEmitCPointerTypeGetTypeID(void) {
+  return wrap(emitc::PointerType::getTypeID());
+}
+
+MlirType mlirEmitCPointerTypeGet(MlirType pointee) {
+  return wrap(emitc::PointerType::get(unwrap(pointee)));
+}
+
+//===---------------------------------------------------------------------===//
+// PtrDiffTType
+//===---------------------------------------------------------------------===//
+
+bool mlirTypeIsAEmitCPtrDiffTType(MlirType type) {
+  return isa<emitc::PtrDiffTType>(unwrap(type));
+}
+
+MlirTypeID mlirEmitCPtrDiffTTypeGetTypeID(void) {
+  return wrap(emitc::PtrDiffTType::getTypeID());
+}
+
+MlirType mlirEmitCPtrDiffTTypeGet(MlirContext ctx) {
+  return wrap(emitc::PtrDiffTType::get(unwrap(ctx)));
+}
+
+//===---------------------------------------------------------------------===//
+// SignedSizeTType
+//===---------------------------------------------------------------------===//
+
+bool mlirTypeIsAEmitCSignedSizeTType(MlirType type) {
+  return isa<emitc::SignedSizeTType>(unwrap(type));
+}
+
+MlirTypeID mlirEmitCSignedSizeTTypeGetTypeID(void) {
+  return wrap(emitc::SignedSizeTType::getTypeID());
+}
+
+MlirType mlirEmitCSignedSizeTTypeGet(MlirContext ctx) {
+  return wrap(emitc::SignedSizeTType::get(unwrap(ctx)));
+}
+
+//===---------------------------------------------------------------------===//
+// SizeTType
+//===---------------------------------------------------------------------===//
+
+bool mlirTypeIsAEmitCSizeTType(MlirType type) {
+  return isa<emitc::SizeTType>(unwrap(type));
+}
+
+MlirTypeID mlirEmitCSizeTTypeGetTypeID(void) {
+  return wrap(emitc::SizeTType::getTypeID());
+}
+
+MlirType mlirEmitCSizeTTypeGet(MlirContext ctx) {
+  return wrap(emitc::SizeTType::get(unwrap(ctx)));
+}

@TGMM
Copy link
Contributor Author

TGMM commented Dec 12, 2024

I'm not convinced on the CmpPredicate value casting to uint64_t since it removes useful type information. If there's a better way to expose this enum to C, I'd love to hear it.

@TGMM
Copy link
Contributor Author

TGMM commented Dec 20, 2024

Ping

@mgehre-amd
Copy link
Contributor

The code looks good to me! Regarding the CmpPredicate, you could do it like here https://github.com/llvm/llvm-project/blob/main/mlir/lib/CAPI/Dialect/SparseTensor.cpp#L23, where the LevelFormat enum in the SparseTensor dialect is mirrored to C.

@TGMM TGMM force-pushed the tgmm/emitc_c_api_types branch from 0e20365 to 1b498bd Compare December 29, 2024 06:00
@TGMM
Copy link
Contributor Author

TGMM commented Dec 30, 2024

The code looks good to me! Regarding the CmpPredicate, you could do it like here https://github.com/llvm/llvm-project/blob/main/mlir/lib/CAPI/Dialect/SparseTensor.cpp#L23, where the LevelFormat enum in the SparseTensor dialect is mirrored to C.

Thanks for the suggestion! I've made these changes based off the code that you sent.

Copy link
Contributor

@mgehre-amd mgehre-amd left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

Copy link
Member

@marbre marbre left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not familiar with the CAPI but looks pretty good.

@TGMM
Copy link
Contributor Author

TGMM commented Jan 2, 2025

Should I do something about the IsA functions? I tried following the naming convention specified in the guidelines, but I do admit it sounds better (more natural) with An instead.

If not, I think this is ready to be merged.

@marbre
Copy link
Member

marbre commented Jan 2, 2025

Should I do something about the IsA functions? I tried following the naming convention specified in the guidelines, but I do admit it sounds better (more natural) with An instead.

If not, I think this is ready to be merged.

I think IsA is probably fine.

@marbre
Copy link
Member

marbre commented Jan 9, 2025

If not, I think this is ready to be merged.

Do you want one of us to merge on your behalf?

@TGMM
Copy link
Contributor Author

TGMM commented Jan 9, 2025

If not, I think this is ready to be merged.

Do you want one of us to merge on your behalf?

Yes, please!

@marbre
Copy link
Member

marbre commented Jan 13, 2025

If not, I think this is ready to be merged.

Do you want one of us to merge on your behalf?

Yes, please!

This would be merged with an email address only. Do you want to add your name or is just the email fine for you?

@TGMM
Copy link
Contributor Author

TGMM commented Jan 14, 2025

This would be merged with an email address only. Do you want to add your name or is just the email fine for you?

Could you clarify on what you mean by that? I thought it would use my Github info in the merge commit, is that not the case? How would I go on about adding my name? If it's too much trouble, I'm fine with just the email, though.

@marbre
Copy link
Member

marbre commented Jan 14, 2025

This would be merged with an email address only. Do you want to add your name or is just the email fine for you?

Could you clarify on what you mean by that? I thought it would use my Github info in the merge commit, is that not the case? How would I go on about adding my name? If it's too much trouble, I'm fine with just the email, though.

I think it will be authored as TGMM <[email protected]> (https://patch-diff.githubusercontent.com/raw/llvm/llvm-project/pull/119645.patch) if that's fine for you.

@TGMM
Copy link
Contributor Author

TGMM commented Jan 14, 2025

This would be merged with an email address only. Do you want to add your name or is just the email fine for you?

Could you clarify on what you mean by that? I thought it would use my Github info in the merge commit, is that not the case? How would I go on about adding my name? If it's too much trouble, I'm fine with just the email, though.

I think it will be authored as TGMM <[email protected]> (https://patch-diff.githubusercontent.com/raw/llvm/llvm-project/pull/119645.patch) if that's fine for you.

Yeah, that's fine by me. Thanks for the heads up!

@marbre marbre merged commit 43491f0 into llvm:main Jan 14, 2025
8 checks passed
@marbre
Copy link
Member

marbre commented Jan 14, 2025

Yeah, that's fine by me. Thanks for the heads up!

Thanks for your patience and the patch. Merged it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants