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

[A. Skrobov via llvm-commits]

tyomitch added a comment.

In http://reviews.llvm.org/D13733#270949, @rengolin wrote:

> In http://reviews.llvm.org/D13733#270160, @tyomitch wrote:
>
> > The motivation is to enable http://reviews.llvm.org/D13862 -- I've now set you as a reviewer there, too.
>
>
> Right, so the patterns that ended up as target-specific (and thus not lowered optimally) were due to that change first?


Not quite: in order to be able to apply http://reviews.llvm.org/D13862, we need to take the DIV,REM -> DIVREM combining out of the lowering stage.
Otherwise, when we set DIV and REM to be legalized to libcalls, instead of the DIVREM, we lose the ability to combine them together.

Therefore, my plan was to apply http://reviews.llvm.org/D13862 **after** http://reviews.llvm.org/D13733.
In other words, http://reviews.llvm.org/D13733 doesn't fix any problems arising from http://reviews.llvm.org/D13862 -- it prevents their occurrence :)


================
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)
----------------
tyomitch wrote:
> rengolin wrote:
> > Why not getValue(0), here?
> That's how it was in the original code in LegalizeDAG.cpp
> 
> I don't have a particular preference, and I don't mind changing it to getValue(0)
Any preference here?


http://reviews.llvm.org/D13733