From 631303d7645b58bb8a2f19a48c7b13aafb965eed Mon Sep 17 00:00:00 2001 From: Martin Kinkelin Date: Sun, 21 Nov 2021 01:14:31 +0100 Subject: [PATCH 1/4] Lower all non-memcmp-able array equality comparisons to __equals template --- src/dmd/expressionsem.d | 104 +++++++++++++++++------- test/fail_compilation/diag7420.d | 2 +- test/fail_compilation/verifyhookexist.d | 2 +- 3 files changed, 76 insertions(+), 32 deletions(-) diff --git a/src/dmd/expressionsem.d b/src/dmd/expressionsem.d index a1b42c57ba32..123ab0983d13 100644 --- a/src/dmd/expressionsem.d +++ b/src/dmd/expressionsem.d @@ -11472,54 +11472,85 @@ private extern (C++) final class ExpressionSemanticVisitor : Visitor } } - Type t1 = exp.e1.type.toBasetype(); - Type t2 = exp.e2.type.toBasetype(); + if (auto e = exp.op_overload(sc)) + { + result = e; + return; + } + // Indicates whether the comparison of the 2 specified array types - // requires an object.__equals() lowering. - static bool needsDirectEq(Type t1, Type t2, Scope* sc) + // can be implemented with a memcmp call in the glue layer instead + // of lowering to object.__equals(). + static bool canMemcmpArrays(const ref Loc loc, Type t1, Type t2) { Type t1n = t1.nextOf().toBasetype(); Type t2n = t2.nextOf().toBasetype(); - if ((t1n.ty.isSomeChar && t2n.ty.isSomeChar) || - (t1n.ty == Tvoid || t2n.ty == Tvoid)) + + static Type getBaseElementType(Type t) { - return false; + t = t.baseElemOf(); + if (auto tv = t.isTypeVector()) + t = tv.elementType(); + if (t.ty == Tvoid) + t = Type.tuns8; + return t; } - if (t1n.constOf() != t2n.constOf()) - return true; - Type t = t1n; - while (t.toBasetype().nextOf()) - t = t.nextOf().toBasetype(); - if (auto ts = t.isTypeStruct()) + Type e1 = getBaseElementType(t1n); + Type e2 = getBaseElementType(t2n); + if (e1.isintegral() && e2.isintegral()) { - // semanticTypeInfo() makes sure hasIdentityEquals has been computed - if (global.params.useTypeInfo && Type.dtypeinfo) - semanticTypeInfo(sc, ts); + // element sizes must match + if (t1n.size(loc) != t2n.size(loc)) + return false; + + // base integer sizes too + const size = e1.size(loc); + if (size != e2.size(loc)) + return false; - return ts.sym.hasIdentityEquals; // has custom opEquals + // integers < 4 bytes are promoted to int => no memcmp for diverging signed-ness + return size >= 4 || e1.isunsigned() == e2.isunsigned(); } - return false; - } + if ((t1n.ty == Tpointer && t2n.ty == Tpointer) || + (t1n.ty == Tfunction && t2n.ty == Tfunction) || + (t1n.ty == Tdelegate && t2n.ty == Tdelegate)) + { + return t1n.equivalent(t2n); + } - if (auto e = exp.op_overload(sc)) - { - result = e; - return; + return false; } + Type t1 = exp.e1.type.toBasetype(); + Type t2 = exp.e2.type.toBasetype(); const isArrayComparison = (t1.ty == Tarray || t1.ty == Tsarray) && (t2.ty == Tarray || t2.ty == Tsarray); - const needsArrayLowering = isArrayComparison && needsDirectEq(t1, t2, sc); + const needsArrayLowering = isArrayComparison && !canMemcmpArrays(exp.loc, t1, t2); - if (!needsArrayLowering) + // bring both sides to common type + bool doTypeCombine; + if (!isArrayComparison) + doTypeCombine = true; + else if (needsArrayLowering) + { + const ty1n = t1.nextOf().toBasetype().ty; + const ty2n = t2.nextOf().toBasetype().ty; + doTypeCombine = + // __equals e.g. supports comparing int[2][3] vs. short[][] - + // but still need to e.g. adjust empty array literals (`(string[] a) => a == []`, `[]` is void[]-typed) + (ty1n == Tvoid || ty2n == Tvoid) || + // and check for mismatching string-like types (e.g., string vs. wstring - no autodecoding) + (ty1n.isSomeChar && ty2n.isSomeChar); + } + if (doTypeCombine) { if (auto e = typeCombine(exp, sc)) { - result = e; + result = e; // error return; } } @@ -11529,6 +11560,17 @@ private extern (C++) final class ExpressionSemanticVisitor : Visitor if (f1 || f2) return setError(); + // check for mismatching lengths when comparing 2 static arrays + if (isArrayComparison) + if (auto ts1 = exp.e1.type.toBasetype().isTypeSArray()) + if (auto ts2 = exp.e2.type.toBasetype().isTypeSArray()) + if (ts1.dim.toUInteger() != ts2.dim.toUInteger()) + { + exp.error("incompatible types for array comparison: `%s` and `%s`", + exp.e1.type.toChars(), exp.e2.type.toChars()); + return setError(); + } + exp.type = Type.tbool; if (!isArrayComparison) @@ -11541,8 +11583,8 @@ private extern (C++) final class ExpressionSemanticVisitor : Visitor } } - // lower some array comparisons to object.__equals(e1, e2) - if (needsArrayLowering || (t1.ty == Tarray && t2.ty == Tarray)) + // lower non-memcmp-able array comparisons to object.__equals(e1, e2) + if (needsArrayLowering) { //printf("Lowering to __equals %s %s\n", exp.e1.toChars(), exp.e2.toChars()); @@ -11575,9 +11617,11 @@ private extern (C++) final class ExpressionSemanticVisitor : Visitor return; } - if (exp.e1.type.toBasetype().ty == Taarray) - semanticTypeInfo(sc, exp.e1.type.toBasetype()); + t1 = exp.e1.type.toBasetype(); + t2 = exp.e2.type.toBasetype(); + if (t1.ty == Taarray) + semanticTypeInfo(sc, t1); if (!target.isVectorOpSupported(t1, exp.op, t2)) { diff --git a/test/fail_compilation/diag7420.d b/test/fail_compilation/diag7420.d index 3267e6692432..9a31157ca84d 100644 --- a/test/fail_compilation/diag7420.d +++ b/test/fail_compilation/diag7420.d @@ -5,7 +5,6 @@ TEST_OUTPUT: fail_compilation/diag7420.d(21): Error: static variable `x` cannot be read at compile time fail_compilation/diag7420.d(21): while evaluating: `static assert(x < 4)` fail_compilation/diag7420.d(22): Error: static variable `y` cannot be read at compile time -fail_compilation/diag7420.d(22): called from here: `__equals(y, "abc")` fail_compilation/diag7420.d(22): while evaluating: `static assert(y == "abc")` fail_compilation/diag7420.d(23): Error: static variable `y` cannot be read at compile time fail_compilation/diag7420.d(23): while evaluating: `static assert(cast(ubyte[])y != null)` @@ -16,6 +15,7 @@ fail_compilation/diag7420.d(25): while evaluating: `static assert(y[0..1] --- */ + int x = 2; char[] y = "abc".dup; static assert(x < 4); diff --git a/test/fail_compilation/verifyhookexist.d b/test/fail_compilation/verifyhookexist.d index d7b8f6646c3b..035085ead70f 100644 --- a/test/fail_compilation/verifyhookexist.d +++ b/test/fail_compilation/verifyhookexist.d @@ -23,7 +23,7 @@ MyStruct[] castToMyStruct(int[] arr) { } void test() { - int[] arrA, arrB; + float[] arrA, arrB; bool a = arrA[] == arrB[]; bool b = arrA < arrB; From 6fb1ad5b598834a670a225926ab509f35a370118 Mon Sep 17 00:00:00 2001 From: Martin Kinkelin Date: Sun, 21 Nov 2021 02:52:17 +0100 Subject: [PATCH 2/4] Restore needsDirectEq() --- src/dmd/expressionsem.d | 71 +++++++++++++++++++++++++---------------- 1 file changed, 44 insertions(+), 27 deletions(-) diff --git a/src/dmd/expressionsem.d b/src/dmd/expressionsem.d index 123ab0983d13..9e4314bf5458 100644 --- a/src/dmd/expressionsem.d +++ b/src/dmd/expressionsem.d @@ -11479,6 +11479,35 @@ private extern (C++) final class ExpressionSemanticVisitor : Visitor } + // Indicates whether the comparison of the 2 specified array types + // requires an elision of typeCombine. + static bool needsDirectEq(Type t1, Type t2, Scope* sc) + { + Type t1n = t1.nextOf().toBasetype(); + Type t2n = t2.nextOf().toBasetype(); + if ((t1n.ty.isSomeChar && t2n.ty.isSomeChar) || + (t1n.ty == Tvoid || t2n.ty == Tvoid)) + { + return false; + } + if (t1n.constOf() != t2n.constOf()) + return true; + + Type t = t1n; + while (t.toBasetype().nextOf()) + t = t.nextOf().toBasetype(); + if (auto ts = t.isTypeStruct()) + { + // semanticTypeInfo() makes sure hasIdentityEquals has been computed + if (global.params.useTypeInfo && Type.dtypeinfo) + semanticTypeInfo(sc, ts); + + return ts.sym.hasIdentityEquals; // has custom opEquals + } + + return false; + } + // Indicates whether the comparison of the 2 specified array types // can be implemented with a memcmp call in the glue layer instead // of lowering to object.__equals(). @@ -11529,24 +11558,9 @@ private extern (C++) final class ExpressionSemanticVisitor : Visitor const isArrayComparison = (t1.ty == Tarray || t1.ty == Tsarray) && (t2.ty == Tarray || t2.ty == Tsarray); - const needsArrayLowering = isArrayComparison && !canMemcmpArrays(exp.loc, t1, t2); // bring both sides to common type - bool doTypeCombine; - if (!isArrayComparison) - doTypeCombine = true; - else if (needsArrayLowering) - { - const ty1n = t1.nextOf().toBasetype().ty; - const ty2n = t2.nextOf().toBasetype().ty; - doTypeCombine = - // __equals e.g. supports comparing int[2][3] vs. short[][] - - // but still need to e.g. adjust empty array literals (`(string[] a) => a == []`, `[]` is void[]-typed) - (ty1n == Tvoid || ty2n == Tvoid) || - // and check for mismatching string-like types (e.g., string vs. wstring - no autodecoding) - (ty1n.isSomeChar && ty2n.isSomeChar); - } - if (doTypeCombine) + if (!isArrayComparison || !needsDirectEq(t1, t2, sc)) { if (auto e = typeCombine(exp, sc)) { @@ -11560,16 +11574,19 @@ private extern (C++) final class ExpressionSemanticVisitor : Visitor if (f1 || f2) return setError(); - // check for mismatching lengths when comparing 2 static arrays - if (isArrayComparison) - if (auto ts1 = exp.e1.type.toBasetype().isTypeSArray()) - if (auto ts2 = exp.e2.type.toBasetype().isTypeSArray()) - if (ts1.dim.toUInteger() != ts2.dim.toUInteger()) - { - exp.error("incompatible types for array comparison: `%s` and `%s`", - exp.e1.type.toChars(), exp.e2.type.toChars()); - return setError(); - } + version (none) + { + // check for mismatching lengths when comparing 2 static arrays + if (isArrayComparison) + if (auto ts1 = exp.e1.type.toBasetype().isTypeSArray()) + if (auto ts2 = exp.e2.type.toBasetype().isTypeSArray()) + if (ts1.dim.toUInteger() != ts2.dim.toUInteger()) + { + exp.error("incompatible types for array comparison: `%s` and `%s`", + exp.e1.type.toChars(), exp.e2.type.toChars()); + return setError(); + } + } exp.type = Type.tbool; @@ -11584,7 +11601,7 @@ private extern (C++) final class ExpressionSemanticVisitor : Visitor } // lower non-memcmp-able array comparisons to object.__equals(e1, e2) - if (needsArrayLowering) + if (isArrayComparison && !canMemcmpArrays(exp.loc, exp.e1.type.toBasetype(), exp.e2.type.toBasetype())) { //printf("Lowering to __equals %s %s\n", exp.e1.toChars(), exp.e2.toChars()); From ac9026c10f3519cc990ae61de9600888eda3a782 Mon Sep 17 00:00:00 2001 From: Martin Kinkelin Date: Sun, 21 Nov 2021 15:32:41 +0100 Subject: [PATCH 3/4] Fix runnable/sdtor.d assertions wrt. destructed array literals --- test/runnable/sdtor.d | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/test/runnable/sdtor.d b/test/runnable/sdtor.d index 56c5125cdaf0..fe7916b0a58d 100644 --- a/test/runnable/sdtor.d +++ b/test/runnable/sdtor.d @@ -3935,9 +3935,9 @@ bool test14022() t.sb[1].x = 'y'; assert(sa == [S('a'), S('b'), S('c')]); assert(t.sb == [S('x'), S('y')]); - assert(op == "BC"); + assert(op == "BCcbcbayx"); } - assert(op == "BCyxcba"); + assert(op == "BCcbcbayxyxcba"); op = null; { @@ -3948,7 +3948,7 @@ bool test14022() assert(op == "BxCy"); assert(t.sb == [S('b'), S('c')]); } - assert(op == "BxCycbcba"); + assert(op == "BxCycbcbcba"); return true; } @@ -4017,9 +4017,9 @@ bool test14023() { S[3] sa = [S('a'), S('b'), S('c')]; test(sa[1..3]); - assert(op == "BxCx"); + assert(op == "BxCxcb"); } - assert(op == "BxCxcba"); + assert(op == "BxCxcbcba"); return true; } From 438c73aa182c1aa89b93200149dff68f74eac4ad Mon Sep 17 00:00:00 2001 From: Martin Kinkelin Date: Sun, 21 Nov 2021 15:33:47 +0100 Subject: [PATCH 4/4] Adapt glue layer to all array EqualExp being memcmp-able Thus never calling `_adEq2` anymore. --- src/dmd/backend/drtlsym.d | 1 - src/dmd/backend/rtlsym.d | 2 +- src/dmd/e2ir.d | 132 ++++++++++++++++---------------------- src/dmd/inline.d | 10 +-- 4 files changed, 59 insertions(+), 86 deletions(-) diff --git a/src/dmd/backend/drtlsym.d b/src/dmd/backend/drtlsym.d index b31172e2e02f..bb207b4c35f9 100644 --- a/src/dmd/backend/drtlsym.d +++ b/src/dmd/backend/drtlsym.d @@ -171,7 +171,6 @@ Symbol *getRtlsym(RTLSYM i) case RTLSYM.ARRAYCTOR: symbolz(ps,FLfunc,FREGSAVED,"_d_arrayctor", 0, t); break; case RTLSYM.ARRAYSETASSIGN: symbolz(ps,FLfunc,FREGSAVED,"_d_arraysetassign", 0, t); break; case RTLSYM.ARRAYSETCTOR: symbolz(ps,FLfunc,FREGSAVED,"_d_arraysetctor", 0, t); break; - case RTLSYM.ARRAYEQ2: symbolz(ps,FLfunc,FREGSAVED,"_adEq2", 0, t); break; case RTLSYM.ARRAYCMPCHAR: symbolz(ps,FLfunc,FREGSAVED,"_adCmpChar", 0, t); break; case RTLSYM.EXCEPT_HANDLER2: symbolz(ps,FLfunc,fregsaved,"_except_handler2", 0, tsclib); break; diff --git a/src/dmd/backend/rtlsym.d b/src/dmd/backend/rtlsym.d index ce912aaba981..d3afb7b7400f 100644 --- a/src/dmd/backend/rtlsym.d +++ b/src/dmd/backend/rtlsym.d @@ -99,7 +99,7 @@ enum RTLSYM ARRAYSETCTOR, ARRAYCAST, // unused ARRAYEQ, // unused - ARRAYEQ2, + ARRAYEQ2, // unused ARRAYCMP, // unused ARRAYCMP2, // unused ARRAYCMPCHAR, // unused diff --git a/src/dmd/e2ir.d b/src/dmd/e2ir.d index b709b2dcb2f6..15ea8c689464 100644 --- a/src/dmd/e2ir.d +++ b/src/dmd/e2ir.d @@ -1859,89 +1859,71 @@ extern (C++) class ToElemVisitor : Visitor else if ((t1.ty == Tarray || t1.ty == Tsarray) && (t2.ty == Tarray || t2.ty == Tsarray)) { - 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 && (a.length == 0 || memcmp(a.ptr, b.ptr, size)==0) - // For a!=b: a.length!=b.length || (a.length != 0 || 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 = toElem(ee.e1, irs); - elem* earr2 = toElem(ee.e2, 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(target.is64bit ? 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, (cast(TypeSArray)t1).dim.toInteger()); - esiz1 = el_long(TYsize_t, t1.size()); - earr1 = addressElem(earr1, t1); - eptr1 = el_same(&earr1); - } + // Only memcmp-able array comparisons make it to the glue layer + // (others are lowered to object.__equals). Implement via: + // * for a==b: a.length==b.length && (a.length == 0 || memcmp(a.ptr, b.ptr, size)==0) + // * for a!=b: a.length!=b.length || (a.length != 0 && memcmp(a.ptr, b.ptr, size)!=0) + // size is a.length*sizeof(a[0]) for dynamic arrays, or sizeof(a) for static arrays. - if (t2.ty == Tarray) - { - elen2 = el_una(target.is64bit ? 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, (cast(TypeSArray)t2).dim.toInteger()); - esiz2 = el_long(TYsize_t, t2.size()); - earr2 = addressElem(earr2, t2); - eptr2 = el_same(&earr2); - } + elem* earr1 = toElem(ee.e1, irs); + elem* earr2 = toElem(ee.e2, 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 = t1.nextOf().toBasetype().size(); // Size of one element + + if (t1.ty == Tarray) + { + elen1 = el_una(target.is64bit ? 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, (cast(TypeSArray)t1).dim.toInteger()); + esiz1 = el_long(TYsize_t, t1.size()); + earr1 = addressElem(earr1, t1); + eptr1 = el_same(&earr1); + } - elem *esize = t2.ty == Tsarray ? esiz2 : esiz1; + if (t2.ty == Tarray) + { + elen2 = el_una(target.is64bit ? 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, (cast(TypeSArray)t2).dim.toInteger()); + esiz2 = el_long(TYsize_t, t2.size()); + earr2 = addressElem(earr2, t2); + eptr2 = el_same(&earr2); + } - e = el_param(eptr1, eptr2); - e = el_bin(OPmemcmp, TYint, e, esize); - e = el_bin(eop, TYint, e, el_long(TYint, 0)); + elem *esize = t2.ty == Tsarray ? esiz2 : esiz1; - elem *elen = t2.ty == Tsarray ? elen2 : elen1; - elem *esizecheck = el_bin(eop, TYint, el_same(&elen), el_long(TYsize_t, 0)); - e = el_bin(ee.op == TOK.equal ? OPoror : OPandand, TYint, esizecheck, e); + 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(ee.op == TOK.equal ? OPandand : OPoror, TYint, elencmp, e); - } + elem *elen = t2.ty == Tsarray ? elen2 : elen1; + elem *esizecheck = el_bin(eop, TYint, el_same(&elen), el_long(TYsize_t, 0)); + e = el_bin(ee.op == TOK.equal ? OPoror : OPandand, TYint, esizecheck, e); - // Ensure left-to-right order of evaluation - e = el_combine(earr2, e); - e = el_combine(earr1, e); - elem_setLoc(e, ee.loc); - result = e; - return; + if (t1.ty == Tsarray && t2.ty == Tsarray) + assert(t1.size() == t2.size()); + else + { + elem *elencmp = el_bin(eop, TYint, elen1, elen2); + e = el_bin(ee.op == TOK.equal ? OPandand : OPoror, TYint, elencmp, e); } - elem *ea1 = eval_Darray(ee.e1); - elem *ea2 = eval_Darray(ee.e2); - - elem *ep = el_params(getTypeInfo(ee.loc, telement.arrayOf(), irs), - ea2, ea1, null); - const rtlfunc = RTLSYM.ARRAYEQ2; - e = el_bin(OPcall, TYint, el_var(getRtlsym(rtlfunc)), ep); - if (ee.op == TOK.notEqual) - e = el_bin(OPxor, TYint, e, el_long(TYint, 1)); - elem_setLoc(e,ee.loc); + // Ensure left-to-right order of evaluation + e = el_combine(earr2, e); + e = el_combine(earr1, e); + elem_setLoc(e, ee.loc); + result = e; + return; } else if (t1.ty == Taarray && t2.ty == Taarray) { diff --git a/src/dmd/inline.d b/src/dmd/inline.d index fa182afc1988..7c916293ff63 100644 --- a/src/dmd/inline.d +++ b/src/dmd/inline.d @@ -793,15 +793,7 @@ public: visit(cast(BinExp)e); Type t1 = e.e1.type.toBasetype(); - if (t1.ty == Tarray || t1.ty == Tsarray) - { - Type t = t1.nextOf().toBasetype(); - while (t.toBasetype().nextOf()) - t = t.nextOf().toBasetype(); - if (t.ty == Tstruct) - semanticTypeInfo(null, t); - } - else if (t1.ty == Taarray) + if (t1.ty == Taarray) { semanticTypeInfo(null, t1); }