Fix 23857 - backend inliner takes too long on recursive function call

[dkorpel]

Blunt fix: disable backend inliner for now

[dlang-bot]

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

Auto-close	Bugzilla	Severity	Description
✓	23857	normal	backend inliner takes too long on recursive function call

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 "stable + dmd#15437"

[dkorpel]

Huh, test23307 needs backend inlining or it fails the assert.

#14733 says:

It's working for me, so just adding the test case.

Looks like there's still a bug there.

[maxhaton]

Target stable.

Also is there not a inclining heuristic that can be adjusted?

[ljmf00]

Huh, test23307 needs backend inlining or it fails the assert.

That seems weird. We shouldn't have a testcase that only works with the inliner.

[dkorpel]

That seems weird. We shouldn't have a testcase that only works with the inliner.

The issue reported it only happens with dmd -O -inline, so those were set as REQUIRED_ARGS

[ljmf00]

The issue reported it only happens with dmd -O -inline, so those were set as REQUIRED_ARGS

Conceptually, the inliner doesn't change the behavior of the program. Having it or not should behave exactly the same. Its a bug on the codegen. If we are asserting codegen execution, we shouldn't enforce it to pass only with -inline on or off, exclusively. We should permute the arguments, in most of the cases.

[ljmf00]

Why do we even use this backend anyway :(

[maxhaton]

The reason why is that DMD doesn't unittest properly so you have top level integration tests for extremely sparse bits of the tree.

…

On Fri, 21 Jul 2023, 18:14 Luís Ferreira, ***@***.***> wrote: The issue reported it only happens with dmd -O -inline, so those were set as REQUIRED_ARGS Conceptually, the inliner doesn't change the behavior of the program. Having it or not should behave exactly the same. Its a bug on the codegen. If we are asserting codegen execution, we shouldn't enforce it to pass only with -inline on or off, exclusively. We should permute the arguments, in most of the cases. — Reply to this email directly, view it on GitHub <#15437 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ABLI75C5VYICQRSVVRFNGBLXRK2IBANCNFSM6AAAAAA2SUO2GE> . You are receiving this because you commented.Message ID: ***@***.***>

[ljmf00]

permute these flags, please

[WalterBright]

I'd rather the problem was identified and fixed rather than disabling things.

[WalterBright]

#15447 doesn't disable the inliner

[dkorpel]

I'd rather the problem was identified and fixed rather than disabling things.

Of course. I only disable faulty optimizations when they don't get fixed (I emailed you about this bug 3 months ago), because correctness is more important than generation of fast code, especially with dmd. Unlike ldc and gdc, dmd is known to generate code fast, but not fast code.

[dkorpel]

Superseded by #15447

[WalterBright]

I emailed you about this bug 3 months ago

I obviously missed that. Oops.