[PATCH] D92402: [InstSimplify] Allow isUndefValue(V) to return true if V is poison

[Juneyoung Lee via Phabricator via llvm-commits]

aqjune created this revision.
aqjune added reviewers: nikic, spatel, lebedev.ri, RKSimon, fhahn, craig.topper, reames, efriedma.
Herald added subscribers: llvm-commits, hiraditya.
Herald added a project: LLVM.
aqjune requested review of this revision.

In the past, isUndefValue()/getWithoutUndef() was added to SimplifyQuery to avoid relying on an assumption that undef was folded into some constant in some cases.

For example, ExpandBinOp uses getWithoutUndef() because this can happen:

(1) Given `undef & (1 ^ 1)`, ExpandBinOp first transforms it into `(undef & 1) ^ (undef & 1)` by distributivity.

(2) A first simplify call answers that the first `(undef & 1)` is 0 whereas another simplify call answers the second `(undef & 1)` is `undef` (this can happen when undefs are hidden under variables)

(3) Since `0 ^ undef` = `undef`, ExpandBinOp returns `undef`.

However, this is incorrect because `undef & (1 ^ 1)` is 0 whereas the result is `undef`.
To avoid this, ExpandBinOp is calling subsequent queries with getWithoutUndef() applied to avoid folding undefs.

With poison, this transformation is now okay: `poison & (1 ^ 1)` is simply `poison`.

This patch

(1) renames isUndefValue() to isUndefOrPoison(), and

(2) let isUndefOrPoison() return true if the value is poison.

Considering the miscompilation due to the old ExpandBinOp in the past (https://reviews.llvm.org/D83360#2145746): if poison is used instead, it now becomes correct.

  define i1 @src(i32 %x, i32 %y) {
  %0:
    %cmp9.not.1 = icmp eq i32 %x, %y
    %cmp15 = icmp slt i32 %x, %y
    %spec.select39 = select i1 %cmp9.not.1, i1 poison, i1 %cmp15
    %spec.select40 = xor i1 %cmp9.not.1, 1
    %spec.select = and i1 %spec.select39, %spec.select40
    ret i1 %spec.select
  }
  =>
  define i1 @tgt(i32 %x, i32 %y) {
  %0:
    %cmp9.not.1 = icmp eq i32 %x, %y
    %cmp15 = icmp slt i32 %x, %y
    %spec.select39 = select i1 %cmp9.not.1, i1 poison, i1 %cmp15
    ret i1 %spec.select39
  }
  Transformation seems to be correct!


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D92402

Files:
  llvm/include/llvm/Analysis/InstructionSimplify.h
  llvm/lib/Analysis/InstructionSimplify.cpp
  llvm/test/Transforms/InstSimplify/select.ll

-------------- next part --------------
A non-text attachment was scrubbed...
Name: D92402.308690.patch
Type: text/x-patch
Size: 13125 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20201201/1d95eecc/attachment.bin>