-
Notifications
You must be signed in to change notification settings - Fork 15.4k
Description
| Bugzilla Link | 31632 |
| Version | trunk |
| OS | All |
| CC | @fhahn,@hfinkel,@aqjune,@regehr,@sanjoy,@rotateright |
Extended Description
The following test case is miscompiled if both SimplifyCFG and InstCombine are run together:
$ cat select.ll
define i1 @g(i8 %x) {
%add_is_nsw = icmp ne i8 %x, 127
br i1 %add_is_nsw, label %is_nsw, label %may_wrap
is_nsw:
%add = add nsw i8 %x, 1
br label %merge
may_wrap:
br label %merge
merge:
%never_poison = phi i8 [ undef, %may_wrap ], [ %add, %is_nsw ]
%result = icmp sgt i8 %never_poison, %x
ret i1 %result
}
$ opt -S -simplifycfg select.ll
define i1 @g(i8 %x) {
%add_is_nsw = icmp ne i8 %x, 127
%add = add nsw i8 %x, 1
%never_poison = select i1 %add_is_nsw, i8 %add, i8 undef
%result = icmp sgt i8 %never_poison, %x
ret i1 %result
}
and with InstCombine:
$ opt -S -simplifycfg -instcombine select.ll
define i1 @g(i8 %x) {
ret i1 true
}
The original, unoptimized, test case returns 0 for %x==127, while the optimized version returns 1.
This bug happens because SimplifyCFG and InstCombine assume different semantics for select with a poison value. SimplifyCFG assumes a select is only poison if its dynamically chosen value is poison, while InstCombine is assuming that select is poison if any of its operands is poison.
We have proposed in the mailing list that select should follow SimplifyCFG's version (search for "Discussion on select" here: http://lists.llvm.org/pipermail/llvm-dev/2016-October/106182.html).
Therefore, Alive says that InstCombine is wrong: http://rise4fun.com/Alive/AI0
(test case from Sanjoy)