[PATCH] D96013: GlobalISel: Try to combine G_[SU]DIV and G_[SU]REM

[Christudasan Devadasan via Phabricator via llvm-commits]

cdevadas added a comment.

I will make an entry for the new opcodes in docs/GlobalISel/GenericOpcode.rst



================
Comment at: llvm/lib/CodeGen/GlobalISel/CombinerHelper.cpp:1026
+  MI.eraseFromParent();
+  OtherMI->eraseFromParent();
+}
----------------
arsenm wrote:
> I'm worried about what happens if multiple div/rem users exist. Can you add some tests for that?
This patch currently performs a one-to-one match. It doesn't look for multiple div/rem users. 
For example, it won't optimally match the following pattern: 
G_SDIV x, y, G_SREM x, y and an extra G_SDIV x, y
The first div/rem pair will be combined and the second div will be retained.
Instead, it should have generated d, r = G_SDIVREM x, y ;  replace all users of the second G_SDIV with 'd' and then delete the extra G_SDIV as well.
I will rework this part.



================
Comment at: llvm/lib/CodeGen/GlobalISel/LegalizerHelper.cpp:6277
+
+LegalizerHelper::LegalizeResult LegalizerHelper::lowerDIVREM(MachineInstr &MI) {
+  // Split DIVREM into individual instructions.
----------------
paquette wrote:
> Can the Legalizer changes come in a follow-up patch?
I thought about it initially. But it makes more sense to legalize the operations along with the patch that introduces the opcodes. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96013