Templatize _adEq2

[Albert24GG]

This PR replaces all uses of _adEq2 with the existing __equals templated hook, thus closing draft PR #13331.

There may be room for some cleanups or optimizations, so let me know your thoughts.

[dlang-bot]

Thanks for your pull request and interest in making D better, @Albert24GG! We are looking forward to reviewing it, and you should be hearing from a maintainer soon.
Please verify that your PR follows this checklist:

• My PR is fully covered with tests (you can see the coverage diff by visiting the details link of the codecov check)
• My PR is as minimal as possible (smaller, focused PRs are easier to review than big ones)
• I have provided a detailed rationale explaining my changes
• New or modified functions have Ddoc comments (with Params: and Returns:)

Please see CONTRIBUTING.md for more information.

---

If you have addressed all reviews or aren't sure how to proceed, don't hesitate to ping us with a simple comment.

Bugzilla references

Your PR doesn't reference any Bugzilla issue.

If your PR contains non-trivial changes, please reference a Bugzilla issue or create a manual changelog.

Testing this PR locally

If you don't have a local development environment setup, you can use Digger to test this PR:

dub run digger -- build "master + dmd#21513"

[thewilsonator]

why remove the attributes here?

[Albert24GG]

All checks are done before lowering.

[Albert24GG]

@thewilsonator Because of the fact that now the result expression is the original one with a .lowering field that is set to null when memcmp is used (the lowering to memcmp happens in e2ir.d), the GC checks will be done on the original expression. This hack allows to preserve the current upstream behavior by avoiding flagging the array literals as @gc. Is there a better way of achieving the same goal, but without this type of hacks?

[thewilsonator]

silly question: what's this change for? ditto for the rest of these makedeps changes

[Albert24GG]

The previous refs expected the test output not to include the last line from the makedeps output. If you run -makedeps you will se that all line except the last have a '' at the end. I tried writing something more elegant using the existing regex capabilities, but I couldn't get it right because of the weird parser implementation in d_do_test.d.

[teodutu]

Looks fine!

[dkorpel]

This introduced a regression: #21945

[schveiguy]

If I'm reading this correctly, we removed memcmp as the mechanism to compare pod arrays? If so, we need to put this back. memcmp is an extremely optimized function.

I'm seeing slowdowns of at least 2x for string comparisons, and in some tests 3x slowdown.

[teodutu]

This was because DMD lowered POD array comparisons to memcmp() by itself. @Albert24GG, can you please check that the arrays are memcmp()'d in the example in #21976?

[Albert24GG]

Yes, I just checked. The logic is correct, but in the current implementation the equality is lowered to OPmemcmp [1], which dmd implements manually using REP* instructions [2]. Before this PR, that array comparison was being lowered to __equals which directly called memcmp@plt (the optimized libc function).

[1]

dmd/compiler/src/dmd/glue/e2ir.d

Line 2146 in 60d62ca

e = el_bin(OPmemcmp, TYint, e, esize);

[2]

dmd/compiler/src/dmd/backend/x86/cod2.d

Line 3755 in 60d62ca

REPE CMPSB ;compare string

[teodutu]

@schveiguy do you have the possiblitiy to test your code with gdc or ldc (provided they integrated this PR)? I'm pretty sure they lower POD comparisons to memcmp().

[schveiguy]

I happen to have built an ldc with all the recent updates, which I think includes this PR.

Testing issue #21976 with this results in 59-60ms runtime, so ldc is doing the right thing, and it does seem like the memcmp should be selected with the current code.

I would suggest if we never expect __equals to be called with something that should be using memcmp, you insert a static if branch that covers that case, which just static assert(0, "should have been handled by compiler!"); or something like that. This way we know the compiler isn't making mistakes. But probably not the cause of this regression.

[ibuclaw]

I'd expect ldc to be aware of what memcmp is (or its LLVM IR) - gcc middle end certainly does - and emit an inline (optimised) comparison for small data or a libc call to memcmp when it sees str1 == str2

[Herringway]

I'd expect ldc to be aware of what memcmp is (or its LLVM IR) - gcc middle end certainly does - and emit an inline (optimised) comparison for small data or a libc call to memcmp when it sees str1 == str2

I believe that is the case. I encountered that while trying to implement a memcmp at one point... that overflowed the stack real quick.