Optimize simple array comparison (Issue 9477)

CyberShadow · Mar 26, 2013 · c984175 · c984175
1 parent c5286c3
commit c984175
Show file tree

Hide file tree

Showing 2 changed files with 136 additions and 1 deletion.
diff --git a/src/e2ir.c b/src/e2ir.c
@@ -2475,7 +2475,73 @@ elem *EqualExp::toElem(IRState *irs)
     else if ((t1->ty == Tarray || t1->ty == Tsarray) &&
              (t2->ty == Tarray || t2->ty == Tsarray))
     {
-        Type *telement = t1->nextOf()->toBasetype();
+        Type *telement  = t1->nextOf()->toBasetype();
+        Type *telement2 = t2->nextOf()->toBasetype();
+
+        if ((telement->isintegral() || telement->ty == Tvoid) && telement->ty == telement2->ty)
+        {
+            // Optimize comparisons of arrays of basic types
+            // For arrays of integers/characters, and void[],
+            // replace druntime call with:
+            // For a==b: a.length==b.length && memcmp(a.ptr, b.ptr, size)==0
+            // For a!=b: a.length!=b.length || memcmp(a.ptr, b.ptr, size)!=0
+            // size is a.length*sizeof(a[0]) for dynamic arrays, or sizeof(a) for static arrays.
+
+            elem *earr1 = e1->toElem(irs);
+            elem *earr2 = e2->toElem(irs);
+            elem *eptr1, *eptr2; // Pointer to data, to pass to memcmp
+            elem *elen1, *elen2; // Length, for comparison
+            elem *esiz1, *esiz2; // Data size, to pass to memcmp
+            d_uns64 sz = telement->size(); // Size of one element
+
+            if (t1->ty == Tarray)
+            {
+                elen1 = el_una(I64 ? OP128_64 : OP64_32, TYsize_t, el_same(&earr1));
+                esiz1 = el_bin(OPmul, TYsize_t, el_same(&elen1), el_long(TYsize_t, sz));
+                eptr1 = array_toPtr(t1, el_same(&earr1));
+            }
+            else
+            {
+                elen1 = el_long(TYsize_t, ((TypeSArray *)t1)->dim->toInteger());
+                esiz1 = el_long(TYsize_t, t1->size());
+                earr1 = addressElem(earr1, t1);
+                eptr1 = el_same(&earr1);
+            }
+
+            if (t2->ty == Tarray)
+            {
+                elen2 = el_una(I64 ? OP128_64 : OP64_32, TYsize_t, el_same(&earr2));
+                esiz2 = el_bin(OPmul, TYsize_t, el_same(&elen2), el_long(TYsize_t, sz));
+                eptr2 = array_toPtr(t2, el_same(&earr2));
+            }
+            else
+            {
+                elen2 = el_long(TYsize_t, ((TypeSArray *)t2)->dim->toInteger());
+                esiz2 = el_long(TYsize_t, t2->size());
+                earr2 = addressElem(earr2, t2);
+                eptr2 = el_same(&earr2);
+            }
+
+            elem *esize = t2->ty == Tsarray ? esiz2 : esiz1;
+
+            e = el_param(eptr1, eptr2);
+            e = el_bin(OPmemcmp, TYint, e, esize);
+            e = el_bin(eop, TYint, e, el_long(TYint, 0));
+
+            if (t1->ty == Tsarray && t2->ty == Tsarray)
+                assert(t1->size() == t2->size());
+            else
+            {
+                elem *elencmp = el_bin(eop, TYint, elen1, elen2);
+                e = el_bin(op==TOKequal ? OPandand : OPoror, TYint, elencmp, e);
+            }
+
+            // Ensure left-to-right order of evaluation
+            e = el_combine(earr2, e);
+            e = el_combine(earr1, e);
+            el_setLoc(e,loc);
+            return e;
+        }
 
         elem *ea1 = eval_Darray(irs, e1);
         elem *ea2 = eval_Darray(irs, e2);

diff --git a/test/runnable/xtest46.d b/test/runnable/xtest46.d
@@ -6027,6 +6027,74 @@ void test9428()
     assert(r4[1][1] == 2);
 }
 
