As discussed on Issue #59217, under certain circumstances the DAG can generate duplicate MUL and MUL_LOHI nodes, often during MULO legalization.
This patch attempts to replace MUL nodes with additional uses of the LO result from the MUL_LOHI node.
Paths
| Differential D138790
[DAG] Attempt to replace a mul node with an existing umul_lohi/smul_lohi node (PR59217) ClosedPublic Authored by RKSimon on Nov 28 2022, 4:17 AM.
Details Summary As discussed on Issue #59217, under certain circumstances the DAG can generate duplicate MUL and MUL_LOHI nodes, often during MULO legalization. This patch attempts to replace MUL nodes with additional uses of the LO result from the MUL_LOHI node.
Diff Detail
Event TimelineRKSimon retitled this revision from [DAG] Attempt to replace a mul nodes with an existing umul_lohi/smul_lohi node (PR59217) to [DAG] Attempt to replace a mul node with an existing umul_lohi/smul_lohi node (PR59217).Nov 28 2022, 8:26 AM Comment Actions The change that are relatively small all come either better or equivalent. While it's hard to evaluate the giant multiplications ones, this definitively looks like a win.
This revision is now accepted and ready to land.Nov 28 2022, 9:31 AM
This revision was landed with ongoing or failed builds.Nov 29 2022, 4:51 AM Closed by commit rG30eff7f29f97: [DAG] Attempt to replace a mul node with an existing umul_lohi/smul_lohi node… (authored by RKSimon). · Explain Why This revision was automatically updated to reflect the committed changes.
Revision Contents
Diff 478530 llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
llvm/test/CodeGen/AMDGPU/llvm.mulo.ll
llvm/test/CodeGen/X86/combine-mul.ll
llvm/test/CodeGen/X86/muloti.ll
llvm/test/CodeGen/X86/smul-with-overflow.ll
llvm/test/CodeGen/X86/smul_fix_sat.ll
llvm/test/CodeGen/X86/smulo-128-legalisation-lowering.ll
llvm/test/CodeGen/X86/vec_smulo.ll
llvm/test/CodeGen/X86/xmulo.ll
|
As you noted, the code is not the most elegant, but it gets the job done. I'm not sure there is a better way to write this at the moment.