[PATCH] D72264: GlobalISel: Implement s32->s64 G_FPTOSI lowering

[Jessica Paquette via Phabricator via llvm-commits]

paquette added inline comments.


================
Comment at: llvm/lib/CodeGen/GlobalISel/LegalizerHelper.cpp:4025
+  // This algorithm comes from compiler-rt's implementation of fixsfdi:
+  // https://github.com/llvm/llvm-project/blob/master/compiler-rt/lib/builtins/fixsfdi.c
+
----------------
Do we need the whole GitHub URL here? Why not just compiler-rt/lib/builtins/fixsfdi.c?

Also it looks like most the work is in fp_fixint_impl.inc? Would it be better to point at that instead?


================
Comment at: llvm/lib/CodeGen/GlobalISel/LegalizerHelper.cpp:4027-4072
+  unsigned SrcEltBits = SrcTy.getScalarSizeInBits();
+
+  auto ExponentMask = MIRBuilder.buildConstant(SrcTy, 0x7F800000);
+  auto ExponentLoBit = MIRBuilder.buildConstant(SrcTy, 23);
+
+  auto AndExpMask = MIRBuilder.buildAnd(SrcTy, Src, ExponentMask);
+  auto ExponentBits = MIRBuilder.buildLShr(SrcTy, AndExpMask, ExponentLoBit);
----------------
Can we have some comments explaining what's going on here? I guess it's *technically* already in the compiler-rt code, but it would be nice to see it right here, especially in case that code ever changes.


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

https://reviews.llvm.org/D72264