[PATCH] D81987: AMDGPU/GlobalISel: Legalize 64-bit G_SDIV/G_SREM

[Jay Foad via Phabricator via llvm-commits]

foad accepted this revision.
foad added inline comments.
This revision is now accepted and ready to land.


================
Comment at: llvm/lib/Target/AMDGPU/AMDGPULegalizerInfo.cpp:2777
 
-  if (IsDiv) {
+  if (!IsRem) {
     auto Sel1 = B.buildSelect(
----------------
I'd prefer to put an un-negated condition here, given that there's an "else".


================
Comment at: llvm/lib/Target/AMDGPU/AMDGPULegalizerInfo.cpp:2851-2852
+    auto DSign = B.buildXor(Ty, LHSign, RHSign);
+    UDivRem = B.buildXor(Ty, UDivRem, DSign).getReg(0);
     B.buildSub(DstReg, UDivRem, DSign);
   }
----------------
Unrelated but it seems a bit silly to duplicate these lines from 2847-2848.


================
Comment at: llvm/lib/Target/AMDGPU/AMDGPULegalizerInfo.h:118
+  void legalizeUDIV_UREM64Impl(MachineIRBuilder &B,
+                               Register DstReg, Register Num, Register Den,
+                               bool IsRem) const;
----------------
Parameter names Num and Den don't match the definition.


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

https://reviews.llvm.org/D81987