[PATCH] D35035: [InstCombine] Prevent memcpy generation for small data size

[Sanjay Patel via Phabricator via llvm-commits]

spatel added inline comments.


================
Comment at: lib/Transforms/InstCombine/InstCombineCalls.cpp:202-203
+  unsigned LargestInt = DL.getLargestLegalIntTypeSizeInBits();
+  if (LargestInt == 0)
+    LargestInt = 32;
+
----------------
Why 32? I'm not sure what it means if there are no legal types in the DL. Please add a code comment to explain.


================
Comment at: lib/Transforms/InstCombine/InstCombineCalls.cpp:206
+  if (Size > 2*LargestInt/8 || (Size&(Size-1)))
     return nullptr;  // If not 1/2/4/8 bytes, exit.
 
----------------
This code comment isn't correct any more.


================
Comment at: test/Transforms/InstCombine/builtin_memcpy_pattern.ll:1
+; RUN: opt -O2  < %s -S | FileCheck %s
+
----------------
1. The test should be limited to -instcombine, not -O2.
2. The tests here have way too much extraneous stuff. Please minimize to just the IR necessary to show the transform. 
3. You can check the tests in ahead of this patch to show the current IR, then this patch will just show the diffs from the existing code.
4. Please use the script at util/update_tests_checks.py to auto-generate the complete CHECK lines.


https://reviews.llvm.org/D35035