[PATCH] Added instruction combine to transform few more negative values addition to subtraction (Part 2)

[Jingyue Wu]

I see. I wasn't involved with the first part, so was confused. Thanks for clarification!

================
Comment at: lib/Transforms/InstCombine/InstCombineAddSub.cpp:980
@@ +979,3 @@
+   // if ONE is on other side, swap
+   if (match(RHS, m_Add(m_Value(X), m_One())))
+     std::swap(LHS, RHS);
----------------
If
```
LHS = w + 1
RHS = ((z & c) ^ c) + 1
```
the code swaps LHS and RHS and may miss some optimization opportunities. 

Maybe instead of premature swapping, we can look deeper into both LHS and RHS. What do you think? 

I may be over-concerned, because Reassociate will optimize (a + 1) + (b + 1) to a + (b + 2). I am not sure about the ordering. 

================
Comment at: lib/Transforms/InstCombine/InstCombineAddSub.cpp:998
@@ +997,3 @@
+       // ADD(ADD(X, 1), RHS) == ADD(X, ADD(RHS, 1)) == SUB(RHS, OR(Z, ~C1))
+       else if (!C1->countTrailingZeros()) {
+         if (match(Y, m_And(m_Value(Z), m_APInt(C2))) && (*C1 == *C2)) {
----------------
Nit: I prefer
```
C1->countTrailingZeros() == 0
```
because C1->countTrailingZeros() is not a boolean. 

================
Comment at: lib/Transforms/InstCombine/InstCombineAddSub.cpp:998
@@ +997,3 @@
+       // ADD(ADD(X, 1), RHS) == ADD(X, ADD(RHS, 1)) == SUB(RHS, OR(Z, ~C1))
+       else if (!C1->countTrailingZeros()) {
+         if (match(Y, m_And(m_Value(Z), m_APInt(C2))) && (*C1 == *C2)) {
----------------
Jingyue Wu wrote:
> Nit: I prefer
> ```
> C1->countTrailingZeros() == 0
> ```
> because C1->countTrailingZeros() is not a boolean. 
Nit: Does LLVM coding standard say anything about: 
```
} else if {
```
or
```
}
else if {
```

The first style seems more commonly used. 

================
Comment at: lib/Transforms/InstCombine/InstCombineAddSub.cpp:1001
@@ +1000,3 @@
+           Value *NewOr = Builder->CreateOr(Z, ~(*C1));
+           return Builder->CreateSub(RHS, NewOr, "", IHasNUW, IHasNSW);
+         }
----------------
I doubt using IHasNUW and IHasNSW is correct due to the logic at Line 985-986. The logic there reassociates
```
I = (x + 1) + ((z & c) ^ c)
```
into
```
I' = x + (((z & c) ^ c) + 1)
```
So, the toplevel "+" in I' may not inherit the NSW/NUW of the toplevel "+" in I. 

Essentially, (a + 1) +nsw b != a +nsw (b + 1).  Does that make sense to you?

http://reviews.llvm.org/D4209