[all-commits] [llvm/llvm-project] be2cb8: [riscv] Remove mutation of prior vsetvli from inse...

[Philip Reames via All-commits]

  Branch: refs/heads/main
  Home:   https://github.com/llvm/llvm-project
  Commit: be2cb824d0d49a6483f310552dac47a35b72face
      https://github.com/llvm/llvm-project/commit/be2cb824d0d49a6483f310552dac47a35b72face
  Author: Philip Reames <preames at rivosinc.com>
  Date:   2022-05-25 (Wed, 25 May 2022)

  Changed paths:
    M llvm/lib/Target/RISCV/RISCVInsertVSETVLI.cpp
    M llvm/test/CodeGen/RISCV/rvv/vsetvli-insert-crossbb.ll
    M llvm/test/CodeGen/RISCV/rvv/vsetvli-insert.ll

  Log Message:
  -----------
  [riscv] Remove mutation of prior vsetvli from insertion dataflow

This moves mutation entirely out of the main algorithm.

The immediate trigger is that we hit another case of the same issue I thought we'd fixed in 72925d9. It turns out we hadn't considered the cross block case.

As a brief summary, the issue being fixed is that if we mutate a previous vsetvli in phase 3, there's a possibility that some later use of that vsetvli changes "compatibility". In the cross_block_mutate test, this later vsetvli occurs in another block (and is thus visit order dependent too!). This causes us to fail strict asserts. (To be explicit, the current on by default workaround should compensate. It's only when we turn that off that we have problems.)

Now, I want to explicitly call out an alternate workaround. We could leave the mutation in phase 3, and simplify restrict it to the case where the previous vsetvli's GPR result is unused. That covers the case we've actually seen. (I'll note that codegen regressions with a simple form of this were significant. We might have to check specifically for the use outside block case to keep them reasonable, which complicates the workaround slightly.)

Personally, I'm at the point where I want the mutation pulled out just for robustness sake. I'm worried there's yet one more form of this bug we haven't thought about.

The other motivation for this change is that it does give us a couple of minor codegen wins. None appear to be hugely significant, but improvements never hurt right?

Differential Revision: https://reviews.llvm.org/D125270