[PATCH] Optimize memcmp(x, y, n)==0 for small n and suitably aligned x/y.

Eelis eelis at eelis.net
Wed Jan 14 06:59:06 PST 2015


Thanks David! I'll attach an updated patch, but I don't know how to get rid of the "8" constant.


================
Comment at: lib/Transforms/Utils/SimplifyLibCalls.cpp:955
@@ +954,3 @@
+  // (And same for 16/32 bit.)
+  if (Len <= 8 && !(Len & (Len-1))
+      && isOnlyUsedInZeroEqualityComparison(CI)
----------------
majnemer wrote:
> Isn't the RHS just `isPowerOf2_64`?
> 
> Also, '8' sounds awfully target specific.  I doubt this is the correct number for the MSP430 target.
Oh, I just took this from InstCombiner::SimplifyMemTransfer, which does:

  if (Size > 8 || (Size&(Size-1)))
    return nullptr;  // If not 1/2/4/8 bytes, exit.

I'll use isPowerOf2_64 instead, but I'm not sure what I should I use instead of 8.

================
Comment at: lib/Transforms/Utils/SimplifyLibCalls.cpp:956-958
@@ +955,5 @@
+  if (Len <= 8 && !(Len & (Len-1))
+      && isOnlyUsedInZeroEqualityComparison(CI)
+      && getKnownAlignment(LHS, DL, nullptr, CI) >= Len
+      && getKnownAlignment(RHS, DL, nullptr, CI) >= Len) {
+
----------------
majnemer wrote:
> This seems strangely formatted, consider running clang-format over these lines.
I like the &&s aligned on the left so I easily see it's a repeated conjunction, but I see that clang-format wants them on the right, so I'll do that.

================
Comment at: lib/Transforms/Utils/SimplifyLibCalls.cpp:960
@@ +959,3 @@
+
+    if (Type *IntType = IntegerType::get(CI->getContext(), Len << 3)) {
+
----------------
majnemer wrote:
> There is no need to manually strength-reduce multiplication by eight.  Also, this can never fail.
Oh, I just took the "Len << 3" from InstCombiner::SimplifyMemTransfer. Will fix. :)

================
Comment at: lib/Transforms/Utils/SimplifyLibCalls.cpp:962-965
@@ +961,6 @@
+
+      Type *LHSPtrTy = PointerType::get(IntType,
+        cast<PointerType>(LHS->getType())->getAddressSpace());
+      Type *RHSPtrTy = PointerType::get(IntType,
+        cast<PointerType>(RHS->getType())->getAddressSpace());
+
----------------
majnemer wrote:
> Perhaps:
>   Type *LHSPtrTy =
>       IntType->getPointerTo(LHS->getType()->getPointerAddressSpace());
>   Type *RHSPtrTy =
>       IntType->getPointerTo(RHS->getType()->getPointerAddressSpace());
This too I just took from InstCombiner::SimplifyMemTransfer. I will use what you suggest instead.

http://reviews.llvm.org/D6952

EMAIL PREFERENCES
  http://reviews.llvm.org/settings/panel/emailpreferences/






More information about the llvm-commits mailing list