This is an archive of the discontinued LLVM Phabricator instance.

[mlir] expose standard attributes to C API
ClosedPublic

Authored by ftynse on Aug 18 2020, 7:55 AM.

Details

Summary

Provide C API for MLIR standard attributes. Since standard attributes live
under lib/IR in core MLIR, place the C APIs in the IR library as well (standard
ops will go in a separate library).

Affine map and integer set attributes are only exposed as placeholder types
with IsA support due to the lack of C APIs for the corresponding types.

Integer and floating point attribute APIs expecting APInt and APFloat are not
exposed pending decision on how to support APInt and APFloat.

Diff Detail

Event Timeline

ftynse created this revision.Aug 18 2020, 7:55 AM
Herald added a project: Restricted Project. · View Herald Transcript
ftynse requested review of this revision.Aug 18 2020, 7:55 AM
stellaraccident accepted this revision.Aug 18 2020, 5:42 PM

I could go either way on my comment about style for returning uniqued, backing string data. Feel free to submit as-is and possibly resolve forward if there is a further discussion to be had.

mlir/include/mlir-c/StandardAttributes.h
182

Do we want to make any claims about the lifetime of the pointer passed to the callback? In practice, it is bounded by the containing context.

For these "always return a single chunk" cases that are more about bridging string representations, I do wonder if we should be directly giving access to the backing buffer as in:

// Populates the data buffer and returns its length.
intptr_t mlirStringAttrGetValue(MlirAttribute attr, const char **data);

Practically, with the callback, we aren't restricting that access anyway so maybe go after it more directly?

This revision is now accepted and ready to land.Aug 18 2020, 5:42 PM
ftynse added inline comments.Aug 19 2020, 9:09 AM
mlir/include/mlir-c/StandardAttributes.h
182

I was considering to expose StringRef as struct { const char *, intptr_t }, but StringRef is an LLVM type. This hooks into the other TODOs I left here about APInt and APFloat. These all are LLVM types and we need to decide whether we want to expose them in LLVM C API and make MLIR-C depend on LLVM-C (also check with LLVM folks if they are okay with that), or expose them in MLIR-C as LLVMSupport.h. So I went with the thing we had already pending further discussion. I'll add TODOs here as well and clarify the lifetime.

ftynse updated this revision to Diff 286577.Aug 19 2020, 9:18 AM

Add TODOs regarding StringRef modeling

This revision was landed with ongoing or failed builds.Aug 19 2020, 9:50 AM
This revision was automatically updated to reflect the committed changes.