Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[MLIR] Support returning IntegerAttr larger than 64 bits to Python #84190

Open
mikeurbach opened this issue Mar 6, 2024 · 5 comments
Open

[MLIR] Support returning IntegerAttr larger than 64 bits to Python #84190

mikeurbach opened this issue Mar 6, 2024 · 5 comments
Labels
mlir:python MLIR Python bindings

Comments

@mikeurbach
Copy link
Contributor

I'm using the MLIR Python bindings in CIRCT to represent some large integers, and while both IntegerAttr and Python can support arbitrary precision integers, the bindings currently use these methods that force the value into an int64_t or uint64_t:

static py::int_ toPyInt(PyIntegerAttribute &self) {
MlirType type = mlirAttributeGetType(self);
if (mlirTypeIsAIndex(type) || mlirIntegerTypeIsSignless(type))
return mlirIntegerAttrGetValueInt(self);
if (mlirIntegerTypeIsSigned(type))
return mlirIntegerAttrGetValueSInt(self);
return mlirIntegerAttrGetValueUInt(self);
}

I'm currently handling this downstream, by calling int(str(myattr)), with a custom __str__ that uses a C API to call APInt's toString. This feels like a hack, so I'm wondering if there is interest in returning large bitwidth IntegerAttrs to Python upstream.

@uenoku pointed me to an old Discourse thread discussing C APIs for APInt here: https://discourse.llvm.org/t/llvm-support-types-in-c-api/1751. That discussion seemed to fizzle out. Is there interest in providing some sort of C API to support this Python binding use case?

@mikeurbach mikeurbach added the mlir:python MLIR Python bindings label Mar 6, 2024
@llvmbot
Copy link
Member

llvmbot commented Mar 6, 2024

@llvm/issue-subscribers-mlir-python

Author: Mike Urbach (mikeurbach)

I'm using the MLIR Python bindings in CIRCT to represent some large integers, and while both IntegerAttr and Python can support arbitrary precision integers, the bindings currently use these methods that force the value into an int64_t or uint64_t: https://github.com/llvm/llvm-project/blob/67c6ad6f30e35c7670bce9bca902caa4b1c8c0e8/mlir/lib/Bindings/Python/IRAttributes.cpp#L452-L459

I'm currently handling this downstream, by calling int(str(myattr)), with a custom __str__ that uses a C API to call APInt's toString. This feels like a hack, so I'm wondering if there is interest in returning large bitwidth IntegerAttrs to Python upstream.

@uenoku pointed me to an old Discourse thread discussing C APIs for APInt here: https://discourse.llvm.org/t/llvm-support-types-in-c-api/1751. That discussion seemed to fizzle out. Is there interest in providing some sort of C API to support this Python binding use case?

@stellaraccident
Copy link
Contributor

I think that discussion happened at the dawn of time, and at the time we were trying to have something (vs getting every detail worked out).

I have to admit that I don't know what the preferred API looks like from a caller's perspective (for python specifically). We could always expose the APInt internal representation, but it would be good to know how hard this is to make glueless with python ints.

As long as this is a slow path for when things need it, it makes sense to me to support in some way. Just not sure how. Unless if done right, it could easily end up being the case that tripping through a str is better.

@mikeurbach
Copy link
Contributor Author

Thanks for weighing in Stella. I agree that a slow path for when things need it would be good to support. I haven't thought too deeply about how to make sure this is "done right"... I'm already able to pass through a str downstream, and I could send a patch to have that slow path upstream, but I guess I'm more interested if anyone has thoughts on how to do it right upstream. It's not a burning need for me, so maybe I'll take some time to think about a design for this and send a proposal.

@stellaraccident
Copy link
Contributor

I guess by "done right", I was acknowledging that I don't actually know what the API looks like on the python side and whether it would mate up. We can build something in the abstract, but for your use case, it seems like it would only be useful if it could be interopped with Python's data structures in a way that is better than str.

@SpriteOvO
Copy link
Member

SpriteOvO commented Apr 3, 2024

Similar to this issue, passing IntegerAttr larger than 64 bits into MLIR is also difficult.

We are building MLIR dialect IRs from external programs, mlirIntegerAttrGet is used to construct constants, however it only accepts a int64_t value as an argument, so constants larger than 64 bits are not possible to be constructed at the moment.

//===----------------------------------------------------------------------===//
// Integer attribute.
//===----------------------------------------------------------------------===//
// TODO: add support for APFloat and APInt to LLVM IR C API, then expose the
// relevant functions here.
/// Checks whether the given attribute is an integer attribute.
MLIR_CAPI_EXPORTED bool mlirAttributeIsAInteger(MlirAttribute attr);
/// Creates an integer attribute of the given type with the given integer
/// value.
MLIR_CAPI_EXPORTED MlirAttribute mlirIntegerAttrGet(MlirType type,
int64_t value);

There is a TODO about APInt for integer attribute C-API.

A string hack workaround might work, by exposing ctor APInt::APInt(unsigned numBits, StringRef str, uint8_t radix) in the downstream, but an upstream complete support would be best.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
mlir:python MLIR Python bindings
Projects
None yet
Development

No branches or pull requests

4 participants