[PATCH] D13733: Perform DIV, REM -> DIVREM combining for all affected nodes at once.

Renato Golin via llvm-commits llvm-commits at lists.llvm.org
Mon Oct 19 05:08:19 PDT 2015


rengolin added a comment.

Right, I think I got the idea of the code...

This will combine any div+rem or rem+div, but also account for div+divrem and rem+divrem. Only not for divrem+divrem, if both were converted to divrem due to ABI rules, ex: a div+rem that were individually converted earlier, and now are being merged. I imagine this doesn't occur in practice.

Now, what I don't understand is the motivation. I get it that you found some patterns that weren't being recognised, but there are no tests to show what it is. It'd be good to add a new test with all the cases that couldn't be merged before.

cheers,
--renato


================
Comment at: lib/CodeGen/SelectionDAG/DAGCombiner.cpp:2232
@@ +2231,3 @@
+      if (UserOpc == ISD::SDIV || UserOpc == ISD::UDIV)
+        CombineTo(User, combined);
+      else if (UserOpc == ISD::SREM || UserOpc == ISD::UREM)
----------------
Why not getValue(0), here?


http://reviews.llvm.org/D13733





More information about the llvm-commits mailing list