[Mlir-commits] [mlir] [MLIR] Fuse parent region location when hoisting constants (PR #75258)

[Billy Zhu]

zyx-billy wrote:

> Why is this different from other CSE cases (or other code motion)?

Oh do you mean other CSE or code motion in general? I don't think this one's special. I'm just starting with this case, and if we get to an agreed practice for managing locations during code motion & CSE, we can apply it to other optimizations.

Compared to the [previous PR](https://github.com/llvm/llvm-project/pull/74670), the difference is this handles moving & materializing ops, while the previous one only handled deduplicating ops. It's likely these happen together, but they can also occur individually (the tests added in this PR checks that).

>> the constant op that is moved (or materialized elsewhere) should have its location fused with the location of the parent region of its insertion point.
>
> Is this description actually correct? Or is it taking advantage that we always materialize at the beginning of the entry block of a region?

Right, this is only correct because we move to / materialize at the beginning of the entry block in this optimization. Other patterns would need to be considered separately.

> what if there are block-arguments with locations that have themselves a lowering that will insert operations at the beginning of the block? Aren't you creating a worsen problem?

If we settle on the convention that a "prologue" of a region (all the user-invisible/compiler-generated setup instructions) should carry the location of the parent region, then the lowerings for the block arguments should also make sure to use the parent region location (if the arguments don't already carry the parent location).


https://github.com/llvm/llvm-project/pull/75258