Refactored `arraysetlengthT` and `arraysetlengthiT` into a Single `arraysetlengthT` Template by shivangshukla7020 · Pull Request #21151 · dlang/dmd

shivangshukla7020 · 2025-04-05T04:05:55Z

Changes Introduced

This PR merges arraysetlengthT and arraysetlengthiT into a single template, improving compile-time specialization and reducing redundancy. The key updates include:

Unified arraysetlengthT and arraysetlengthiT into a single template, distinguishing them based on initialization behavior.
Optimized memory handling for zero-initialized (arraysetlengthT) and non-zero-initialized (arraysetlengthiT) cases.
Used emplace for non-trivial types instead of memcpy to ensure correct postblit behavior.
Integrated gc_expandArrayUsed for in-place array expansion where applicable.
Added D_ProfileGC support via _d_HookTraceImpl.

This refactoring improves performance by removing TypeInfo lookups and enabling better compile-time specialization.

dlang-bot · 2025-04-05T04:05:58Z

Thanks for your pull request and interest in making D better, @shivangshukla7020! 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#21151"

compiler/src/dmd/backend/rtlsym.d

teodutu

Overall this is looking good. I made some small suggestions.

compiler/src/dmd/expressionsem.d

druntime/src/core/internal/array/capacity.d

druntime/src/core/internal/array/concatenation.d

druntime/src/core/internal/array/capacity.d

compiler/src/dmd/expression.d

compiler/src/dmd/e2ir.d

compiler/src/dmd/expressionsem.d

compiler/src/dmd/expression.d

druntime/src/core/internal/array/capacity.d

compiler/src/dmd/expression.d

compiler/src/dmd/expressionsem.d

compiler/test/runnable/extra-files/hello-profile.d.trace.def

druntime/src/core/internal/array/capacity.d

compiler/src/dmd/expressionsem.d

teodutu

