[PATCH] D138790: [DAG] Attempt to replace a mul node with an existing umul_lohi/smul_lohi node (PR59217)

[Amaury SECHET via Phabricator via llvm-commits]

deadalnix accepted this revision.
deadalnix added a comment.
This revision is now accepted and ready to land.

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.



================
Comment at: llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp:4042
+      SDVTList LoHiVT = DAG.getVTList(VT, VT);
+      // TODO: Can we match commutable operands with getNodeIfExists?
+      if (SDNode *LoHi = DAG.getNodeIfExists(LoHiOpc, LoHiVT, {N0, N1}))
----------------
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.


================
Comment at: llvm/test/CodeGen/X86/smul-with-overflow.ll:187
 
 define { i129, i1 } @smul_ovf(i129 %x, i129 %y) nounwind {
 ; X86-LABEL: smul_ovf:
----------------
I have no idea how to evaluate if this is better or worse.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138790