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

Wed Feb 17 12:59:51 PST 2021

arsenm added inline comments.

================
Comment at: llvm/lib/CodeGen/GlobalISel/CombinerHelper.cpp:1021
+      Opcode == TargetOpcode::G_SDIV || Opcode == TargetOpcode::G_SREM;
+  Builder.setInstrAndDebugLoc(MI);
+  Builder.buildInstr(IsSigned ? TargetOpcode::G_SDIVREM
----------------
I still suspect something is wrong with the insertion. I don't believe it's guaranteed that the first instruction you find will dominate the second, so you don't know where to place the insert point. 

================
Comment at: llvm/test/CodeGen/AMDGPU/GlobalISel/prelegalizer-combiner-divrem.mir:381-382
+    %ptr2:_(p1) = COPY $vgpr10_vgpr11
+    %src1:_(<4 x s32>) = G_BUILD_VECTOR %x0:_(s32), %x1:_(s32), %x2:_(s32), %x3:_(s32)
+    %src2:_(<4 x s32>) = G_BUILD_VECTOR %y0:_(s32), %y1:_(s32), %y2:_(s32), %y3:_(s32)
+    %rem:_(<4 x s32>) = G_UREM %src1:_(<4 x s32>), %src2:_(<4 x s32>)
----------------
You can shorten the test by just copying tuple registers, we just lower arguments with the separate copies

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96013