[Mlir-commits] [mlir] [MLIR] Fuse locations of hoisted / merged constants (PR #74670)

[Billy Zhu]

zyx-billy wrote:

Thanks for the reviews! Sounds like I can probably split up constant op location fusing & block parent location fusing, but let me elaborate on the motivation since they both are for the same cause.

Like @Mogball said, these locations end up informing the debugger which line of source code is being stepped on, which is a nice proxy for testing whether the location of an op still makes sense*. Any time we do these constant merging, even though procedurally we're erasing ops & moving ops to the front, semantically it's more like saying we're eliminating all the same constants in the block, and creating a unified one at the front of the block to be shared by all their users.

The reason we have this new constant op in the front of the block, is because of all the source lines that contributed to the original constants in this block. That's why I think both "the original constants" and "this block" are equally important in the creation of the new constant (after all, the block dictated the scope & position of this shared constant in the IR). At this point it's basically common setup code for the block (just like you might have language-dependent function setup code at the front of a function, and those carry the location of the function definition).

The parallel optimization in LLVM actually does the same thing (though it achieves it in a different way, perhaps a bit more weird?) for the same reason (https://reviews.llvm.org/D38088):
- When merging constants, it does the same thing by merging locations of the constants.
- When hoisting constants, it merges the location of the _insertion point instruction_.

Location merging in LLVM returns the location of the common parent scope, which will result in a similar behavior as us. Unlike us, it will unfortunately lose the location of the original constant, whereas we can hold both.

\* I'm not sure if this represents a slight semantic shift for what locations mean (I'm hoping it's captured by the original definition), but if it is, I'm also open to considering new concepts for capturing a "hoisted location", or perhaps more broadly a debug location vs. source location distinction if we want to go down that route. LMK what you think. Happy to discuss more!

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