[pkg-d-commits] [ldc] 68/95: Implement optimization: compare slices using memcmp if valid (#2047)

Matthias Klumpp mak at moszumanska.debian.org
Thu Jul 13 20:54:02 UTC 2017


This is an automated email from the git hooks/post-receive script.

mak pushed a commit to annotated tag v1.3.0-beta1
in repository ldc.

commit a4055962fee3665758d33182fdbaa4855f2f1a4f
Author: Johan Engelen <jbc.engelen at gmail.com>
Date:   Wed Mar 29 20:00:21 2017 +0200

    Implement optimization: compare slices using memcmp if valid (#2047)
    
    This also fixes a pessimization where the `memcmp` call would become an `invoke` if the user provides his own `memcmp` prototype in the code (the prototype would not carry the `nounwind` function attribute).
---
 gen/arrays.cpp                                  | 78 ++++++++++++++++++-------
 tests/codegen/array_equals_memcmp_2.d           | 11 ++++
 tests/codegen/array_equals_memcmp_dyn.d         | 44 ++++++++++++++
 tests/codegen/array_equals_memcmp_neverinvoke.d | 24 ++++++++
 4 files changed, 135 insertions(+), 22 deletions(-)

diff --git a/gen/arrays.cpp b/gen/arrays.cpp
index ed7ee1e..bc5b39b 100644
--- a/gen/arrays.cpp
+++ b/gen/arrays.cpp
@@ -1046,15 +1046,12 @@ bool validCompareWithMemcmpType(Type *t) {
 /// - User-defined opEquals
 bool validCompareWithMemcmp(DValue *l, DValue *r) {
   auto *ltype = l->type->toBasetype();
-
-  // TODO: Remove this check once `DtoArrayEqCmp_memcmp` can handle dynamic
-  // array comparisons.
-  if (ltype->ty != Tsarray)
-    return false;
-
   auto *rtype = r->type->toBasetype();
+
   // Only memcmp equivalent types (memcmp should be used for `const int[3] ==
   // int[3]`, but not for `int[3] == short[3]`).
+  // Note: Type::equivalent returns true for `int[4]` and `int[]`, and also for
+  // `int[4]` and `int[3]`! That is exactly what we want in this case.
   if (!ltype->equivalent(rtype))
     return false;
 
@@ -1062,32 +1059,69 @@ bool validCompareWithMemcmp(DValue *l, DValue *r) {
   return validCompareWithMemcmpType(elemType);
 }
 
+// Create a call instruction to memcmp.
+llvm::CallInst *callMemcmp(Loc &loc, IRState &irs, LLValue *l_ptr,
+                           LLValue *r_ptr, LLValue *numElements) {
+  assert(l_ptr && r_ptr && numElements);
+  LLFunction *fn = getRuntimeFunction(loc, gIR->module, "memcmp");
+  assert(fn);
+  auto sizeInBytes = numElements;
+  size_t elementSize = getTypeAllocSize(l_ptr->getType()->getContainedType(0));
+  if (elementSize != 1) {
+    sizeInBytes = irs.ir->CreateMul(sizeInBytes, DtoConstSize_t(elementSize));
+  }
+  // Call memcmp.
+  LLValue *args[] = {DtoBitCast(l_ptr, getVoidPtrType()),
+                     DtoBitCast(r_ptr, getVoidPtrType()), sizeInBytes};
+  return irs.ir->CreateCall(fn, args);
+}
+
 /// Compare `l` and `r` using memcmp. No checks are done for validity.
 ///
-/// This function can currently only deal with comparisons of static arrays
+/// This function can deal with comparisons of static and dynamic arrays
 /// with memcmp.
-/// TODO: Implement dynamic array comparison: length equality check
-/// (and perhaps pointer equality check) before memcmp.
+///
+/// Note: the dynamic array length check is not covered by (LDC's) PGO.
 LLValue *DtoArrayEqCmp_memcmp(Loc &loc, DValue *l, DValue *r, IRState &irs) {
   IF_LOG Logger::println("Comparing arrays using memcmp");
-  LLFunction *fn = getRuntimeFunction(loc, gIR->module, "memcmp");
-  assert(fn);
-
-  // TODO: Remove this check once dynamic arrays are correctly dealt with.
-  assert(l->type->toBasetype()->ty == Tsarray);
 
   auto *l_ptr = DtoArrayPtr(l);
   auto *r_ptr = DtoArrayPtr(r);
-
-  auto *size = DtoArrayLen(l);
-  size_t elementSize = getTypeAllocSize(l_ptr->getType()->getContainedType(0));
-  if (elementSize != 1) {
-    size = irs.ir->CreateMul(size, DtoConstSize_t(elementSize));
+  auto *l_length = DtoArrayLen(l);
+
+  // Early return for the simple case of comparing two static arrays.
+  const bool staticArrayComparison = (l->type->toBasetype()->ty == Tsarray) &&
+                                     (r->type->toBasetype()->ty == Tsarray);
+  if (staticArrayComparison) {
+    // TODO: simply codegen when comparing static arrays with different length (int[3] == int[2])
+    return callMemcmp(loc, irs, l_ptr, r_ptr, l_length);
   }
 
-  LLValue *args[] = {DtoBitCast(l_ptr, getVoidPtrType()),
-                     DtoBitCast(r_ptr, getVoidPtrType()), size};
-  return irs.funcGen().callOrInvoke(fn, args).getInstruction();
+  // First compare the array lengths
+  auto lengthsCompareEqual =
+      irs.ir->CreateICmp(llvm::ICmpInst::ICMP_EQ, l_length, DtoArrayLen(r));
+
+  llvm::BasicBlock *incomingBB = irs.scopebb();
+  llvm::BasicBlock *memcmpBB = irs.insertBB("domemcmp");
+  llvm::BasicBlock *memcmpEndBB = irs.insertBBAfter(memcmpBB, "memcmpend");
+  irs.ir->CreateCondBr(lengthsCompareEqual, memcmpBB, memcmpEndBB);
+
+  // If lengths are equal: call memcmp.
+  // Note: no extra null checks are needed before passing the pointers to memcmp.
+  // The array comparison is UB for non-zero length, and memcmp will correctly
+  // return 0 (equality) when the length is zero.
+  irs.scope() = IRScope(memcmpBB);
+  auto memcmpAnswer = callMemcmp(loc, irs, l_ptr, r_ptr, l_length);
+  irs.ir->CreateBr(memcmpEndBB);
+
+  // Merge the result of length check and memcmp call into a phi node.
+  irs.scope() = IRScope(memcmpEndBB);
+  llvm::PHINode *phi =
+      irs.ir->CreatePHI(LLType::getInt32Ty(gIR->context()), 2, "cmp_result");
+  phi->addIncoming(DtoConstInt(1), incomingBB);
+  phi->addIncoming(memcmpAnswer, memcmpBB);
+
+  return phi;
 }
 } // end anonymous namespace
 
diff --git a/tests/codegen/array_equals_memcmp_2.d b/tests/codegen/array_equals_memcmp_2.d
new file mode 100644
index 0000000..41fddfd
--- /dev/null
+++ b/tests/codegen/array_equals_memcmp_2.d
@@ -0,0 +1,11 @@
+// Tests that static array (in)equality of unequal lengths is optimized to `false`.
+
+// RUN: %ldc -c -O3 -output-ll -of=%t.ll %s && FileCheck %s --check-prefix=LLVM < %t.ll
+
+// LLVM-LABEL: define{{.*}} @{{.*}}different_lengths
+// ASM-LABEL: different_lengths{{.*}}:
+bool different_lengths(bool[4] a, bool[3] b)
+{
+    // LLVM: ret i1 false
+    return a == b;
+}
diff --git a/tests/codegen/array_equals_memcmp_dyn.d b/tests/codegen/array_equals_memcmp_dyn.d
new file mode 100644
index 0000000..26ca55f
--- /dev/null
+++ b/tests/codegen/array_equals_memcmp_dyn.d
@@ -0,0 +1,44 @@
+// Tests that dynamic array (in)equality is optimized to a memcmp call when valid.
+
+// RUN: %ldc -c -output-ll -of=%t.ll %s && FileCheck %s --check-prefix=LLVM < %t.ll
+// RUN: %ldc -c -output-s -O3 -of=%t.s  %s && FileCheck %s --check-prefix=ASM  < %t.s
+// RUN: %ldc -O0 -run %s
+// RUN: %ldc -O3 -run %s
+
+module mod;
+
+// LLVM-LABEL: define{{.*}} @{{.*}}static_dynamic
+// ASM-LABEL: static_dynamic{{.*}}:
+bool static_dynamic(bool[4] a, bool[] b)
+{
+    // LLVM: call i32 @memcmp(
+
+    // Also test that LLVM recognizes and optimizes-out the call to memcmp for 4 byte arrays:
+    // ASM-NOT: memcmp
+    return a == b;
+}
+
+// LLVM-LABEL: define{{.*}} @{{.*}}inv_dynamic_dynamic
+// ASM-LABEL: inv_dynamic_dynamic{{.*}}:
+bool inv_dynamic_dynamic(bool[] a, bool[] b)
+{
+    // LLVM: call i32 @memcmp(
+    return a != b;
+}
+
+void main()
+{
+    assert( static_dynamic([true, false, true, false], [true, false, true, false]));
+    assert(!static_dynamic([true, false, true, false], [true, false, true, true]));
+    assert(!static_dynamic([true, false, true, false], [true, false, true, false, true]));
+    assert(!static_dynamic([true, false, true, false], [true, false, true]));
+
+    assert(!inv_dynamic_dynamic([true, false, true, false], [true, false, true, false]));
+    assert( inv_dynamic_dynamic([true, false, true, false], [true, false, true, true]));
+    assert( inv_dynamic_dynamic([true, false], [true]));
+    assert( inv_dynamic_dynamic([true, false, true, false], [true, false, true]));
+
+    // Make sure that comparing zero-length arrays with ptr=null is allowed.
+    bool* ptr = null;
+    assert(!inv_dynamic_dynamic(ptr[0..0], ptr[0..0]));
+}
diff --git a/tests/codegen/array_equals_memcmp_neverinvoke.d b/tests/codegen/array_equals_memcmp_neverinvoke.d
new file mode 100644
index 0000000..7708f55
--- /dev/null
+++ b/tests/codegen/array_equals_memcmp_neverinvoke.d
@@ -0,0 +1,24 @@
+// Tests that memcmp array comparisons `call` memcmp instead of `invoke`.
+
+// RUN: %ldc -c -output-ll -of=%t.ll %s && FileCheck %s --check-prefix=LLVM < %t.ll
+
+// When the user defines memcmp, it overrides the prototype defined by LDC.
+// The user's prototype does not have the nounwind attribute, and a call to memcmp may become `invoke`.
+extern(C) int memcmp(void*, void*, size_t);
+
+void foo();
+
+// Test that memcmp is not `invoked`
+// LLVM-LABEL: define{{.*}} @{{.*}}never_invoke
+void never_invoke(bool[2] a, bool[2] b)
+{
+    try
+    {
+        // LLVM: call i32 @memcmp({{.*}}, {{.*}}, i{{32|64}} 2)
+        auto result = a == b;
+        foo(); // Compiler has to assume that this may throw
+    }
+    catch (Exception e)
+    {
+    }
+}

-- 
Alioth's /usr/local/bin/git-commit-notice on /srv/git.debian.org/git/pkg-d/ldc.git



More information about the pkg-d-commits mailing list