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

[Sanjay Patel via Phabricator via llvm-commits]

spatel added inline comments.


================
Comment at: llvm/include/llvm/Analysis/LogicalExpr.h:35
+///      001 * 010 * 001 + 001 * 010 * 100 + 001 * 010 + 001 + 100 + 010 * 100 *
+///      001 + -1C * 001
+/// Caculate multiplication:
----------------
The "*" operation with -1 is also a special-case, so we should mention it in the text above here.
If I understand the code, if we have "a * -1", then the result is always "a".


================
Comment at: llvm/include/llvm/Analysis/LogicalExpr.h:36-38
+/// Caculate multiplication:
+///      011 + 111 + 011 + 001 + 100 + 111 + 001
+/// Caculate addiction:
----------------
Spelling: Caculate -> Calculate
Spelling: addiction -> addition


================
Comment at: llvm/include/llvm/Analysis/LogicalExpr.h:95
+        // a & 1 -> a
+        if (NewMask != ExprAllOne && ((NewMask & ExprAllOne) != 0))
+          NewMask &= ~ExprAllOne;
----------------
When we get here, we know that both LHSMask and RHSMask are not equal to "0".
Can we also assert that if a mask has ExprAllOne set, then no other bit in the mask is set?
So would it be clearer to move this check up and write this as:
  // 1 & a -> a
  // a & 1 -> a
  if (LHSMask == ExprAllOne)
    NewMask = RHSMask;
  else if (RHSMask == ExprAllOne)
    NewMask = LHSMask;


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

https://reviews.llvm.org/D142803