[PATCH] D100726: AMDGPU/GlobalISel: Legalize G_[SU]DIVREM instructions

[Christudasan Devadasan via Phabricator via llvm-commits]

cdevadas added inline comments.


================
Comment at: llvm/lib/Target/AMDGPU/AMDGPULegalizerInfo.cpp:586-589
+      .customFor({S32, S64})
+      .clampScalar(0, S32, S64)
+      .widenScalarToNextPow2(0, 32)
+      .scalarize(0);
----------------
foad wrote:
> Matt usually objects to this indentation change on the grounds that clang-format is wrong :)
Oh. It was auto-formatted by clang-format.


================
Comment at: llvm/lib/Target/AMDGPU/AMDGPULegalizerInfo.cpp:2985
     auto Sel1 = B.buildSelect(
         S64, B.buildICmp(CmpInst::ICMP_NE, S1, C6, Zero32), Add4, Add3);
+    B.buildSelect(DstDivReg, B.buildICmp(CmpInst::ICMP_NE, S1, C3, Zero32),
----------------
foad wrote:
> Pull this buildICmp out of the "if"s, since it is identical in both cases, and you don't want to build it twice in the divrem case?
Thanks Jay. It is addressed with D103083.


================
Comment at: llvm/lib/Target/AMDGPU/AMDGPULegalizerInfo.h:113
 
   bool legalizeUDIV_UREM64(MachineInstr &MI, MachineRegisterInfo &MRI,
                            MachineIRBuilder &B) const;
----------------
foad wrote:
> It looks odd that you did not change the name of this function, but I see it is not implemented, so maybe just remove the declaration here?
Yes, it was a dead declaration. Cleaned it up with e3b8e6d48251


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100726