[PATCH] D157798: [ValueTracking] Add tests for deducing non-zero based for incoming phi-edges; NFC

[Noah Goldstein via Phabricator via llvm-commits]

goldstein.w.n marked 2 inline comments as done.
goldstein.w.n added inline comments.


================
Comment at: llvm/test/Analysis/ValueTracking/phi-known-nonzero.ll:24
+F:
+  br i1 true, label %T, label %F
+}
----------------
nikic wrote:
> Does just `br label %T` not work here? I don't think the conditional is needed.
Switched to do that.


================
Comment at: llvm/test/Analysis/ValueTracking/phi-known-nonzero.ll:211
+  ret i1 %r
+}
----------------
nikic wrote:
> goldstein.w.n wrote:
> > nikic wrote:
> > > Missing test for the equal successor case?
> > > 
> > > And unless I missed it, there is no test for swapped successors (F, T instead of T, F)?
> > > Missing test for the equal successor case?
> > What do you mean 'equal successor case'?
> > > And unless I missed it, there is no test for swapped successors (F, T instead of T, F)?
> > eq_non_zero2 and all the `*lt/*le` cases?
> > But added a failure case for this as well.
> > What do you mean 'equal successor case'?
> 
> `br i1 %cmp, label %T, label %T`
> 
> > What do you mean 'equal successor case'?
> 
> Oh, I see. You always use `br i1 %cmp, label %T, label %F`, but in some cases the phi is in `%T` and in some in  `%F`. Okay, that also works.
Switched to the uncond logic.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157798