[all-commits] [llvm/llvm-project] ffcae0: [NFC][InstCombine] Add a test for assume-induced m...

[Roman Lebedev via All-commits]

  Branch: refs/heads/master
  Home:   https://github.com/llvm/llvm-project
  Commit: ffcae008d74c2008697211e66a72ff9a20696bc9
      https://github.com/llvm/llvm-project/commit/ffcae008d74c2008697211e66a72ff9a20696bc9
  Author: Roman Lebedev <lebedev.ri at gmail.com>
  Date:   2019-12-20 (Fri, 20 Dec 2019)

  Changed paths:
    M llvm/test/Transforms/InstCombine/assume.ll

  Log Message:
  -----------
  [NFC][InstCombine] Add a test for assume-induced miscompile

@escape() may throw here, we don't know that assumption, which is located
afterwards in the same block, is executed, therefore %load arg of
call to @escape() can not be marked as non-null.

As noted in D71660 review by @nikic.


  Commit: 92083a295a02f46ecd168438d2145a0ca3c9b6ec
      https://github.com/llvm/llvm-project/commit/92083a295a02f46ecd168438d2145a0ca3c9b6ec
  Author: Roman Lebedev <lebedev.ri at gmail.com>
  Date:   2019-12-20 (Fri, 20 Dec 2019)

  Changed paths:
    M llvm/lib/Analysis/ValueTracking.cpp
    M llvm/test/Transforms/InstCombine/assume.ll

  Log Message:
  -----------
  [ValueTracking] isValidAssumeForContext(): CxtI itself also must transfer execution to successor

This is a pretty rare case, when CxtI and assume are
in the same basic block, with assume being located later.

We were already checking that assumption was guaranteed to be
executed, but we omitted CxtI itself from consideration,
and as the test (miscompile) shows, that is incorrect.

As noted in D71660 review by @nikic.


  Commit: 047186cc986f5bb53ce716dfe363ba517b7d0ed8
      https://github.com/llvm/llvm-project/commit/047186cc986f5bb53ce716dfe363ba517b7d0ed8
  Author: Roman Lebedev <lebedev.ri at gmail.com>
  Date:   2019-12-20 (Fri, 20 Dec 2019)

  Changed paths:
    M llvm/lib/Analysis/ValueTracking.cpp
    M llvm/test/Transforms/InstCombine/assume.ll
    M llvm/test/Transforms/InstSimplify/assume-non-zero.ll

  Log Message:
  -----------
  [ValueTracking] isKnownNonZero() should take non-null-ness assumptions into consideration (PR43267)

Summary:
It is pretty common to assume that something is not zero.
Even optimizer itself sometimes emits such assumptions
(e.g. `addAssumeNonNull()` in `PromoteMemoryToRegister.cpp`).

But we currently don't deal with such assumptions :)
The only way `isKnownNonZero()` handles assumptions is
by calling `computeKnownBits()` which calls `computeKnownBitsFromAssume()`.
But `x != 0` does not tell us anything about set bits,
it only says that there are *some* set bits.
So naturally, `KnownBits` does not get populated,
and we fail to make use of this assumption.

I propose to deal with this special case by special-casing it
via adding a `isKnownNonZeroFromAssume()` that returns boolean
when there is an applicable assumption.

While there, we also deal with other predicates,
mainly if the comparison is with constant.

Fixes [[ https://bugs.llvm.org/show_bug.cgi?id=43267 | PR43267 ]].

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


Compare: https://github.com/llvm/llvm-project/compare/caaacb839950...047186cc986f