[PATCH] D142803: [ComplexLogicCombine 1/?] Implement a general way to simplify complex logical operations.

[chenglin.bi via Phabricator via llvm-commits]

bcl5980 added inline comments.


================
Comment at: llvm/include/llvm/Analysis/LogicalExpr.h:16
+// (0b1111 + 0b1001 + 0b0010 + 0b1101) * (0b0001 + 0b0101)
+// -->
+// (0b1111 + 0b1001 + 0b0010 + 0b1101) * 0b0001+ (0b1111 + 0b1001 + 0b0010 +
----------------
spatel wrote:
> I'm still confused by the notation. Each "-->" step needs a comment to describe exactly what is happening. If we are not showing some unique math/logic property with each of the terms in the equation/set, then it would be easier to follow the logic with a smaller example.
> 
> In this step, we are splitting the RHS masks to operate over the LHS? But are those "+" and "*" symbols representing real math operations or are they bitwise logical operations?
"+" and "*" representing math operation on the boolean ring.
"+" is the same to xor, "*" is the same to and. And they also follow the distributive laws and commutative law like the normal "*", "+".
 


================
Comment at: llvm/include/llvm/Analysis/LogicalExpr.h:20
+// -->
+// (0b1111 | 0b0001) + (0b1001 | 0b0001) + (0b0010 | 0b0001) + (0b1101 | 0b0001)
+// + (0b1111 | 0b0101) + (0b1001 | 0b0101) + (0b0010 | 0b0101) + (0b1101 |
----------------
spatel wrote:
> Here we have distributed the RHS mask values over the LHS mask values? Why did "*" become "|"?
For example:  `ab * bd  = abd`
The expression `ab * bd` convert to mask will be `0b0011 * 0b1010`. 
The result `abcd` convert to mask will become 0b1011.
So for the "*" operation is actually "|" LHS and RHS 's masks.


================
Comment at: llvm/include/llvm/Analysis/LogicalExpr.h:24
+// -->
+// 0b1111 + 0b1001 + 0b0010 + 0b1101 + 0b1111 + 0b1101 + 0b0111 + 0b1101
+// -->
----------------
spatel wrote:
> I don't know what operation was done there. It's not logical-or or multiplication?
For the "+" operation, we can replace them to xor. So if we find two mask is the same, we can remove both of them.


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

https://reviews.llvm.org/D142803