The compiler code looks OK, but the runtime checks for __traits(compiles might be superfluous.

druntime/src/core/internal/array/capacity.d

compiler/src/dmd/expressionsem.d

druntime/src/core/internal/array/capacity.d

teodutu

It's looking good now. Since you have 73 commits here, it would be easier for eventual regressions to squash them into a single one before merging.

thewilsonator · 2025-04-30T22:45:39Z

Plese rebase and resolve the merge conflicts

shivangshukla7020 · 2025-05-01T08:58:39Z

Plese rebase and resolve the merge conflicts

Done

thewilsonator

Thanks

bangbangsheshotmedown · 2025-05-01T15:20:07Z

runtime hook change should probably require a change log entry

teodutu · 2025-05-01T20:12:25Z

runtime hook change should probably require a change log entry

Oh yeah you're right, I forgot to mention this. @shivangshukla7020 please raise another PR where you add a file to this folder (check those already existing for formatting): https://github.com/dlang/dmd/tree/master/changelog

shivangshukla7020 · 2025-05-03T18:27:34Z

Okay, I'll update and add the changelog accordingly

CyberShadow · 2025-09-08T21:50:38Z

This pull request introduced a regression: #21832

ibuclaw · 2025-11-20T09:49:26Z

This PR caused a build regression

core/internal/array/capacity.d:251:23: error: none of the overloads of template ‘core.checkedint.mulu’ are callable using argument types ‘!()(ulong, ulong, ubyte)’
  251 |         newsize = mulu(sizeelem, newlength, overflow);
      |                       ^
core/checkedint.d:761:6: note: Candidates are: ‘mulu()(uint x, uint y, ref bool overflow)’
  761 | uint mulu()(uint x, uint y, ref bool overflow)
      |      ^
core/checkedint.d:792:7: note:                 ‘mulu()(ulong x, uint y, ref bool overflow)’
  792 | ulong mulu()(ulong x, uint y, ref bool overflow)
      |       ^
core/checkedint.d:803:7: note:                 ‘mulu()(ulong x, ulong y, ref bool overflow)’
  803 | ulong mulu()(ulong x, ulong y, ref bool overflow)
      |       ^

ibuclaw · 2025-11-20T09:52:10Z

druntime/src/core/internal/array/capacity.d

+    ubyte overflow = 0;
+
+    size_t newsize = void;
+
+    version (D_InlineAsm_X86)
+    {
+        asm pure nothrow @nogc
+        {
+            mov EAX, sizeelem;
+            mul newlength;        // EDX:EAX = EAX * newlength
+            mov newsize, EAX;
+            setc overflow;
+        }
+    }
+    else version (D_InlineAsm_X86_64)
+    {
+        asm pure nothrow @nogc
        {
-            auto ti = typeid(Tarr);
+            mov RAX, sizeelem;
+            mul newlength;        // RDX:RAX = RAX * newlength
+            mov newsize, RAX;
+            setc overflow;
+        }
+    }
+    else
+    {
+        newsize = mulu(sizeelem, newlength, overflow);
+    }


... here, overflow has wrong type.

shivangshukla7020 requested a review from ibuclaw as a code owner April 5, 2025 04:05

ibuclaw reviewed Apr 5, 2025

View reviewed changes

compiler/src/dmd/backend/rtlsym.d Show resolved Hide resolved

teodutu suggested changes Apr 5, 2025

View reviewed changes

shivangshukla7020 force-pushed the template branch from e8265b8 to d1bc7e6 Compare April 6, 2025 17:39

teodutu suggested changes Apr 7, 2025

View reviewed changes

teodutu suggested changes Apr 12, 2025

View reviewed changes

shivangshukla7020 force-pushed the template branch 2 times, most recently from 09df08b to 0e0cd9c Compare April 21, 2025 11:00

teodutu suggested changes Apr 22, 2025

View reviewed changes

shivangshukla7020 force-pushed the template branch 3 times, most recently from 30a3d1b to 2ab5424 Compare April 23, 2025 19:05

teodutu reviewed Apr 23, 2025

View reviewed changes

compiler/src/dmd/expressionsem.d Outdated Show resolved Hide resolved

shivangshukla7020 force-pushed the template branch 3 times, most recently from f329524 to 136317e Compare April 24, 2025 06:43

teodutu suggested changes Apr 25, 2025

View reviewed changes

shivangshukla7020 force-pushed the template branch from 30d42ab to 94d9c90 Compare April 26, 2025 08:17

teodutu approved these changes Apr 30, 2025

View reviewed changes

shivangshukla7020 force-pushed the template branch from 8102985 to 00cd468 Compare May 1, 2025 05:34

Templatized arraysetlength{,i}T

6b0463c

shivangshukla7020 force-pushed the template branch from 00cd468 to 6b0463c Compare May 1, 2025 05:41

thewilsonator approved these changes May 1, 2025

View reviewed changes

thewilsonator merged commit 94bad47 into dlang:master May 1, 2025
42 checks passed

teodutu mentioned this pull request May 1, 2025

Templatize _d_arraysetcapacity hook #21143

Merged

This was referenced May 4, 2025

Added changelog file for Templatized arraysetlengthT #21337

Closed

Added changelog file for Templatized _d_arraysetlengthT #21338

Merged

teodutu mentioned this pull request May 9, 2025

Templatize _d_arrayappendcTX #21205

Merged

CyberShadow mentioned this pull request Sep 8, 2025

[REG master] Can't expand array with implicitly non-copyable types #21832

Closed

ibuclaw reviewed Nov 20, 2025

View reviewed changes

Uh oh!

Conversation

shivangshukla7020 commented Apr 5, 2025

Changes Introduced

Uh oh!

dlang-bot commented Apr 5, 2025

Bugzilla references

Testing this PR locally

Uh oh!

Uh oh!

teodutu left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

teodutu left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

teodutu left a comment

Choose a reason for hiding this comment

Uh oh!

thewilsonator commented Apr 30, 2025

Uh oh!

shivangshukla7020 commented May 1, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

thewilsonator left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

bangbangsheshotmedown commented May 1, 2025

Uh oh!

teodutu commented May 1, 2025

Uh oh!

shivangshukla7020 commented May 3, 2025

Uh oh!

CyberShadow commented Sep 8, 2025

Uh oh!

ibuclaw commented Nov 20, 2025

Uh oh!

ibuclaw Nov 20, 2025

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

shivangshukla7020 commented May 1, 2025 •

edited

Loading