[PATCH] D123581: [RISCV] Teach vsetvli insertion to handle PseudoReadVL.

[Philip Reames via Phabricator via llvm-commits]

reames added a comment.

Coming into this a bit late, but been spending a decent amount of time looking at related code recently.

I am generally not a fan of this patch.  The entire PsuedoReadVL mechanism feels like a hack, and the unchecked assumption that the SEW and policy bits on the ReadVL match the prior VLE is worrying.  Beyond that, this adds a decent amount of complexity.

I find myself agreeing with @craig.topper's comment above.  It really feels like we need a pseudo instruction for VLEnFF itself here.  Such a psuedo would produce two outputs, the second being the GPR version of VL.  After this pass, we could lower the pseudo into two instructions (e.g. the actual VLEnFF and the CSR read).  This would make the state update on this patch very minimal.  This really feels to me like the "right" approach here.

An alternative you could pursue is to approximate the simplicity of matching of the pseudo with a local peephole on top of the current split structure.  I mocked up a POC of this idea (https://reviews.llvm.org/D126227).  It needs some cleanup, but its less invasive/risky than this is.  Note that the test diff shows a slightly different impact of this patch.  I'm not sure which is correct.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123581