[PATCH] D75145: [PassManager] adjust VectorCombine placement

[Sanjay Patel via Phabricator via llvm-commits]

spatel added a comment.

In D75145#1899652 <https://reviews.llvm.org/D75145#1899652>, @spatel wrote:

> In D75145#1899555 <https://reviews.llvm.org/D75145#1899555>, @fhahn wrote:
>
> > The changes seem relatively save, but I am wondering if the vector combine pass makes the CSE problem more acute? Otherwise it might be better to add the extra EarlyCSE run separately (I'm not sure the name will be quite accurate after the change, it runs quite late now :))
>
>
> Yes, it's more just plain CSE. Several targets like AArch64, AMDGPU, and PowerPC already use that pass even later during target-specific IR codegen, so this isn't even pushing the edge. :)
>  And yes, there are 3 somewhat independent diffs here as mentioned in the description. The addition of CSE is the only 1 that I'm aware of that will result in a test diff. So I can commit the others separately to reduce risk, but I'm not sure how to extract a test diff for those changes.


Not sure if I answered that question well enough: does the vector combine pass makes the CSE problem more acute? Yes - as seen in the IR test diffs, we're potentially creating more CSE opportunities than existed before. In the bug reports, this interferes with later SLP transforms (and that pass may create CSE opportunities itself, but we're not addressing that with this patch).


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

https://reviews.llvm.org/D75145