[PATCH] D89566: [LV] Epilogue Vectorization with Optimal Control Flow

Thu Nov 26 01:37:52 PST 2020

fhahn added a comment.

In D89566#2417874 <https://reviews.llvm.org/D89566#2417874>, @SjoerdMeijer wrote:

> Thanks for working on this, I am happy with this patch.
>
> The way I understand it, is that we want to make this VPlan-future-proof, not make any VPlan work more difficult than necessary. Looking at D92132 <https://reviews.llvm.org/D92132>, that doesn't seemed to be the case to me. I.e., even though that patch is work-in-progress, if that patch is representative to achieve that, we don't have anything to worry about. So, I would be happy if this patch lands if we also progress D92132 <https://reviews.llvm.org/D92132> (and friends). But @fhahn can correct me if I am wrong here.

The point I tried to highlight with the patch is that I think we can solve the limitations without subclassing/customising code other than the skeleton creation. In that case, I think it would be preferable to go with a more targeted approach discussed (only providing a custom ‘skeleton’ creator). I outlined the potential drawbacks I see inline, but the main one is that it encourages adding more codegen specialisations that we have to entangle again later.

As mentioned earlier, I don’t think it is worth holding things up due to VPlan, but I think if we have a clear and short path towards addressing the limitations using VPlan, we should do that and avoid broad sub classing.

I might be missing some other cases where full subclassing might be needed. I think that’s something worth further discussing before committing.

On an unrelated note, I think it would be good to have some target-independent tests for this or for some additional targets, so this gets wide coverage on the public bots (some of which might not enable the PPC target)

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89566