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

[chenglin.bi via Phabricator via llvm-commits]

bcl5980 added inline comments.


================
Comment at: llvm/include/llvm/Analysis/LogicCombine.h:22
+private:
+  LogicCombiner *Helper;
+  Value *Val;
----------------
nikic wrote:
> It's weird that LogicalOpNode has a reference back to LogicCombiner. Is this just for printing? Would it be better to pass it to the print method?
I call print in operator "<<" . I can't pass the LogicCombiner into the override operator "<<". Do you have any ideal for that?


================
Comment at: llvm/include/llvm/Analysis/LogicalExpr.h:56
+  // TODO: can we use APInt define the mask to enlarge the max leaf number?
+  uint64_t LeafMask;
+
----------------
nikic wrote:
> Am I missing something, or is LeafMask never actually used?
The initial patch hasn't use LeafMask. The following up change D143155 will use that. 


================
Comment at: llvm/include/llvm/Analysis/LogicalExpr.h:86
+      // a & 0 -> 0
+      if (LHS == 0)
+        continue;
----------------
nikic wrote:
> Hm, can these actually occur? It looks like they should be excluded by a ^ a canonicalization.
For now it should happen in the case with constant 0 like: %and = and i8 %a,  0.
Constant 0 can be represented by empty set or one element with 0 value. If I canonicalize one element with 0 value to empty set then it won't happen.



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

https://reviews.llvm.org/D142803