This is an archive of the discontinued LLVM Phabricator instance.

[mlir] Expose value replacement methods through CAPI & Python bindings
Needs ReviewPublic

Authored by mortbopet on Jul 18 2022, 6:52 AM.
This revision needs review, but there are no reviewers specified.

Details

Reviewers
None

Diff Detail

Event Timeline

mortbopet created this revision.Jul 18 2022, 6:52 AM
mortbopet requested review of this revision.Jul 18 2022, 6:52 AM
jdd requested changes to this revision.Jul 19 2022, 2:04 PM

As we discussed offline, I don't think this is the appropriate level to expose through the C API. Some sort of getUses function would be more appropriate IMO.

This revision now requires changes to proceed.Jul 19 2022, 2:04 PM

Looking into this, it seems more difficult than initially thought to expose low-level operations to perform value replacement through the CAPI. We'd expect getUses to return OpOperands (by inheritance, IrOperand) which in turn is what we would call replace on.
Main pain points which i see here are:

  1. OpOperand is heavily encapsulated to protect how and who can construct an operand. https://github.com/llvm/llvm-project/blob/main/mlir/include/mlir/IR/Value.h#L265
  2. By the above, there's no immediate way that we can roundtrip the value through CAPI, which would require that an OpOperand can be constructed statically through an opaque pointer.

There may be a way around this if the constructors are made public. However, that design choice was made for a reason so that would not be a sound decision. Suggestions are welcome, but I don't immediately see any non-intrusive options apart from the one presented in this PR.

This revision now requires review to proceed.Jul 25 2022, 2:29 AM
jdd added a subscriber: jdd.Aug 3 2022, 5:42 PM

I'm pretty sure that OpOperand just needs to implement getAsOpaquePointer/getFromOpaquePointer and it is safe for those implementations to just cast from/to this.

jdd added a comment.Aug 3 2022, 5:58 PM

Or just represent MlirOpOperand as (Operation*, operand index).