[PATCH] D124089: [RISCV] Add a test showing incorrect VSETVLI insertion

[Fraser Cormack via Phabricator via llvm-commits]

frasercrmck added a comment.

In D124089#3463965 <https://reviews.llvm.org/D124089#3463965>, @rogfer01 wrote:

> One interesting thing is that `computeIncomingVLVTYPE` doesn't seem to be fully aligned with what `emitVSETVLIs` will do. If the latter chooses to skip a `vsetvl` then the `Exit` of that basic block might potentially be different to the one that we determined in `computeVLVTYPEChanges` and `computeIncomingVLVTYPE`.
>
> Maybe aligning `computeIncomingVLVTYPE` with the expectations of `emitVSETVLIs` is possible. Looks like once we have computed `InInfo` in `computeIncomingVLVTYPE` we may have to run again `computeVLVTYPEChanges` for that block (and there make sure we use the same skip criteria as in `emitVSETVLIs`). The latter would now receive the `InInfo` (in contrast to Phase 1 where it'd be unknown) and it would compute a potentially different `Exit` value. This also suggests that Phase 1 might be embedded as part of Phase 2 once we have computed the `InInfo`. This might make the algorithm a bit slower.
>
> D119518 <https://reviews.llvm.org/D119518> mitigates the lack of alignment by reconciling both but it means that in your case (if we do `CurInfo = BlockInfo[MBB.getNumber()].Pred;` for the case in which we can skip a `vsetvli` due to the predecessors) we get an unnecessary change at the end of `bb1` which we'd want to have in `bb2` instead.

Sorry for not replying earlier @rogfer01, but I wanted to thank you for your thoughts. After a bit of a break, I am about to dig in to see how best to fix it. The fact that the phases see different information is really not ideal.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124089