[PATCH] D159499: [InstCombine] Fold or(phi1,phi2) into or(icmp1,icmp2)

[Noah Goldstein via Phabricator via llvm-commits]

goldstein.w.n added inline comments.


================
Comment at: llvm/lib/Transforms/InstCombine/InstCombineCompares.cpp:2030
+                      m_ICmp(EqPred, m_Specific(operand->get()), m_Zero())) &&
+                EqPred == ICmpInst::ICMP_NE))))
+          return nullptr;
----------------
bipmis wrote:
> goldstein.w.n wrote:
> > This seems very much so like a DAG type fold where you are basing the decision on existing expressions.
> > `phi` doesn't exist in backend (dagcombiner) so its a bit tricky.
> > @nikic, any place this can be put where it doesn't req iterating the use-list?
> > 
> > generally in instcombine we get our duplication by just blindly canoniclizing to a standard format.
> > 
> > Is there a regression if you blindly do the transform?
> I am seeing quite some benefit with this Transform as we are able to reuse instructions which happens to be the other use of phi.
> However just the direct transform 
> or(i64 a,i64 b) -> or((icmp ne a ,0), (icmp ne b ,0)) is expensive. On the contrary the reverse is true in InstCombine.
> 
> I can check if the transform or(i64 phia,i64 phib) regresses.
There are a lot of folds that we can do / improve if we optimized around "what exists". If there is a place for such folds (prior to DAGCombiner) that would want this to be moved there. Maybe nikic has an idea about that.


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

https://reviews.llvm.org/D159499