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

DIVYA SHANMUGHAN via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Aug 7 06:57:01 PDT 2017


DIVYA marked an inline comment as done.
DIVYA added inline comments.


================
Comment at: lib/Transforms/InstCombine/InstCombineCalls.cpp:185-186
 
   // If MemCpyInst length is 1/2/4/8 bytes then replace memcpy with
   // load/store.
   ConstantInt *MemOpLength = dyn_cast<ConstantInt>(MI->getArgOperand(2));
----------------
spatel wrote:
> This comment is wrong now? I'm still confused about the current behavior and what this patch is changing. 
> 
> Did you see my earlier comment to check in minimal tests to trunk before this patch so we have a baseline view of the current behavior? Do the tests really need loops, attributes, globals, etc?
> 
> If there are no legal types in the DL, why wouldn't we just bail out rather than assuming that 32-bit and smaller is safe/desirable to transform?
The test contains loops to show that builtin_memcpy within the loops , will also be converted to either memcpy or store and load operations depending on the maximum allowed stores


https://reviews.llvm.org/D35035





More information about the llvm-commits mailing list