fix Issue 17098 - Takes hours to -O compile then fails with Internal …

[WalterBright]

…error: backend/cgreg.c 405

The long compilation times are not fixed, there's not much to be done about that as the behavior of the optimizer is inherently quadratic. But the test case compiles now.

The test case isn't included because it takes hours to compile.

[dlang-bot]

Thanks for your pull request, @WalterBright!

Bugzilla references

Auto-close	Bugzilla	Severity	Description
✓	17098	regression	Takes hours to -O compile then fails with Internal error: backend/cgreg.c 405

⚠️⚠️⚠️ Warnings ⚠️⚠️⚠️

• Regression or critical bug fixes should always target the stable branch. Learn more about rebasing to stable or the D release process.

To target stable perform these two steps:

1. Rebase your branch to upstream/stable:

git rebase --onto upstream/stable upstream/master

2. Change the base branch of your PR to stable

Testing this PR locally

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

dub fetch digger
dub run digger -- build "master + dmd#10789"

[WalterBright]

This is a very minor regression and very ancient, so no need to go into stable.

[Geod24]

This is a very minor regression and very ancient, so no need to go into stable.

TL;DR Safe fixes should always go into stable, all the time.

Maybe we should amend that DIP if we are not going to follow the process outlined in it.

[WalterBright]

It's not a safety issue.

[Geod24]

"Safe fixes" => "Fixes that are safe to merge", as opposed to "fixes that require significant work / refactoring / change to existing code".

But that is a good example on what part of the wording could use an update.

[WalterBright]

This particular issue is very old and blocks nobody. The example for it is a toy program. Putting it into stable suggests to users that they should waste their time downloading a point release for this. Besides, all fixes carry an inherent risk of causing another problem, and for this PR, that would be further disruptive. The only reason to even fix this is a QoI dot-i-and-cross-t sort of thing.

The rules do not take precedence over common sense.

[RazvanN7]

I'm ok with this not going into stable, but I am concerned about the missing test case. I do not doubt the implementation, but I fear that people might break this in the feature and we have no way of preventing that.

[WalterBright]

I would include it, but having a test case that takes 2 hours to run is not very practical for the autotester.

[RazvanN7]

Isn't it possible to add a simplified test case that covers the benefit = int.max; and would compile faster?

[WalterBright]

I don't know how. You need a gigantic function to do it.