-
Notifications
You must be signed in to change notification settings - Fork 11.9k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Simplify sadd_overflow
+assume
to add nsw
[InstCombine]
#80637
Comments
Do you have an example with larger context here? As written, this is going to lose information, so I'm not sure whether this makes sense. I assume that there is some follow-on optimization failure that you want to avoid? |
I don't have a specific case, since I've always been able to use
|
The opposite transform is only legal because you are passing the value to a |
Ah, makes sense! Thanks. I guess I'll close this, then, since I didn't feel particularly strongly about it and you found a reason not to do it. |
…=<try> Also get `add nuw` from `uN::checked_add` When I was doing this for `checked_{sub,shl,shr}`, it was mentioned rust-lang#124114 (comment) that it'd be worth trying for `checked_add` too. It makes a particularly-big difference for `x.checked_add(C)`, as doing this means that LLVM removes the intrinsic and does it as a normal `x <= MAX - C` instead. cc `@DianQK` who had commented about `checked_add` related to rust-lang/hashbrown#509 before cc llvm/llvm-project#80637 for how LLVM is unlikely to do this itself
…=<try> Also get `add nuw` from `uN::checked_add` When I was doing this for `checked_{sub,shl,shr}`, it was mentioned rust-lang#124114 (comment) that it'd be worth trying for `checked_add` too. It makes a particularly-big difference for `x.checked_add(C)`, as doing this means that LLVM removes the intrinsic and does it as a normal `x <= MAX - C` instead. cc `@DianQK` who had commented about `checked_add` related to rust-lang/hashbrown#509 before cc llvm/llvm-project#80637 for how LLVM is unlikely to do this itself
…=Amanieu Also get `add nuw` from `uN::checked_add` When I was doing this for `checked_{sub,shl,shr}`, it was mentioned rust-lang#124114 (comment) that it'd be worth trying for `checked_add` too. It makes a particularly-big difference for `x.checked_add(C)`, as doing this means that LLVM removes the intrinsic and does it as a normal `x <= MAX - C` instead. cc `@DianQK` who had commented about `checked_add` related to rust-lang/hashbrown#509 before cc llvm/llvm-project#80637 for how LLVM is unlikely to do this itself
…=Amanieu Also get `add nuw` from `uN::checked_add` When I was doing this for `checked_{sub,shl,shr}`, it was mentioned rust-lang#124114 (comment) that it'd be worth trying for `checked_add` too. It makes a particularly-big difference for `x.checked_add(C)`, as doing this means that LLVM removes the intrinsic and does it as a normal `x <= MAX - C` instead. cc `@DianQK` who had commented about `checked_add` related to rust-lang/hashbrown#509 before cc llvm/llvm-project#80637 for how LLVM is unlikely to do this itself
Also get `add nuw` from `uN::checked_add` When I was doing this for `checked_{sub,shl,shr}`, it was mentioned rust-lang/rust#124114 (comment) that it'd be worth trying for `checked_add` too. It makes a particularly-big difference for `x.checked_add(C)`, as doing this means that LLVM removes the intrinsic and does it as a normal `x <= MAX - C` instead. cc `@DianQK` who had commented about `checked_add` related to rust-lang/hashbrown#509 before cc llvm/llvm-project#80637 for how LLVM is unlikely to do this itself
Also get `add nuw` from `uN::checked_add` When I was doing this for `checked_{sub,shl,shr}`, it was mentioned rust-lang/rust#124114 (comment) that it'd be worth trying for `checked_add` too. It makes a particularly-big difference for `x.checked_add(C)`, as doing this means that LLVM removes the intrinsic and does it as a normal `x <= MAX - C` instead. cc `@DianQK` who had commented about `checked_add` related to rust-lang/hashbrown#509 before cc llvm/llvm-project#80637 for how LLVM is unlikely to do this itself
Also get `add nuw` from `uN::checked_add` When I was doing this for `checked_{sub,shl,shr}`, it was mentioned rust-lang/rust#124114 (comment) that it'd be worth trying for `checked_add` too. It makes a particularly-big difference for `x.checked_add(C)`, as doing this means that LLVM removes the intrinsic and does it as a normal `x <= MAX - C` instead. cc `@DianQK` who had commented about `checked_add` related to rust-lang/hashbrown#509 before cc llvm/llvm-project#80637 for how LLVM is unlikely to do this itself
The probably never comes up in C++, but in Rust people can easily end up there from things that boil down to
Proof: https://alive2.llvm.org/ce/z/m-n_PG
And of course the
uadd_overflow
+assume
→add nuw
too.The text was updated successfully, but these errors were encountered: