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

Aditya Kumar via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Sun Sep 16 23:54:26 PDT 2018


hiraditya added inline comments.


================
Comment at: lib/Transforms/InstCombine/InstCombineCalls.cpp:202-203
+  unsigned LargestInt = DL.getLargestLegalIntTypeSizeInBits();
+  if (LargestInt == 0)
+    LargestInt = 32;
+
----------------
craig.topper wrote:
> hiraditya wrote:
> > spatel wrote:
> > > 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.
> > > Why 32?
> > 32 because we want to default to 8, same as the previous behavior before this patch.
> > 
> > > I'm not sure what it means if there are no legal types in the DL. Please add a code comment to explain.
> > For example, in test cases when the target datalayout is not specified, DL.getLargestLegalIntTypeSizeInBits() returns 0.
> > 
> But the previous behavior allowed 8 bytes so shouldn't it be 64?
It's been a while when we submitted the patch but I think you're right unless @DIVYA has some comments. We'll make the changes.


================
Comment at: lib/Transforms/InstCombine/InstCombineCalls.cpp:195
 
-  if (Size > 8 || (Size&(Size-1)))
-    return nullptr;  // If not 1/2/4/8 bytes, exit.
+
+  // Since we don't have perfect knowledge here, make some assumptions: assume
----------------
craig.topper wrote:
> Is there an extra blank line here?
We'll fix this.


https://reviews.llvm.org/D35035





More information about the llvm-commits mailing list