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

Amaury SECHET via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri Jun 9 06:14:59 PDT 2023

deadalnix 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:
> 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.

  rG LLVM Github Monorepo



More information about the llvm-commits mailing list