The MemRefDataFlow pass does store to load forwarding only for affine store/loads. This patch updates the pass to use affine read/write interface which enables vector forwarding.
This looks good - thanks for contributing. A bunch of minor doc related comments.
Nit: stored value -> value to be stored
|2 ↗||(On Diff #279810)|
Please add a line on what these test cases are testing.
|32–35 ↗||(On Diff #279810)|
Please indent suitably.
|50–51 ↗||(On Diff #279810)|
You may want to capture the store value and check if the add operand is that.
|53 ↗||(On Diff #279810)|
I don't understand what "underlying value" means in this context. Do you mean the value read by this operation?
Nit: return cast<ConcreteOp>(this->getOperaiton()); looks more concise. return this->getOperation()->getResult(0); looks equally concise and arguably clearer about the intent as opposed to an explicit cast to ConcreteOp followed by an implicit cast to Value.
|1 ↗||(On Diff #280071)|
Why can't this live in mlir/test/Transforms/memref-dataflow-opt.mlir?
These are _operation_ interfaces, they don't represent values. The fact that single-result operations are implicitly convertible to values should not affect this. So "the value read by this operation" sounds like the right formulation here.
The OneResult should automatically be added by the generator. You could verify this. Doing .getResult() should already give you the loaded value, right? Why do we need this?
I don't think there is an easy way. Some time ago, I had a prototype that inherited the interface trait class from another trait class (for FunctionLike), but the code to support that was quite convoluted. That being said, having cast<ConcreteOp>(this->getOperation().getResult() should dispatch properly, without having to know if ConcreteOp has the trait or if it got the method from elsewhere. If you want just a check, something like static_assert(std::is_base_of<InterfaceName::Trait, ConcreteOp>::value, "") should do the trick.
The OneResult trait is automatically added to the concrete ops but not to the interfaces. My hope was that we could add OneResult to the interface and then get rid of the getValue() method altogether and replace it with .getResult() instead. However, that doesn't seem to be possible right now so... fair enough! Thanks for the info!