[all-commits] [llvm/llvm-project] 47021b: [IndVars] Option verify-indvars is broken (and alw...

[Max Kazantsev via All-commits]

  Branch: refs/heads/main
  Home:   https://github.com/llvm/llvm-project
  Commit: 47021bda646a4e52027f0956c84a3be5080fbc5e
      https://github.com/llvm/llvm-project/commit/47021bda646a4e52027f0956c84a3be5080fbc5e
  Author: Max Kazantsev <mkazantsev at azul.com>
  Date:   2023-03-14 (Tue, 14 Mar 2023)

  Changed paths:
    M llvm/lib/Transforms/Scalar/IndVarSimplify.cpp

  Log Message:
  -----------
  [IndVars] Option verify-indvars is broken (and always has been), delete it

This option is switched off by default, and it seems that it has never worked correctly.
What it basically does is: it remembers current BECount SCEV, and after all transforms
tries to validate some facts for it. However, between these two points this SCEV may
become invalid (e.g. because some SCEVUnknown it references may be deleted as dead
code). So basically it may work with broken pointers.

Besides, its implementation does strange things (e.g. forgetLoop) which are invasive and
may affect behavior in other parts of the system (specifically verification), concealing some
other problems. Another issue is that it may use SCEVCouldNotCompute object without
checking this.

The option is not used in any unit tests, and if switched on by default, the following tests
fail:
```
********************
Failed Tests (14):
  LLVM :: Transforms/IndVarSimplify/2005-06-15-InstMoveCrash.ll
  LLVM :: Transforms/IndVarSimplify/2007-06-06-DeleteDanglesPtr.ll
  LLVM :: Transforms/IndVarSimplify/2008-10-03-CouldNotCompute.ll
  LLVM :: Transforms/IndVarSimplify/2009-05-24-useafterfree.ll
  LLVM :: Transforms/IndVarSimplify/2011-10-27-lftrnull.ll
  LLVM :: Transforms/IndVarSimplify/ARM/code-size.ll
  LLVM :: Transforms/IndVarSimplify/X86/deterministic-scev-verify.ll
  LLVM :: Transforms/IndVarSimplify/X86/pr57187.ll
  LLVM :: Transforms/IndVarSimplify/X86/verify-scev.ll
  LLVM :: Transforms/IndVarSimplify/bbi-63564.ll
  LLVM :: Transforms/IndVarSimplify/invalidate-modified-lcssa-phi.ll
  LLVM :: Transforms/IndVarSimplify/loop-predication.ll
  LLVM :: Transforms/IndVarSimplify/post-inc-range.ll
  LLVM :: Transforms/IndVarSimplify/turn-to-invariant.ll

********************
Unexpectedly Passed Tests (1):
  LLVM :: Transforms/IndVarSimplify/pr55689.ll
```

None of these looks like real problems found by verification, these are
bugs in the verifying code itself (such as use of deleted SCEVs and
SCEVCouldNotCompute's).

I think it all gives enough justification for its removal.

https://github.com/llvm/llvm-project/issues/61302

Differential Revision: https://reviews.llvm.org/D145894
Reviewed By: nikic


  Commit: c113d0b766983635eafad3c4a4ce93fe32f6146d
      https://github.com/llvm/llvm-project/commit/c113d0b766983635eafad3c4a4ce93fe32f6146d
  Author: Max Kazantsev <mkazantsev at azul.com>
  Date:   2023-03-14 (Tue, 14 Mar 2023)

  Changed paths:
    M llvm/lib/Transforms/Scalar/IndVarSimplify.cpp

  Log Message:
  -----------
  [NFC] Drop NDEBUG around MSSA verification

Proposed on review of https://reviews.llvm.org/D145894.


Compare: https://github.com/llvm/llvm-project/compare/e4ea2d59191c...c113d0b76698