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

Tue May 25 02:56:52 PDT 2021

foad 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);
----------------
Matt usually objects to this indentation change on the grounds that clang-format is wrong :)

================
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),
----------------
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?

================
Comment at: llvm/lib/Target/AMDGPU/AMDGPULegalizerInfo.h:113

   bool legalizeUDIV_UREM64(MachineInstr &MI, MachineRegisterInfo &MRI,
                            MachineIRBuilder &B) const;
----------------
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?

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100726