replace frontend inliner with backend inliner

[WalterBright]

The front end inliner has some unfixable problems, and it is way overly complex. I made a mistake with this design, and am tired of it. This replaces it with one in the backend that inlines based on the optimized intermediate code. This is vastly simpler, and will inline much more.

It is currently in a draft state, and disables the frontend one, although the code for it remains for now.

Current shortcomings:

1. it can only inline functions it has already compiled
2. it can only inline functions that consist of one basic block

Comments are welcome, just remember it's only a draft at the moment! I expect the usual problems with it.

[dlang-bot]

Thanks for your pull request, @WalterBright!

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#14194"

[WalterBright]

Unfortunately, the test suite tries to compile the runtime library first. This is backwards. It should compile the compiler test suite first. Only when that passes should it attempt to compile the runtime library, as the test suite is designed to make it easy to track down problems.

[ibuclaw]

Unfortunately, the test suite tries to compile the runtime library first. This is backwards. It should compile the compiler test suite first. Only when that passes should it attempt to compile the runtime library, as the test suite is designed to make it easy to track down problems.

The tests in the testsuite should not be importing the library then.

Can't you run the testsuite locally?

[maxhaton]

If we could actually remove the frontend one entirely that would be a very nice red diff.

[WalterBright]

checkwhitespace fails, but gives no clue about which file is errant.

[maxhaton]

This would require splitting the test suite up into basically simple tests and D tests. The language cannot be implemented without druntime and hence cannot be properly tested without it being compiled unless the feature being tested is a pay as you go one like the work with the template hooks. This split can be done though. It'd need a simplified object.d which I think we do have somewhere.

…

On Thu, 9 Jun 2022, 06:52 Walter Bright, ***@***.***> wrote: It would be REALLY REALLY nice if the testers would run the tests in dmd/test/runnable and dmd/test/compilable FIRST, because those are designed to debug the compiler. Getting a seg fault in other places is much HARDER to debug. — Reply to this email directly, view it on GitHub <#14194 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ABLI75EELUOLP7DQQWZKGV3VOGBAZANCNFSM5YFPN76Q> . You are receiving this because you commented.Message ID: ***@***.***>

[WalterBright]

The problems have been reduced to:

1. dshell/dll.d fails on the mac. Don't know why
2. the rest of the failures are all the compiler not warning when it can't inline a function. Since the backend is doing the inlining, this may not be fixable
3. buildkite has some mysterious failures. Don't know if they are related or not. It seems that there are lots and lots of deprecation messages in those builds

[ibuclaw]

The problems have been reduced to:

1. dshell/dll.d fails on the mac. Don't know why

I'll see if I can spin up one of my macboxes to run the testsuite there later.

[rikkimax]

This may not be possible, but it would be nice if this would work with inline assembly.

It would mean that we wouldn't have to replace the assembly in core.internal.atomic with intrinsics.

[maxhaton]

This may not be possible, but it would be nice if this would work with inline assembly.

It would mean that we wouldn't have to replace the assembly in core.internal.atomic with intrinsics.

Why does it matter as long as the memory ordering is correct? The backend also isn't a proper load-store ISA so the intrinsics might look a bit weird internally, although with x86 you basically only need to emit the right prefix in a few cases so that plus volatile might be enough.

[rikkimax]

Why does it matter as long as the memory ordering is correct?

Segfaults. You get segfaults.

Lock-free concurrent data structures are one of the few areas left of CS where instruction counting still persists. A function call will cause segfaults due to memory unmapping by the time the next step in the algorithm takes place.

[maxhaton]

Why does it matter as long as the memory ordering is correct?

Segfaults. You get segfaults.

Lock-free concurrent data structures are one of the few areas left of CS where instruction counting still persists. A function call will cause segfaults due to memory unmapping by the time the next step in the algorithm takes place.

I'm going to need to see an example that segfaults with dmd today because that sounds backwards.

[rikkimax]

Why does it matter as long as the memory ordering is correct?

Segfaults. You get segfaults.
Lock-free concurrent data structures are one of the few areas left of CS where instruction counting still persists. A function call will cause segfaults due to memory unmapping by the time the next step in the algorithm takes place.

I'm going to need to see an example that segfaults with dmd today because that sounds backwards.

Lock-free concurrent data structures have no way to synchronize free'ing of memory. They rely solely on timeing to get things right. Andrei has written an introductory paper on this topic. But he only barely touches upon the memory deallocation side of things (which are still to this day not solved).

We are getting pretty off-topic now.

[ibuclaw]

Alls looking nice and green. There are still some fix-ups left to do in the implementation to make it nice and neat.

Are we going to switch to this proper now? Or hide it behind a preview flag? Moving forward with this should give us a ~3000 line removal.

[ibuclaw]

Be sure to change this line too

dmd/src/dmd/expressionsem.d

Lines 1757 to 1758 in 13f9d97

	arg = p.defaultArg;
	arg = inlineCopy(arg, sc);

That should instead be:

arg = p.defaultArg.copy();

After that, then there'll really be no calls into the frontend inliner.

[WalterBright]

I turned the original inliner back on so both are operating. This is because the backend one doesn't handle all the cases yet the front end one does.

This PR is ready to go. The one failing test is a heisenbug.