[PATCH] D38676: [LV] Model masking in VPlan, introducing VPInstructions

Renato Golin via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Oct 12 08:13:46 PDT 2017


rengolin added a comment.

Hi Ayal/Gil,

This is an interesting pattern and I welcome the docs changes with the code. But the patch is quite big and it's hard to understand what's the actual change and what's a pre-requisite.

AFAICS, there are two main stages here:

1. Groundwork to get the new constructs (VPlanValue/VPlanBuilder/VPlan changes) and necessary changes to LV and ILV.
2. New recipes that use them (VPBlendRecipe/VPWidenMemoryInstructionRecipe) and necessary cleanups in LV/ILV as well as VPlan*.

I'd imagine this patch can be split into at least three blocks:

- New constructs + docs + (I)LV cleanups
- VPBlendRecipe + tests
- VPWidenMemoryInstructionRecipe + tests

but I wouldn't be surprised if you needed some generic cleanups before the first patch and after the last one.

I'm also not expecting a lot of new tests, given that this is just doing better what we already do.

Would it be possible to split the patch, so that we can review them in a more concise way?

Thanks!
--renato


https://reviews.llvm.org/D38676





More information about the llvm-commits mailing list