Consider using new LLVM intrinsic for i32::saturating_add etc.
hanna-kruppe opened this issue · 9 comments
As featured in LLVM Weekly, https://reviews.llvm.org/rL344629 introduces @llvm.sadd.sat for saturating signed integer addition. Not sure whether it's as transparent to the optimizer yet as open-coding it based on checked_add (as we currently do), but we might want to try it when we update to an LLVM version that has that intrinsic available.
Right now add.sat is completely opaque. I've submitted https://reviews.llvm.org/D54237 for ConstantFolding and InstCombine support, which should be enough to properly handle #52203.
All the optimizations have landed, so we can give this a try after the next LLVM update.
Looks like a sufficiently-new LLVM has landed in nightly, and the folding logic is working: https://play.rust-lang.org/?version=nightly&mode=release&edition=2018&gist=75e0b8364fc0d815edc1d091840fca47
Work on saturating add/sub intrinsics is still ongoing (turns out that getting new intrinsics to optimize and codegen well is a pretty big rabbit hole), but I believe at this point they're good enough that we should give them a try in Rust. I'll submit a PR later this week, if no one beats me to it.
It's awesome seeing how much better things like Chain::size_hint are with saturating, since it now figures out sometimes that it can't overflow and removes the saturating-ness 🙂
If you have available time, consider having LLVM fold uadd.sat(a, b) == 0 into or(a, b) == 0.
@scottmcm Do you have an example where the uadd.sat(a, b) == 0 pattern turns up in practice?
@nikic The place I ran into it was #35428 (comment), trying to figure out whether a Chain is empty based on its size_hint. Simple godbolt demo: https://rust.godbolt.org/z/gKKzIu
(Something other than or could be fine too; I just picked that because LLVM folds x == 0 && y == 0 into (x | y) == 0, so thought it seemed reasonable.)
But I acknowledge you're busy and it's not urgent.
@scottmcm I've opened https://reviews.llvm.org/D69224 to add this fold.