[PATCH] D111806: [LICM] Check the number of divergent paths from loop header to target block

[Philip Reames via Phabricator via llvm-commits]

reames added a comment.

In D111806#3079379 <https://reviews.llvm.org/D111806#3079379>, @modimo wrote:

> To keep the conversation coherent I'll talk about the same feedback for D87551 <https://reviews.llvm.org/D87551> here:
>
> In D111806#3067256 <https://reviews.llvm.org/D111806#3067256>, @reames wrote:
>
>> If we were being presented with hard cases to undo with a cost based transform, I might be willing to entertain the notion that LICM should be non-canonical.  As it stands, I don't see such examples.
>
> The original change that prompted adding this from @wenlei is a more complicated situation https://reviews.llvm.org/D65060#1596899. Specifically, once the values are hoisted out of the 200 switch arms there's not longer a dominating use block to sink them back into. In addition, the block frequency information at that point no longer indicates that there's an issue from the hoisting. Getting MachineLICM to handle that case is complicated and I suspect fragile.

FYI, I find this very unconvincing.  First, the motivating test case appears to have been discussed, but never shared.  Second, as discussed, we appeared to have something along the lines of a switch mapping to address offsets, which is then loaded from.  We can and do pull such geps down through the phi, and that optimization should probably have triggered on the discussed example.  In general, a missing optimization is a *bad* motivation for disabling a canonicalization.


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

https://reviews.llvm.org/D111806