[llvm] [AMDGPU] Swap select operands to allow later v_cndmask shrinking into vop2 (PR #142354)

Nicolai Hähnle via llvm-commits llvm-commits at lists.llvm.org
Thu Jun 19 10:27:14 PDT 2025


nhaehnle wrote:

> > > One of the problems with this version is that this check is called for each select instruction, checking ALL selects every time. In the previous version the check was done only once when looking at compare instruction : #142140
> > 
> > 
> > Agreed, that makes the current version pretty confusing. Maybe the best of both worlds would be to write it as a DAGCombine that vists the SETCC node instead of the SELECT?
> 
> Actually it is not just _confusing_, I think there might be a real problem: the second time you visit the same SETCC, if the first SELECT has already been removed from the DAG (*), then your `ShouldSwap` calculation might come up with a different result. So you might swap some uses but not others, which will make the code worse overall.
> 
> (*) I guess this depends on whether the DAG removes dead nodes on the fly, or cleans them up later.

Indeed. This is actually a big reason for why we ended up with that email thread about perhaps doing it in AMDGPUCodeGenPrepare, which was resolved with the good point about ISel possibly messing with the order again, plus other sources of these instructions.

Another point in favor of LLVM IR was that it would cover GlobalISel as well.

It is a bit of a vexing issue and I don't have a good answer for it, but it's also problematic that we don't seem to have a good answer for it. If we stay limited to SelectionDAG: Is it acceptable to replace *other* nodes of the DAG from this method somehow?

https://github.com/llvm/llvm-project/pull/142354


More information about the llvm-commits mailing list