+/***************************************************/
+// 9477
+
+template Tuple9477(T...) { alias T Tuple9477; }
+template Select9477(bool b, T, U) { static if (b) alias T Select9477; else alias U Select9477; }
+
+void test9477()
+{
+    static bool isEq (T1, T2)(T1 s1, T2 s2) { return s1 == s2; }
+    static bool isNeq(T1, T2)(T1 s1, T2 s2) { return s1 != s2; }
+
+    // Must be outside the loop due to http://d.puremagic.com/issues/show_bug.cgi?id=9748
+    int order;
+    // Must be outside the loop due to http://d.puremagic.com/issues/show_bug.cgi?id=9756
+    auto checkOrder(bool dyn, uint expected)()
+    {
+        assert(order==expected);
+        order++;
+        // Use temporary ("v") to work around http://d.puremagic.com/issues/show_bug.cgi?id=9402
+        auto v = cast(Select9477!(dyn, string, char[1]))"a";
+        return v;
+    }
+
+    foreach (b1; Tuple9477!(false, true))
+        foreach (b2; Tuple9477!(false, true))
+        {
+            version (D_PIC) {} else // Work around http://d.puremagic.com/issues/show_bug.cgi?id=9754
+            {
+                assert( isEq (cast(Select9477!(b1, string, char[0]))"" , cast(Select9477!(b2, string, char[0]))""  ));
+                assert(!isNeq(cast(Select9477!(b1, string, char[0]))"" , cast(Select9477!(b2, string, char[0]))""  ));
+
+                assert(!isEq (cast(Select9477!(b1, string, char[0]))"" , cast(Select9477!(b2, string, char[1]))"a" ));
+                assert( isNeq(cast(Select9477!(b1, string, char[0]))"" , cast(Select9477!(b2, string, char[1]))"a" ));
+            }
+
+            assert( isEq (cast(Select9477!(b1, string, char[1]))"a", cast(Select9477!(b2, string, char[1]))"a" ));
+            assert(!isNeq(cast(Select9477!(b1, string, char[1]))"a", cast(Select9477!(b2, string, char[1]))"a" ));
+
+            assert(!isEq (cast(Select9477!(b1, string, char[1]))"a", cast(Select9477!(b2, string, char[1]))"b" ));
+            assert( isNeq(cast(Select9477!(b1, string, char[1]))"a", cast(Select9477!(b2, string, char[1]))"b" ));
+
+            assert(!isEq (cast(Select9477!(b1, string, char[1]))"a", cast(Select9477!(b2, string, char[2]))"aa"));
+            assert( isNeq(cast(Select9477!(b1, string, char[1]))"a", cast(Select9477!(b2, string, char[2]))"aa"));
+
+            // Note: order of evaluation was not followed before this patch
+            // (thus, the test below will fail without the patch).
+            // Although the specification mentions that as implementation-defined behavior,
+            // I understand that this isn't by design, but rather an inconvenient aspect of DMD
+            // that has been moved to the specification.
+            order = 0;
+            bool result = checkOrder!(b1, 0)() == checkOrder!(b2, 1)();
+            assert(result);
+            assert(order == 2);
+        }
+
+    ubyte[64] a1, a2;
+    foreach (T; Tuple9477!(void, ubyte, ushort, uint, ulong, char, wchar, dchar, float, double))
+    {
+        auto s1 = cast(T[])(a1[]);
+        auto s2 = cast(T[])(a2[]);
+        assert(s1 == s2);
+        a2[$-1]++;
+        assert(s1 != s2);
+        assert(s1[0..$-1]==s2[0..$-1]);
+        a2[$-1]--;
+    }
+}
+
 /***************************************************/
 // 9504
 
@@ -6365,6 +6433,7 @@ int main()
     test8945();
     test163();
     test9428();
+    test9477();
     test9538();
     test9700();