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

[Sanjay Patel via Phabricator via llvm-commits]

spatel added inline comments.


================
Comment at: test/Transforms/InstCombine/builtin_memcpy_patterns.ll:3-6
+target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+ at b = local_unnamed_addr global [16 x i32] [i32 3, i32 4, i32 5, i32 56, i32 6, i32 7, i32 78, i32 6, i32 3, i32 4, i32 54, i32 5, i32 1, i32 2, i32 3, i32 3], align 16
----------------
I still don't understand what value these tests provide over the existing ones. These are the same llvm.memcpy calls except they are in loops? The transform doesn't depend on loops from what I can tell, so why do we need to test this?


================
Comment at: test/Transforms/InstCombine/memcpy-to-load.ll:81
+
+; If there is no datalayout, then all memcpy of size less than 16 (and power-of-2) will be expanded inline with load/store
+
----------------
That is the existing behavior, but why is that valid now that we're using the datalayout?


================
Comment at: test/Transforms/InstCombine/memcpy-to-load.ll:93
+;
+; For datalayout with largest legal integer type size of 64, all memcpy with size less than 32 (and power-of-2) will be expanded inline with load/store
+;
----------------
You're mixing bytes and bits here. It makes sense to me that we would use the datalayout to drive this transform, but why are we then ignoring the datalayout and producing load + store of an illegal type (i128)?


https://reviews.llvm.org/D35035