[PATCH] D59363: [SelectionDAG] Add icmp UNDEF handling to SelectionDAG::FoldSetCC

[Simon Pilgrim via Phabricator via llvm-commits]

RKSimon marked an inline comment as not done.
RKSimon added inline comments.


================
Comment at: test/CodeGen/X86/urem-seteq-vec-nonsplat.ll:686
 ; CHECK-SSE:       # %bb.0:
-; CHECK-SSE-NEXT:    pxor %xmm0, %xmm0
-; CHECK-SSE-NEXT:    pcmpeqd %xmm0, %xmm0
-; CHECK-SSE-NEXT:    psrld $31, %xmm0
+; CHECK-SSE-NEXT:    movaps {{.*#+}} xmm0 = [1,1,1,1]
 ; CHECK-SSE-NEXT:    retq
----------------
spatel wrote:
> lebedev.ri wrote:
> > spatel wrote:
> > > RKSimon wrote:
> > > > lebedev.ri wrote:
> > > > > Hmm, but `-instsimplify` says it's `0` https://godbolt.org/z/1dpyQG
> > > > > Either answer is correct?
> > > > Interesting (please can you raise a bug?) - the patch isn't actually changing the result in DAG, its just managing to constant fold.
> > > Probably not necessary? The urem invokes UB:
> > > https://llvm.org/docs/LangRef.html#urem-instruction
> > > 
> > > So it's not that either answer is correct...*any* answer is correct. :)
> > https://bugs.llvm.org/show_bug.cgi?id=41125
> So there are 2 questions of UB/undef/poison:
> 1. The urem causes immediate UB (same as div-by-0), so anything goes in this example.
> 2. Independent of that, what is the output of an icmp with undef operand? PR41125 shows that we have potentially 2 different answers in IR, so we need to decide which behavior is correct/optimal to mimic here in the DAG.
What I think I'm going to do is add the llvm::ConstantFoldCompareInstruction logic to both SimplifyICmpInst (in a new patch) and to FoldSetCC (in this patch):

If the predicate is eq/ne and any operand is undef then return undef - else return a bool constant using isTrueWhenEqual. That way we are consistent for all cases where the other operand is constant/non-constant,

Does that make sense?


Repository:
  rL LLVM

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

https://reviews.llvm.org/D59363