[PATCH] D152430: [DAG] Peek through freeze when deciding whether we should convert setcc to math or not.

[Nikita Popov via Phabricator via llvm-commits]

nikic added inline comments.


================
Comment at: llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp:12251
   // =>
   //   FREEZE(SETCC(X, CONST, Cond))
   // This is correct if FREEZE(X) has one use and SETCC(FREEZE(X), CONST, Cond)
----------------
deadalnix wrote:
> deadalnix wrote:
> > RKSimon wrote:
> > > Is this fold the underlying problem? Every other freeze fold we have tries to push the freeze upward through the DAG, but this pulling it down?
> > I'm not sure it is the underlying problem, but it is indeed part of the loop.
> > 
> > The underlying problem is that we want to combine setcc to math, unless the setcc is used in a brcond. The math is turned back into a setcc by the brcond in that case, which created an infinite loop and this is why `PreferSetCC` was introduced in the first place.
> > 
> > As to pushing the freeze up or down, I don't really have an opinion on the matter.
> Reworded: the loop problem existed in the past and was fixed, but this transform defeated the previous fix in the presence of a freeze.
> 
> I do indeed think that there needs to be some thought put into how we want to handle freeze, as this doesn't look very solid to me.
Possible alternative: https://reviews.llvm.org/D152544


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D152430