[PATCH] D142300: [InstSimplify] Simplify `icmp eq/ne/ugt/ult X, 0` -> true/false where `X` is known to be zero.

[Nikita Popov via Phabricator via llvm-commits]

nikic requested changes to this revision.
nikic added a comment.
This revision now requires changes to proceed.

This doesn't make sense to me. If the value is known all zeroes, then the value should fold to zero, at which point the icmp becomes trivial (not necessarily in InstSimplify, but in InstCombine).

The basic pattern from your tests already folds to zero: https://llvm.godbolt.org/z/EWTd4P3aY The pattern with the zero comparison does not: https://llvm.godbolt.org/z/xnv4joe9G

At a guess, this is because we specially handle values used inside return instructions in https://github.com/llvm/llvm-project/blob/8e7c1b9a3d5f08046261ca0b7e5d319a4c4c9b25/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp#L3111 -- we should possibly stop doing that, because it gives a misleading picture of which folds work and which don't.

Ands that fold to constants based on known bits are intended to reliably fold as part of demanded bits simplification here: https://github.com/llvm/llvm-project/blob/8e7c1b9a3d5f08046261ca0b7e5d319a4c4c9b25/llvm/lib/Transforms/InstCombine/InstCombineSimplifyDemanded.cpp#L200

The problem is that the pattern you are testing is special-cased in ValueTracking only: https://github.com/llvm/llvm-project/blob/8e7c1b9a3d5f08046261ca0b7e5d319a4c4c9b25/llvm/lib/Analysis/ValueTracking.cpp#LL1088C75-L1088C75 Because demanded bits simplification reimplements the known bits calculation for performance reasons, this is lost in that case.

The right fix here would be to make sure that the special-case known bits calculation for and is shared between ValueTracking and SimplifyDemanded.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142300