Miscompile, probably due to DAGCombiner turning SELECT into AND even if the SELECT operands may be poison

[bjope]

Given a test case like this:

define i16 @foo(i32 %x) {
  %cmp1 = icmp sgt i32 %x, 0
  %sub = sub nsw i32 2147483647, %x
  %cmp2 = icmp sgt i32 %x, %sub
  %r = select i1 %cmp1, i1 %cmp2, i1 false
  %zext = zext i1 %r to i16
  ret i16 %zext
}

we started to see miscompiles after improvements to ValueTracking/KnownBits in 17162b6

Here is a godbolt link using aarch64 as an example (need mattr=+cssc to get a legal SMAX for i32): https://godbolt.org/z/8zxa4YEs5

When using llc -debug-only=isel,dagcombine -mtriple=aarch64 -mattr=+cssc one can see that we get an initial DAG like this:

Initial selection DAG: %bb.0 'foo:'
SelectionDAG has 16 nodes:
  t0: ch,glue = EntryToken
  t2: i32,ch = CopyFromReg t0, Register:i32 %0
    t5: i1 = setcc t2, Constant:i32<0>, setgt:ch
      t7: i32 = sub nsw Constant:i32<2147483647>, t2
    t8: i1 = setcc t2, t7, setgt:ch
  t10: i1 = select t5, t8, Constant:i1<0>
  t11: i16 = zero_extend t10
    t12: i32 = zero_extend t10
  t14: ch,glue = CopyToReg t0, Register:i32 $w0, t12
  t15: ch = AArch64ISD::RET_GLUE t14, Register:i32 $w0, t14:1

Then the DAGCombiner (foldBoolSelectToLogic) will do

Combining: t10: i1 = select t5, t8, Constant:i1<0>
 ... into: t16: i1 = and t5, t8
 
SelectionDAG has 14 nodes:
  t0: ch,glue = EntryToken
  t2: i32,ch = CopyFromReg t0, Register:i32 %0
        t5: i1 = setcc t2, Constant:i32<0>, setgt:ch
          t7: i32 = sub nsw Constant:i32<2147483647>, t2
        t8: i1 = setcc t2, t7, setgt:ch
      t16: i1 = and t5, t8
    t12: i32 = zero_extend t16
  t14: ch,glue = CopyToReg t0, Register:i32 $w0, t12
  t15: ch = AArch64ISD::RET_GLUE t14, Register:i32 $w0, t14:1

which I think might be wrong depending on if we see such an AND as a logical/bitwise AND (or rather if it blocks poison or not).

What happens next is that that the AND is combined into a SMAX and a new SETCC:

SelectionDAG has 13 nodes:
  t0: ch,glue = EntryToken
  t2: i32,ch = CopyFromReg t0, Register:i32 %0
          t7: i32 = sub nsw Constant:i32<2147483647>, t2
        t17: i32 = smax t7, Constant:i32<0>
      t19: i1 = setcc t17, t2, setlt:ch
    t12: i32 = zero_extend t19
  t14: ch,glue = CopyToReg t0, Register:i32 $w0, t12
  t15: ch = AArch64ISD::RET_GLUE t14, Register:i32 $w0, t14:1

And here it is clear that if the sub result in poison, then the result is poison as well.

On a side note. It is a bit sad that the last couple of years more and more things that can "produce poison" (such as nsw/nuw/nneg) are being exposed in the selection DAG, while I think we lack lots of documentation about what the rules are when it comes to dealing with poison in the selection DAG.

[bjope]

I forgot to write that the difference we see, after the improvement in ValueTracking in 17162b6, is that the SMAX will be folded away.
So after type legalization we just get

Optimized type-legalized selection DAG: %bb.0 'foo:'
SelectionDAG has 10 nodes:
  t0: ch,glue = EntryToken
  t2: i32,ch = CopyFromReg t0, Register:i32 %0
      t22: i32 = xor t2, Constant:i32<2147483647>
    t23: i32 = setcc t22, t2, setlt:ch
  t14: ch,glue = CopyToReg t0, Register:i32 $w0, t23
  t15: ch = AArch64ISD::RET_GLUE t14, Register:i32 $w0, t14:1

while we used to get

Optimized type-legalized selection DAG: %bb.0 'foo:'
SelectionDAG has 12 nodes:
  t0: ch,glue = EntryToken
  t2: i32,ch = CopyFromReg t0, Register:i32 %0
        t22: i32 = xor t2, Constant:i32<2147483647>
      t17: i32 = smax t22, Constant:i32<0>
    t23: i32 = setcc t17, t2, setlt:ch
  t14: ch,glue = CopyToReg t0, Register:i32 $w0, t23
  t15: ch = AArch64ISD::RET_GLUE t14, Register:i32 $w0, t14:1

The smax is needed to get correct behavior when the sub (later rewritten as xor) has signed wrap.