[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.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D152430/new/
https://reviews.llvm.org/D152430
More information about the llvm-commits
mailing list