replica
/
hanchenye-llvm-project
2
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

[InstSimplify] do not propagate poison from select arm to icmp user 

This is the cause of the miscompile in:
https://llvm.org/PR50944

The problem has likely existed for some time, but it was made visible with:
5af8bacc94 ( D104661 )
handleOtherCmpSelSimplifications() assumed it can convert select of
constants to bool logic ops, but that does not work with poison.
We had a very similar construct in InstCombine, so the fix here
mimics the fix there.

The bug is in instsimplify, but I'm not sure how to reproduce it outside of
instcombine. The reason this is visible in instcombine is because we have a
hack (FIXME) to bypass simplification of a select when it has an icmp user:
955f125899/llvm/lib/Transforms/InstCombine/InstCombineSelect.cpp (L2632)

So we get to an unusual case where we are trying to simplify an instruction
that has an operand that would have already simplified if we had processed
it in normal order.

Differential Revision: https://reviews.llvm.org/D105298
This commit is contained in:
Sanjay Patel 2021-07-01 17:30:04 -04:00
parent 09e3bf01b3
commit 9eb613b2de
2 changed files with 7 additions and 4 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

7
llvm/lib/Analysis/InstructionSimplify.cpp
View File

@ -189,12 +189,15 @@ static Value *handleOtherCmpSelSimplifications(Value *TCmp, Value *FCmp,
// If the false value simplified to false, then the result of the compare
// is equal to "Cond && TCmp". This also catches the case when the false
// value simplified to false and the true value to true, returning "Cond".
if (match(FCmp, m_Zero()))
// Folding select to and/or isn't poison-safe in general; impliesPoison
// checks whether folding it does not convert a well-defined value into
// poison.
if (match(FCmp, m_Zero()) && impliesPoison(TCmp, Cond))
if (Value *V = SimplifyAndInst(Cond, TCmp, Q, MaxRecurse))
return V;
// If the true value simplified to true, then the result of the compare
// is equal to "Cond || FCmp".
if (match(TCmp, m_One()))
if (match(TCmp, m_One()) && impliesPoison(FCmp, Cond))
if (Value *V = SimplifyOrInst(Cond, FCmp, Q, MaxRecurse))
return V;
// Finally, if the false value simplified to true and the true value to

4
llvm/test/Transforms/InstCombine/icmp.ll
View File

@ -3935,7 +3935,7 @@ bb:
define i1 @thread_cmp_over_select_with_poison_trueval(i1 %b) {
; CHECK-LABEL: @thread_cmp_over_select_with_poison_trueval(
; CHECK-NEXT: ret i1 poison
; CHECK-NEXT: ret i1 false
;
%s = select i1 %b, i32 poison, i32 0
%tobool = icmp ne i32 %s, 0
@ -3944,7 +3944,7 @@ define i1 @thread_cmp_over_select_with_poison_trueval(i1 %b) {
define i1 @thread_cmp_over_select_with_poison_falseval(i1 %b) {
; CHECK-LABEL: @thread_cmp_over_select_with_poison_falseval(
; CHECK-NEXT: ret i1 poison
; CHECK-NEXT: ret i1 true
;
%s = select i1 %b, i32 1, i32 poison
%tobool = icmp ne i32 %s, 0