[PATCH] D156532: [Pipelines] Perform hoisting prior to GVN

Dave Green via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Aug 8 10:04:25 PDT 2023


dmgreen added a comment.

In D156532#4569686 <https://reviews.llvm.org/D156532#4569686>, @nikic wrote:

> The issue for x264 is that the earlier hoisting reduces the size of the loop body enough for it to be unrolled, and SLP doesn't vectorize this case. This is the IR before slp-vectorize: https://gist.github.com/nikic/6ccde4b5320f7f4a6c5e7bd3ff8db1f6
>
> Quite annoying, because the earlier hoisting would otherwise be a clear improvement for that test case.

Yeah I agree, it doesn't feel like the greatest reason but phase ordering can be difficult and this is a fairly large regression in what people keep telling me is a fairly important benchmark.

In D156532#4569747 <https://reviews.llvm.org/D156532#4569747>, @nikic wrote:

> I've added a phase ordering test for quant_4x4 in https://github.com/llvm/llvm-project/commit/b92711931daf45426a23c082c732ddfbf6d02814. Moving the SimplifyCFG run until after LPM2 should avoid this issue, not sure whether that would cause other problems.

Thanks. I guess there are maybe two things going on. The unrolling of loops with conditions creates something this is not easy to deal with in the SLP vectorizer (multiple blocks), and the SLP vectorizer doesn't know how to add runtime alias checks.

I can try moving the simplifycfg run later and see what effect it has. There might also be something that we can do about not unrolling with multiple blocks that are not going to be simplified.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156532



More information about the llvm-commits mailing list