[mlir][bufferization] Empty tensor elimination for materialize_in_destination

[matthias-springer]

This revision adds support for empty tensor elimination to "bufferization.materialize_in_destination" by implementing the SubsetOpInterface.

Furthermore, the One-Shot Bufferize conflict detection is improved for "bufferization.materialize_in_destination".

[srcarroll]

I might be missing the point of this test, but why is %buffer both src and dst here?

[matthias-springer]

The point of the test is that no OpOperand of this op should bufferize out-of-place. In that case, we would end up with 2 memcpy, when in fact there should be no memcpys (because source and destination are the same).

This is essentially a test for the BufferizableOpInterface implementation of MaterializeInDestinationOp. (Ops that read and write two tensors via separate are usually seen as a RaW conflict and bufferize out-of-place.)

[srcarroll]

would you say the tensor elimination pass would be the only use for this interface? Or do you think this will have other uses later?

[matthias-springer]

The original version of this interface was used only for empty tensor elimination. The new SubsetOpInterface is also used by One-Shot Analysis. In the future, it could also be used by the tiling infrastructure (and then move to mlir/Interfaces).

[matthias-springer]

I rewrote large parts of this stack of commits. InferDestinationOpInterface is now called SubsetOpInterface and has better documentation.

[srcarroll]

most of my questions/comments are about isEquivalentSubset. Obviously I'm having a hard time understanding the structure for this, so I'm hoping to get some clarity on it

[srcarroll]

can you explain what equivalenceFn is in the description. It's unclear to me whether the main logic for determining equivalent subset should be in equivalenceFn or isEquivalentSubset. Maybe I'm just missing something, but seems awkward to have both

[matthias-springer]

I expanded the description in #65619. equivalenceFn returns "true" if two values are equivalent. When used in One-Shot Bufferize, I use AnalysisState::areEquivalentBufferizedValues as equivalenceFn. I didn't want to make this interface dependent on other bufferization stuff (such as AnalysisState), so that it can be moved to mlir/Interfaces in the future.

[srcarroll]

there's context I'm missing since I'm new to some of this stuff, so I won't harp on this much longer. But I guess I still don't see the need for the hook. Why can't isEquivalentSubset be implemented without it? For example, the default could be

bool isEquivalentSubset(Value candidate) {
  return getSubset() == candidate;
 }

(sorry if this is a naive question)

[matthias-springer]

This would work for bufferization.materialize_in_destination because the subset is simply the destination tensor. But getSubset may build new IR. E.g., for tensor.insert_slice, it will build a new tensor.extract_slice op and return that one. I'm going to rename getSubset to buildSubset.

[srcarroll]

It seems a little clunky to solely use isEquivalentSubset for determining if "same". I understand that two subsets are "same" only if they are equivalent, so it makes sense to use isEquivalentSubset as part of the logic for this function. But it seems like you're trying to shove the extra logic for "same" into the equivalencFn hook. Although I may be getting this wrong as I'm not entirely sure of the purpose of the hook (see comment above).

[matthias-springer]

I wanted to make equivalenceFn optional with [](Value v1, Value v2) { return v1 == v2; } as as default value for the parameter. But interface methods cannot have optional parameters, so I had to add another function. (Note this is not an InterfaceMethod and cannot be implemented.)

v1 == v2 is the simplest kind of equivalence function. isSameSubset can be used when no equivalence information (e.g., in the form of an AnalysisState) is available.

[srcarroll]

I see this is used to attach an interface to InsertSliceOp, which I admit I'm unfamiliar with attaching interfaces. So this is probably a dumb question, but why not just add the SubsetOpInterface trait to the InsertSliceOp instead?

[matthias-springer]

This is so that the MLIRTensorDialect build target does not have to depend on MLIRBufferizationDialect. That's also why the BufferizableOpInterface implementations in this file are implemented as external models. It also keeps TensorOps.cpp and TensorOps.td a bit simpler.