[PATCH] D103458: [SLP]Improve gathering of scalar elements.

[Guozhi Wei via Phabricator via llvm-commits]

Carrot added a comment.

The problem is exactly in the merging of two undef into poison.

In our original code, the function foo is actually a class constructor, it initializes two bit fields

  entry:
     ...
     br %cond, label %then, label %bug
  
  then:
     call baz()
     br label %bug
  
  bug:
    word1 = load i64 bitfield1                          
    word1.clear = and i64 word1, mask1
    word1.set = or i64 word1.clear, value1            // set bitfield2
    store i64 word1.set, bitfield1                    // store bitfield1
    word2 = load i64 bitfiled2
    word2.clear = and i64 word2, mask2
    word2.set = or i64 word2.clear, value2           // set bitfield2
    store i64 word2.set, bitfield2                   // store bitfield2
    bar()
    ret
  
  Notice that word1 is a 64b integer which contains bitfield1 and some other bitfields, 
  since it is in constructor, these fields have not been initialized, so word1 is actually undef, 
  LLVM doesn't recognize it because there is a function call in block %then (without precise 
  alias analysis). There is similar conclusion with word2.
  
  In GVN pass, it has the knowledge that word1 must have undef value from control flow
  %entry -> %bug, so the load instruction can be deleted in this path, but not sure from the
   control flow %then -> %bug, so it must keep the load in block %then, similar analysis applies
  to word2, so it changes the IR into the form 
  
  entry:
    %nb = alloca %S, align 8
    br i1 %cond, label %then, label %bug
  
  then:
    call void @baz()
    %ptr_2 = getelementptr inbounds %S, %S* %nb, i64 0, i32 0
    %load2 = load i64, i64* %ptr_2, align 8
    %bf_1 = getelementptr inbounds %S, %S* %nb, i64 0, i32 1
    %ptr_1 = bitcast i40* %bf_1 to i64*
    %load1 = load i64, i64* %ptr_1, align 8
    br label %bug
  
  bug:
    %word1 = phi i64 [ %load1, %then ], [ undef, %entry ]
    %word2 = phi i64 [ %load2, %then ], [ undef, %entry ]
    %ptr2 = getelementptr inbounds %S, %S* %nb, i64 0, i32 0
    %word2.clear = and i64 %word2, -1152921504606846976
    %word2.set = or i64 %word2.clear, 1
    store i64 %word2.set, i64* %ptr2, align 8
    %bf1 = getelementptr inbounds %S, %S* %nb, i64 0, i32 1
    %ptr1 = bitcast i40* %bf1 to i64*
    %word1.clear = and i64 %word1, -68719476736
    %word1.set = or i64 %word1.clear, 21
    store i64 %word1.set, i64* %ptr1, align 8
    call void @bar()
    br label %exit

So the two undef values are actually correct and reasonable. But a poison value makes the code completely wrong.
According to https://llvm.org/docs/LangRef.html#poison-values, A poison value is a result of an erroneous operation.
There is no erroneous operation here.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103458