[PATCH] D11918: Constant propagation after hiting assume(icmp)

[David Majnemer via llvm-commits]

majnemer added inline comments.

================
Comment at: lib/Transforms/Scalar/GVN.cpp:2069-2075
@@ -2049,7 +2068,9 @@
 
-    if (LHS == RHS) continue;
+    if (LHS == RHS)
+      continue;
     assert(LHS->getType() == RHS->getType() && "Equality but unequal types!");
 
     // Don't try to propagate equalities between constants.
-    if (isa<Constant>(LHS) && isa<Constant>(RHS)) continue;
+    if (isa<Constant>(LHS) && isa<Constant>(RHS))
+      continue;
 
----------------
Avoid making changes not relevant to this change.  This seems like a fine change to do separately.

================
Comment at: lib/Transforms/Scalar/GVN.cpp:2226-2231
@@ -2204,1 +2225,8 @@
 
+  if (IntrinsicInst *IntrinsicI = dyn_cast<IntrinsicInst>(I)) {
+    if (IntrinsicI->getIntrinsicID() == Intrinsic::assume) {
+
+      Value *V = IntrinsicI->getArgOperand(0);
+      if (ICmpInst *ICmpI = dyn_cast<ICmpInst>(V)) {
+        if (ICmpI->getSignedPredicate() == ICmpInst::Predicate::ICMP_EQ) {
+
----------------
The type is obvious so you could just use `auto *`.  Also, consider splitting this into another function to reduce indentation.

================
Comment at: lib/Transforms/Scalar/GVN.cpp:2234-2237
@@ +2233,6 @@
+          bool Changed = false;
+          Value *Lhs = ICmpI->getOperand(0);
+          Value *Rhs = ICmpI->getOperand(1);
+          Constant *LhsConst = dyn_cast<Constant>(Lhs);
+          Constant *RhsConst = dyn_cast<Constant>(Rhs);
+
----------------
Initialisms are typically capitalized.

================
Comment at: lib/Transforms/Scalar/GVN.cpp:2240
@@ +2239,3 @@
+          // Propagate equality only if only one of operand is const.
+          if ((LhsConst != nullptr) ^ (RhsConst != nullptr)) {
+            for (BasicBlock *successor : successors(I->getParent())) {
----------------
This could be written more concisely as: `!LHSConst != !RHSConst`.  The xor is a little out of place considering you aren't doing anything with bits.


http://reviews.llvm.org/D11918