[all-commits] [llvm/llvm-project] 9d70db: [InstCombine] use poison as placeholder for undema...

[Juneyoung Lee via All-commits]

  Branch: refs/heads/main
  Home:   https://github.com/llvm/llvm-project
  Commit: 9d70dbdc2bf294abffd4b2c9ae524055f72d017a
      https://github.com/llvm/llvm-project/commit/9d70dbdc2bf294abffd4b2c9ae524055f72d017a
  Author: Juneyoung Lee <aqjune at gmail.com>
  Date:   2020-12-28 (Mon, 28 Dec 2020)

  Changed paths:
    M clang/test/CodeGen/SystemZ/builtins-systemz-zvector-constrained.c
    M clang/test/CodeGen/SystemZ/builtins-systemz-zvector.c
    M clang/test/CodeGen/SystemZ/builtins-systemz-zvector2-constrained.c
    M clang/test/CodeGen/SystemZ/builtins-systemz-zvector2.c
    M llvm/lib/Transforms/InstCombine/InstCombineSimplifyDemanded.cpp
    M llvm/test/CodeGen/AMDGPU/vector-alloca-bitcast.ll
    M llvm/test/Transforms/InstCombine/AArch64/tbl1.ll
    M llvm/test/Transforms/InstCombine/ARM/tbl1.ll
    M llvm/test/Transforms/InstCombine/X86/clmulqdq.ll
    M llvm/test/Transforms/InstCombine/X86/x86-addsub-inseltpoison.ll
    M llvm/test/Transforms/InstCombine/X86/x86-addsub.ll
    M llvm/test/Transforms/InstCombine/X86/x86-avx512-inseltpoison.ll
    M llvm/test/Transforms/InstCombine/X86/x86-avx512.ll
    M llvm/test/Transforms/InstCombine/X86/x86-insertps.ll
    M llvm/test/Transforms/InstCombine/X86/x86-masked-memops.ll
    M llvm/test/Transforms/InstCombine/X86/x86-muldq.ll
    M llvm/test/Transforms/InstCombine/X86/x86-pack-inseltpoison.ll
    M llvm/test/Transforms/InstCombine/X86/x86-pack.ll
    M llvm/test/Transforms/InstCombine/X86/x86-pshufb.ll
    M llvm/test/Transforms/InstCombine/X86/x86-sse-inseltpoison.ll
    M llvm/test/Transforms/InstCombine/X86/x86-sse.ll
    M llvm/test/Transforms/InstCombine/X86/x86-sse2-inseltpoison.ll
    M llvm/test/Transforms/InstCombine/X86/x86-sse2.ll
    M llvm/test/Transforms/InstCombine/X86/x86-sse41-inseltpoison.ll
    M llvm/test/Transforms/InstCombine/X86/x86-sse41.ll
    M llvm/test/Transforms/InstCombine/X86/x86-sse4a.ll
    M llvm/test/Transforms/InstCombine/X86/x86-vec_demanded_elts-inseltpoison.ll
    M llvm/test/Transforms/InstCombine/X86/x86-vec_demanded_elts.ll
    M llvm/test/Transforms/InstCombine/X86/x86-vector-shifts-inseltpoison.ll
    M llvm/test/Transforms/InstCombine/X86/x86-vector-shifts.ll
    M llvm/test/Transforms/InstCombine/X86/x86-vpermil.ll
    M llvm/test/Transforms/InstCombine/X86/x86-xop-inseltpoison.ll
    M llvm/test/Transforms/InstCombine/X86/x86-xop.ll
    M llvm/test/Transforms/InstCombine/bitcast-bigendian.ll
    M llvm/test/Transforms/InstCombine/bitcast-inseltpoison.ll
    M llvm/test/Transforms/InstCombine/bitcast-vec-canon-inseltpoison.ll
    M llvm/test/Transforms/InstCombine/bitcast-vec-canon.ll
    M llvm/test/Transforms/InstCombine/bitcast.ll
    M llvm/test/Transforms/InstCombine/broadcast-inseltpoison.ll
    M llvm/test/Transforms/InstCombine/broadcast.ll
    M llvm/test/Transforms/InstCombine/cast.ll
    M llvm/test/Transforms/InstCombine/getelementptr.ll
    M llvm/test/Transforms/InstCombine/insert-const-shuf.ll
    M llvm/test/Transforms/InstCombine/insert-extract-shuffle-inseltpoison.ll
    M llvm/test/Transforms/InstCombine/insert-extract-shuffle.ll
    M llvm/test/Transforms/InstCombine/masked_intrinsics-inseltpoison.ll
    M llvm/test/Transforms/InstCombine/masked_intrinsics.ll
    M llvm/test/Transforms/InstCombine/minmax-fold.ll
    M llvm/test/Transforms/InstCombine/pr2645-0.ll
    M llvm/test/Transforms/InstCombine/shuffle_select.ll
    M llvm/test/Transforms/InstCombine/shufflevector-div-rem-inseltpoison.ll
    M llvm/test/Transforms/InstCombine/shufflevector-div-rem.ll
    M llvm/test/Transforms/InstCombine/sub-of-negatible.ll
    M llvm/test/Transforms/InstCombine/trunc.ll
    M llvm/test/Transforms/InstCombine/vec_demanded_elts-inseltpoison.ll
    M llvm/test/Transforms/InstCombine/vec_demanded_elts.ll
    M llvm/test/Transforms/InstCombine/vec_shuffle-inseltpoison.ll
    M llvm/test/Transforms/InstCombine/vec_shuffle.ll
    M llvm/test/Transforms/InstCombine/vector-casts-inseltpoison.ll
    M llvm/test/Transforms/InstCombine/vector-casts.ll
    M llvm/test/Transforms/InstCombine/vector_insertelt_shuffle-inseltpoison.ll
    M llvm/test/Transforms/InstCombine/vector_insertelt_shuffle.ll
    M llvm/test/Transforms/LoopVectorize/X86/invariant-load-gather.ll
    M llvm/test/Transforms/LoopVectorize/induction.ll
    M llvm/test/Transforms/LoopVectorize/reduction-inloop-pred.ll
    M llvm/test/Transforms/LoopVectorize/reduction-inloop.ll
    M llvm/test/Transforms/PhaseOrdering/X86/horiz-math-inseltpoison.ll
    M llvm/test/Transforms/PhaseOrdering/X86/horiz-math.ll
    M llvm/test/Transforms/SLPVectorizer/AArch64/getelementptr.ll
    M llvm/test/Transforms/SLPVectorizer/X86/alternate-cast-inseltpoison.ll
    M llvm/test/Transforms/SLPVectorizer/X86/alternate-cast.ll
    M llvm/test/Transforms/SLPVectorizer/X86/alternate-int-inseltpoison.ll
    M llvm/test/Transforms/SLPVectorizer/X86/blending-shuffle-inseltpoison.ll

  Log Message:
  -----------
  [InstCombine] use poison as placeholder for undemanded elems

