[all-commits] [llvm/llvm-project] 97a4e7: [InstCombine] remove a buggy set of zext-icmp tran...

[RotateRight via All-commits]

  Branch: refs/heads/main
  Home:   https://github.com/llvm/llvm-project
  Commit: 97a4e7b7ff9ffdd005517c04680c1a73b17928ea
      https://github.com/llvm/llvm-project/commit/97a4e7b7ff9ffdd005517c04680c1a73b17928ea
  Author: Sanjay Patel <spatel at rotateright.com>
  Date:   2021-09-09 (Thu, 09 Sep 2021)

  Changed paths:
    M llvm/lib/Transforms/InstCombine/InstCombineCasts.cpp
    M llvm/lib/Transforms/InstCombine/InstCombineInternal.h
    M llvm/test/Transforms/InstCombine/zext-or-icmp.ll
    M llvm/test/Transforms/InstCombine/zext.ll

  Log Message:
  -----------
  [InstCombine] remove a buggy set of zext-icmp transforms

The motivating case is an infinite loop shown with a reduced test from:
https://llvm.org/PR51762

To solve this, I'm proposing we delete the most obviously broken part of this code.

The bug example shows a fundamental problem: we ask computeKnownBits if a transform
will be profitable, alter the code by creating new instructions, then rely on
computeKnownBits to return the same answer to actually eliminate instructions.

But there's no guarantee that the results will be the same between the 1st and 2nd
calls. In the infinite loop example, we get different answers, so we add
instructions that conflict with some other transform, and we're stuck.

There's at least one other problem visible in the test diff for
`@zext_or_masked_bit_test_uses`: the code doesn't check uses properly, so we can
end up with extra instructions created.

Last, it's not clear if this set of transforms actually improves analysis or
codegen. I spot-checked a few targets and don't see a clear win:
https://godbolt.org/z/x87EWovso

If we do see a regression from this change, codegen seems like the right place to
add a cmp -> bit-hack fold.

If this is too big of a step, we could limit the computeKnownBits calls by not
passing a context instruction and/or limiting the recursion. I checked that those
would stop the infinite loop for PR51762, but that won't guarantee that some other
example does not fall into the same loop.

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