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

[Nemanja Ivanovic via Phabricator via llvm-commits]

nemanjai accepted this revision.
nemanjai added a comment.
This revision is now accepted and ready to land.

This seems OK to me (with the minor nits addressed on the commit). Since I've requested changes to a previous review, I believe that I have to approve the review to allow the change to go ahead. However, @efriedma has raised valid concerns previously and I think you should ensure that you get his approval prior to committing this.



================
Comment at: lib/CodeGen/CodeGenPrepare.cpp:89
+STATISTIC(NumMemCmpNotConstant, "Number of memcmp calls without constant size");
+STATISTIC(NumMemCmpGreaterThanMax, "Number of memcmp calls with size greater than max size");
+STATISTIC(NumMemCmpInlined, "Number of inlined memcmp calls");
----------------
This line is too long. I won't add further comments to this end, but please be sure to run clang-format-diff on this to fix up such issues.


================
Comment at: lib/CodeGen/CodeGenPrepare.cpp:2134
+  PHINode *PhiRes;
+  bool isUsedForZeroCmp;
+  int calculateNumBlocks(unsigned Size);
----------------
Capitals for variable names.


================
Comment at: lib/CodeGen/CodeGenPrepare.cpp:2169
+                                 unsigned NumLoadsPerBlock) {
+  this->CI = CI;
+  this->MaxLoadSize = MaxLoadSize;
----------------
Please initialize these in a member initializer list.


================
Comment at: lib/CodeGen/CodeGenPrepare.cpp:2289
+void MemCmpExpansion::emitLoadCompareBlockMultipleLoads(
+    unsigned Index, unsigned Size, unsigned &NumBytesProcessed) {
+
----------------
nit: formatting seems off here.


================
Comment at: lib/CodeGen/CodeGenPrepare.cpp:2305
+    unsigned GEPIndex = NumBytesProcessed / LoadSize;
+    NumBytesProcessed = NumBytesProcessed + LoadSize;
+    RemainingBytes = RemainingBytes - LoadSize;
----------------
Just use compound assignment (i.e. `+=`, `-=`) on this line and below.


https://reviews.llvm.org/D28637