[PATCH] D112711: [fir] Add FIR CSE specific pass

[Valentin Clement via Phabricator via llvm-commits]

clementval added a comment.

In D112711#3104689 <https://reviews.llvm.org/D112711#3104689>, @mehdi_amini wrote:

> In D112711#3102388 <https://reviews.llvm.org/D112711#3102388>, @clementval wrote:
>
>> In D112711#3098053 <https://reviews.llvm.org/D112711#3098053>, @mehdi_amini wrote:
>>
>>> Right so I'm interested in what it'll take to converge this with the existing CSE pass. What you mentioned here is interesting because it seems all in the domain of memory effects I believe: does it point mostly to the lack of support for the memory effects op interface in the CSE pass?
>>
>> I think what difference we have right now with the memory effects op interface is that in the case of the memory effects op interface it applies to a op all the time. In our case, the `fir.call` can be `pure` or not. I'm not sure how this would work with the memory effects.
>
> When an an operation implements the interface, it can write any kind of C++, including looking up attributes on the operation. This is very much a query made on a specific instance of the operation and not a query that has to be static on the op class I believe.
>
> See for example:
>
> - The op definition here: https://github.com/llvm/llvm-project/blob/main/mlir/test/lib/Dialect/Test/TestOps.td#L2058 (the line after shows the support for custom non-memory effects by the way)
> - The interface implementation for the op: https://github.com/llvm/llvm-project/blob/main/mlir/test/lib/Dialect/Test/TestDialect.cpp#L887-L921
> - The associated test that shows how the same op can have different effects based on the attributes: https://github.com/llvm/llvm-project/blob/main/mlir/test/IR/test-side-effects.mlir

Thanks for the pointers. I'll have a look at that.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D112711/new/

https://reviews.llvm.org/D112711