Quuxplusone / LLVMBugzillaTest

0 stars 0 forks source link

Missing redundancy elimination for uadd.with.overflow and manual overflow check #39817

Open Quuxplusone opened 5 years ago

Quuxplusone commented 5 years ago
Bugzilla Link PR40846
Status NEW
Importance P enhancement
Reported by Nikita Popov (nikita.ppv@gmail.com)
Reported on 2019-02-24 10:18:12 -0800
Last modified on 2019-12-11 11:53:31 -0800
Version trunk
Hardware All All
CC alex.gaynor@gmail.com, comexk@gmail.com, llvm-bugs@lists.llvm.org, nelhage@nelhage.com
Fixed by commit(s) rG8db5143b1a1521915c842ebef23cb9fe8fe607ce
Attachments
Blocks
Blocked by
See also
Originally reported at: https://github.com/rust-lang/rust/issues/58692

The following IR remains unchanged under opt -O3:

define i1 @test(i64 %x, i64 %y) nounwind {
  %a = call { i64, i1 } @llvm.uadd.with.overflow.i64(i64 %x, i64 %y)
  %b = extractvalue { i64, i1 } %a, 1
  br i1 %b, label %trap, label %bb

bb:
  %c = extractvalue { i64, i1 } %a, 0
  %d = icmp ult i64 %c, %x
  ret i1 %d

trap:
  call void @llvm.trap()
  unreachable
}

declare { i64, i1 } @llvm.uadd.with.overflow.i64(i64, i64)
declare void @llvm.trap()

Here %d is the same as %b, and must be false in %bb by implication.

I think this should be fairly easy to fix (at least for this specific case)
with an instcombine pattern that converts the icmp into an extract, and CSE/GVN
will take care of the rest.
Quuxplusone commented 5 years ago

https://reviews.llvm.org/D58644

Quuxplusone commented 4 years ago

Fixed in https://github.com/llvm/llvm-project/commit/8db5143b1a1521915c842ebef23cb9fe8fe607ce.

I'm keeping this issue open, because the negated variation of this is not handled yet.