[PATCH] D28637: [PPC] Inline expansion of memcmp

[Eli Friedman via Phabricator via llvm-commits]

efriedma accepted this revision.
efriedma added a comment.

Looks good! A few minor comments.  Please test carefully before merging.



================
Comment at: lib/CodeGen/CodeGenPrepare.cpp:2311
+  unsigned LoadSize = MaxLoadSize;
+  while (RemainingSize) {
+    NumLoads += RemainingSize / LoadSize;
----------------
I think this function is just `return (Size / MaxLoadSize) + countPopulation(Size % MaxLoadSize)`.


================
Comment at: lib/CodeGen/CodeGenPrepare.cpp:2319
+
+unsigned MemCmpExpansion::getLoadSize(unsigned Size) {
+  unsigned LoadSize = MaxLoadSize;
----------------
`return MinAlign(Size, MaxLoadSize)`?


================
Comment at: lib/CodeGen/CodeGenPrepare.cpp:2549
+  if (IsUsedForZeroCmp)
+    NumBlocks = ceil((float)NumBlocks / NumLoadsPerBlock);
+
----------------
Generally better to avoid floating-point for this sort of thing, especially since converting an 32-bit integer to a 32-bit float isn't exact.  I guess we don't have a helper in MathExtras for this; you can write something like like "NumBlocks / NumLoadsPerBlock + (NumBlocks % NumLoadsPerBlock != 0 ? 1 : 0)".


https://reviews.llvm.org/D28637