[PATCH] D38498: [CodeGen][ExpandMemcmp][NFC] Allow memcmp to expand to vector loads, part 1.

[Sanjay Patel via Phabricator via llvm-commits]

spatel added a comment.

Sorry for the delay. I've made a few inline suggestions. I'm still going through the diffs to try to understand, but if you can reply/update on those, I think it will make it easier.



================
Comment at: lib/CodeGen/CodeGenPrepare.cpp:1731
+  // covering the interval with [{4, 0},{2, 4},{1, 6}}.
+  struct Entry {
+    Entry(unsigned LoadSize, uint64_t Offset)
----------------
"Entry" / "Entries" is vague - "LoadEntry" / "LoadSequence"?


================
Comment at: lib/CodeGen/CodeGenPrepare.cpp:1792-1794
+  while (this->MaxLoadSize > Size) {
+    this->MaxLoadSize /= 2;
+  }
----------------
This used to have log2 rather than a loop, right? I think it's easier to read like this, but I'd keep the comment to explain:
// Scale the max size down if the target can load more bytes than we need.


================
Comment at: lib/CodeGen/CodeGenPrepare.cpp:1912-1921
+  IntegerType *const MaxLoadType =
+      NumLoads == 1 ? nullptr
+                    : IntegerType::get(
+                          CI->getContext(),
+                          std::max_element(Entries.begin(), Entries.end(),
+                                           [](const Entry &a, const Entry &b) {
+                                             return a.LoadSize < b.LoadSize;
----------------
That's a tough "line" of code - split up with some local variables to make this more readable.


https://reviews.llvm.org/D38498