Currently undef is used as a don’t-care vector when constructing a vector using a series of insertelement.
However, this is problematic because undef isn’t undefined enough.
Especially, a sequence of insertelement can be optimized to shufflevector, but using undef as its placeholder makes shufflevector a poison-blocking instruction because undef cannot be optimized to poison.
This makes a few straightforward optimizations incorrect, such as:

```
;  https://bugs.llvm.org/show_bug.cgi?id=44185

define <4 x float> @insert_not_undef_shuffle_translate_commute(float %x, <4 x float> %y, <4 x float> %q) {
  %xv = insertelement <4 x float> %q, float %x, i32 2
  %r = shufflevector <4 x float> %y, <4 x float> %xv, <4 x i32> { 0, 6, 2, undef }
  ret <4 x float> %r ; %r[3] is undef
}
=>
define <4 x float> @insert_not_undef_shuffle_translate_commute(float %x, <4 x float> %y, <4 x float> %q) {
  %r = insertelement <4 x float> %y, float %x, i32 1
  ret <4 x float> %r ; %r[3] = %y[3], incorrect if %y[3] = poison
}

Transformation doesn't verify!
ERROR: Target is more poisonous than source
```

I’d like to suggest
1. Using poison as insertelement’s placeholder value (IRBuilder::CreateVectorSplat should be patched too)
2. Updating shufflevector’s semantics to return poison element if mask is undef

Note that poison is currently lowered into UNDEF in SelDag, so codegen part is okay.
m_Undef() matches PoisonValue as well, so existing optimizations will still fire.

The only concern is hidden miscompilations that will go incorrect when poison constant is given.
A conservative way is copying all tests having `insertelement undef` & replacing it with `insertelement poison` & run Alive2 on it, but it will create many tests and people won’t like it. :(

Instead, I’ll simply locally maintain the tests and run Alive2.
If there is any bug found, I’ll report it.

Relevant links: https://bugs.llvm.org/show_bug.cgi?id=43958 , http://lists.llvm.org/pipermail/llvm-dev/2019-November/137242.html

Reviewed By: nikic

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