[PATCH] D88380: [VPlan] Extend VPValue to also model sub- & 'virtual' values.

[Dave Green via Phabricator via llvm-commits]

dmgreen added a comment.

Hello Florian. Sorry for the delay. I believe that the design of something is the most important part to get right - and the hardest part to change.  I've been trying to take a look at these and the other new patches but I'm not sure I understand yet, likely because I have not had to deal with the complexities it is trying to address. I think my head-canon is simpler than yours, perhaps too simple! It mostly just thinks of things the same as IR instructions, which I like because it's familiar and dependable.

As far as I understand, correct me where I'm wrong:

- Most instruction produce a single value. These are simple enough to deal with, we use a VPValue.
- Most instructions also consume values, for which we use VPUser.
- The complexity comes from interleaving groups which might need to deal with multiple values.
- Interleaving stores do not produce multiple values, but do have multiple operands (?)
- Interleaving loads do need to produce multiple values, in some way.
  - They can either use something like this built into the VPValue class (which I kept getting stuck on having two different type systems in VPValue).
  - They can have the Def/MultiDef from D90558 <https://reviews.llvm.org/D90558> etal (which looks cleaner than the first version I was trying to look at last week, but still involves a lot of little nodes).
  - They could hold a vector of "Members" that are VPUsers and join those together with operands. The complexity gets moved into the VPInterleaveRecipe class and nothing is needed in VPValue/etc

As you might have guessed, I still quite like the last option. But that might well just be because there are somethings about it that I haven't had to deal with. I like how it mirrors the llvm-ir though, it to me seems simple and familiar.

Some other random questions:

- Do you know how Store Interleaving recipes should be handled?
- Do you think that vplan nodes will eventually need types? (From looking at some things I think the answer is probably yes).
- Should we split VPInterleaveRecipe (and maybe VPWidenMemoryInstructionRecipe) into different load and store recipes?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88380