[PATCH] D16735: [ValueTracking] Improve isKnownNonZero for PHI of non-zero constants

[Sanjoy Das via llvm-commits]

sanjoy requested changes to this revision.
sanjoy added a comment.
This revision now requires changes to proceed.

Minor comments inline.


================
Comment at: lib/Analysis/ValueTracking.cpp:2065
@@ -2064,1 +2064,3 @@
     }
+    // Check if all incoming values are non-zero constant.
+    bool HasUnknowValue = false;
----------------
A cleaner way of writing this would be:

```
// llvm::all_of from STLExtras.h
bool AllNonZeroConstants = all_of(PN->operands(), [](Value *V) {
  return isa<ConstantInt>(V) && !cast<ConstantInt>(V)->isZeroValue();
});
if (AllNonZeroConstants)
  return true;

```


================
Comment at: test/Transforms/InstCombine/phi.ll:768
@@ +767,3 @@
+entry:
+  %cmp = icmp sgt i1 %c, 0
+  br i1 %cmp, label %if.then, label %if.else
----------------
Minor: you can simplify the branch to `br i1 %c, label %if.then, label %if.else`.

================
Comment at: test/Transforms/InstCombine/phi.ll:774
@@ +773,3 @@
+
+if.else:                                          ; preds = %entry
+  br label %if.end
----------------
Minor: I'd add a call to an unknown function in one of the if branches.  Without it simplifycfg changes the PHI to a select (and elides the control flow), after which LLVM is able to optimize this function in `-O3`, but with a `call void @foo()` in one of the branches, the phi->select conversion does not happen, and this test case demonstrates a truly missed optimization in LLVM today.


http://reviews.llvm.org/D16735