Skip to content

Cherry-pick upstream changes for WASI#5186

Open
QuantumSegfault wants to merge 11 commits into
ldc-developers:masterfrom
QuantumSegfault:wasi-cherrypick
Open

Cherry-pick upstream changes for WASI#5186
QuantumSegfault wants to merge 11 commits into
ldc-developers:masterfrom
QuantumSegfault:wasi-cherrypick

Conversation

@QuantumSegfault

Copy link
Copy Markdown
Contributor

Cherry-picks all of my WASI related changes thus far from dmd-rewrite-master.

422ab4f isn't a pure cherry-pick, as I had to backport the threading changes (upstream is currently refactoring osthread.d).

@QuantumSegfault

Copy link
Copy Markdown
Contributor Author

@thewilsonator
@kinke

Comment thread runtime/druntime/src/core/sys/wasi/posix/time.d
// On Wasm, it increases codesize noticably.
// Meanwhile, ldc (ldmd2) does not support `-profile-gc`, so there's no point
// keeping it around on Wasm targets
version (WASI) {}

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, code size of what? If unused, the object file shouldn't be linked from static druntime.

(We might be able to add -profile=gc support nowadays, as I think I saw that this is now handled in the frontend, not the glue layer anymore.)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The final binary once statically linked.

The static ~this will always get run. And it drags in some things that programs might never use:

  • qsort
  • fopen
  • fclose
  • fprintf

Whatever the exact cause, it does have an effect:

import core.stdc.stdio;

// Without profilegc: 684K	
// With profilegc: 844K
void main() {
    printf("Hello, WASI!\n");
}

To be fair though, this is at -O3 instead of -Oz. If everything were compiled at -Oz (particularly wasi-libc, the difference wouldn't be as large). Or more complex files that do interact with files would mask it as well.

@kinke kinke Jul 17, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm. There's

ulong accumulatePure(string file, int line, string funcname, string name, ulong size) nothrow pure
{
static ulong impureBypass(string file, int line, string funcname, string name, ulong size) @nogc nothrow
{
import core.internal.traits : externDFunc;
alias accumulate = externDFunc!("rt.profilegc.accumulate", void function(string file, uint line, string funcname, string type, ulong sz) @nogc nothrow);
accumulate(file, line, funcname, name, size);
return size;
}
auto func = cast(ulong function(string file, int line, string funcname, string name, ulong size) @nogc nothrow pure)&impureBypass;
return func(file, line, funcname, name, size);
}

which drags in this rt.profilegc module AFAICT. Maybe this function should be a dummy-template instead, so that the TraceHook instantiations instantiate it as needed instead of baking this wrapper into prebuilt druntime.

Edit: Or moving this function wrapper to the rt.profilegc module (=> same object file in static druntime).

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's not the issue. It's the module destructors. Leaving accumulate has no visible impact. It's the two destructors. Mainly the second one (the first contributes to ~8K; the rest is the second and deps).

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm pretty sure that is the issue, becausing nothing else imports that module (client code cannot even), so the module dtor shouldn't matter as long as nothing references the object file.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep, exactly. Then the prebuilt core.internal.array.utils module doesn't depend on rt.profilegc anymore, only any template instantiations in user code compiled with -profile=gc.

This should bring down the executable sizes in general, not just wasm. [This is most likely a 'regression' from when these templates were added, moving the -profile=gc stuff from the glue layer to the frontend, by lowering to these templates.]

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmmm...

dlang/dmd#22859

Stuff has already been refactored upstream. Looks like it shouldn't be a problem any more?

Nothing else in profile_gc.d is used elsewhere except the hooks, which are only imported when D_ProfileGC is active. So the linker should be able to strip profile_gc, and thus drop rt.profilegc as well?

Or will it still be a problem?

@kinke kinke Jul 17, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mhm, yes, I think your analysis is correct, and so this should be fine in master/v2.114. We should probably still resolve it for v2.113/DMD stable too, mainly because it's a simple fix. I can create an upstream PR too over the weekend, just let me know if you wanna do it yourself.

So for this PR here, I'd say just remove this WASI-specific exclusion of the module. Oh, that's upstream I guess - in which case that would be part of the DMD PR moving the function, and so we'd get it removed with the next dmd-rewrite-stable merge anyway. So no blocker for this PR here. Edit: Nope, your stuff is in master, not stable... :)

Good that you observed this heavy hidden object-file dependency in druntime, and we went to the bottom of it. :)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, I can do that real quick. I already have it prototyped against LDC. Just a quick shuffle. I'll open a PR shortly.

And then I guess a separate PR to remove the version (WASI) once that gets merged.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Comment thread runtime/druntime/src/core/internal/abort.d
@QuantumSegfault

Copy link
Copy Markdown
Contributor Author

e5150d1 is a big new one (since you initially reviewed).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants