[PATCH] D147336: [IVDescriptors] Add pointer InductionDescriptors with non-constant strides (try 2)

Fri Mar 31 10:15:29 PDT 2023

dmgreen added a comment.

Hello. About the performance changes - I have been looking into them a little. A lot of it looks good, especially where gather/scatter are available which this can make use of. There are some cases where it wasn't doing quite as well under some places in MVE, but https://reviews.llvm.org/D147331 should hopefully sort out the largest of them. Sam/Nick should be back on Monday to review it. We have some other cases where gather/scatter are not available for whatever reason and those don't look as good but the changes are much smaller. Again it likely comes down to adjusting the costmodel, perhaps for the cost of the scalarized loads in getMemInstScalarizationCost needs to be a little higher under MVE. I wouldn't consider any of that blockers considering all the improvements.

The other part under AArch64 is a bit more silly I'm afraid, and falls into your Option 3. It is extracted from the code in x264, using the 16x16 version: https://github.com/mirror/x264/blob/master/common/pixel.c#L53, which I believe is also a part of Spec. I was thinking of adding a phase-ordering test but will just put it here: https://godbolt.org/z/qo54f7Pbv. It now decides to version the loop based on a stride of 1, like you mentioned. The loop-vectorized code is not as good as the slp version though, and never executed. I wouldn't expect that to cause a lot of slowdown, it only really adding an extra compare/branch and some dead code. The differences seemed higher that I expected in places though, around 25% for that function in isolation on some cpus. That might be quite noisy though as some were a lot closer to 0.

I might also be able to provide another example of an assert this is still hitting, with `Assertion `Instance.Lane.isFirstLane() && "cannot get lane > 0 for scalar"' failed`. It is reducing now.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147336