[PATCH] D136582: [InstCombine] fold `sub + and` pattern with specific const value

[chenglin.bi via Phabricator via llvm-commits]

bcl5980 added a comment.

In D136582#3905964 <https://reviews.llvm.org/D136582#3905964>, @spatel wrote:

> I still think this is a very specific/narrow transform, but if this is important to optimize, then I won't hold it up. The code seems correct. 
> See inline comments for a few nits.

I agree that the code is very limited, but I also worry about the solution canonicalize `sub + and` to `and + sub`.
If we find more cases get benefit from general canonicalization in the future, we can change to the canonicalization.



================
Comment at: llvm/lib/Transforms/InstCombine/InstCombineAddSub.cpp:2045
+    // C - ((C3 -nuw X) & C2) --> (C - (C2 & C3)) + (X & C2) when:
+    // (C3 - (C2 & C3) + 1) is pow2
+    // ((C2 + C3) & ((C2 & C3) - 1)) == ((C2 & C3) - 1)
----------------
spatel wrote:
> Should this be:
>   // (C3 - (C2 & C3) - 1) is pow2  
If we want to match the alive2 proof, it should be:
    (C3 - ((C2 & C3) - 1)) is pow2
and it is the same to 
    (C3 - (C2 & C3) + 1) is pow2


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D136582/new/

https://reviews.llvm.org/D136582