Skip to content

Conversation

@WalterBright
Copy link
Member

Features needed:

  1. support -mv compiler switch
  2. support -betterC switch
  3. send assert failures to C's assert failure function

dmd 2.074 does these.

This is the canary for those features. They're needed to convert the dmd backend to D.

@dlang-bot
Copy link
Contributor

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 fetch digger
dub run digger -- build "master + dmd#8112"

@wilzbach
Copy link
Contributor

wilzbach commented Apr 2, 2018

dmd 2.074 does these.

These CIs don't have 2.074:

  • auto-tester (CC @braddr)
  • SemaphoreCI (due to GDC, CC @ibuclaw)
  • DAutoTest (due to digger bootstrapping from 2.067 - the last C++ release, CC @CyberShadow)

Consensus from https://forum.dlang.org/post/zbxaviowooiaenaawmgs@forum.dlang.org summarized by @MartinNowak

We've been through bootstrapping discussions a couple of times, so let me repeat what was decided when we made the frontend switch from C++ to D.

  • Other platforms are bootstrapped by cross-compilation.
  • DMD must be compilable with the latest stable/major release of dmd, ldc, and gdc.

To enforce this policy the Travis-CI test was set up.
Hopefully this original purpose of the Travis-CI hasn't been forgotten in the meantime.

  • No other guarantees were negotiated.

(SemaphoreCi is now Travis-CI)

support -betterC switch

I thought that we agreed that just dropping -betterC for GDC was an acceptable solution?
See #6907 (comment)

@WalterBright
Copy link
Member Author

That comment is nearly a year old. How long do we need to wait? The needed features I mentioned are easy to implement.

@rainers
Copy link
Member

rainers commented Apr 2, 2018

Features needed:

I guess it has been discussed before, but could you explain why these are needed (or give a link)? Your changes to divcoeff.d work fine for me without these.

@ibuclaw
Copy link
Member

ibuclaw commented Apr 2, 2018

Why is -mv needed?

@WalterBright
Copy link
Member Author

WalterBright commented Apr 2, 2018

  1. -mv is needed because the backend lies in a different directory structure in dmc++. Trying to force every project that uses the backend into the same directory structure is not very workable.

  2. -betterC is needed to generate cheap assert code. The backend relies much on assert being cheap and on in release mode. The D assert has gotten so larded up it's been disabled in the release dmd (!) because it substantially slows it down. (There was discussion about this in one of Kenji's PRs a while back.)

Both of these features are trivial to implement. I've been asking for them for years now in gdc/ldc.

Your changes to divcoeff.d work fine for me without these.

In the last PR, the asserts in divcoeff.d had to be commented out to get it to work.

@ibuclaw
Copy link
Member

ibuclaw commented Apr 3, 2018

-mv is needed because the backend lies in a different directory structure in dmc++.

But how do does that relate to dmd? There's no sharing of compiled objects between projects right? From what I understand from your occasional PRs is that the backend is forked between them and occasionally synchronised.

-betterC is needed to generate cheap assert code.

The expensive part of D asserts are class/invariant checks, of which classes themselves are notably missing in the majority of the backend.

I'm still not seeing past that the backend is separate between projects and so doesn't have to be identically built.

In the last PR, the asserts in divcoeff.d had to be commented out to get it to work.

How come though? You are linking against druntime, so there won't be any missing symbols for dmd build.

@wilzbach
Copy link
Contributor

wilzbach commented Apr 3, 2018

How come though? You are linking against druntime, so there won't be any missing symbols for dmd build.

That was because DAutoTest/Digger is still using 2.067 and only 2.068 and higher doesn't generate a new assert template for every assert (https://dlang.org/changelog/2.068.0.html#fix14828-warning)

(see also: CyberShadow/ae#35)

@WalterBright
Copy link
Member Author

WalterBright commented Apr 3, 2018

But how do does that relate to dmd?

The backend is shared with dmc++. Diverging them will create a lot of makework for me. Furthermore, the dmc++ test suite has extensive tests for the backend that are not in the dmd test suite.

The -mv switch is for anyone who needs to share code between projects despite a different directory structure. Telling them all to diverge their projects is a bad idea.

The thing is, -mv is TRIVIAL to implement. Just copy the code over - I already did the hard part. Time spent arguing about it has already greatly exceeded the implementation effort. It has been a part of DMD for years.

@WalterBright
Copy link
Member Author

LDC already supports -mv since last summer. We've already had this discussion here: #8112

@CyberShadow
Copy link
Member

Failing with:

posix.mak:295: *** missing separator.  Stop.

@CyberShadow
Copy link
Member

CyberShadow commented Apr 4, 2018

dmd 2.074 does these.

Since this change will introduce another step to the from-source bootstrap process, why not use the newest version possible, to minimize the distance between number of such steps?

@wilzbach
Copy link
Contributor

wilzbach commented Apr 4, 2018

Failing with:

That's because comments in Make use # not //

BTW we could do the following "trick" to use a newer host compiler on the auto-tester:

diff --git a/posix.mak b/posix.mak
index 9a4b24d17..aad671696 100644
--- a/posix.mak
+++ b/posix.mak
@@ -8,7 +8,7 @@ all:
 	$(QUIET)$(MAKE) -C src -f posix.mak all
 
 auto-tester-build: toolchain-info
-	$(QUIET)$(MAKE) -C src -f posix.mak auto-tester-build ENABLE_RELEASE=1
+	$(QUIET)$(MAKE) -C src -f posix.mak auto-tester-build ENABLE_RELEASE=1 AUTO_BOOTSTRAP=1
 
 auto-tester-test: test

(I just tested this with your patch and it works)
The Windows Makefiles don't support auto-bootstrapping, but we could add it to them too.

This leaves the question to:

Do we want DMD to stop being compilable with GDC?

(it's two clicks for me to disable the GDC build on SemaphoreCI)

One could argue

  • there's LDC for bootstrapping and cross-compiling DMD.
  • I think the release archives are currently not even built with LDC, which is a shame as it would make DMD even faster (~30-40%, maybe more with LTO), but even if we want to use a faster backend for the release build, there's still LDC
  • GDC will hopefully soon have a new release upon which it could be added to the CI pipeline again

@WalterBright
Copy link
Member Author

That's because comments in Make use # not //

How embarrassing :-)

@WalterBright
Copy link
Member Author

WalterBright commented Apr 4, 2018

  • 2.068 is 2.5 years old now.
  • I don't mind if ldc/gdc are used to build the release dmd if it's 30% faster.
  • There have been codegen improvements in dmd since 2.068 - loop unrolling and SROA. That won't close the gap, but it's progress.
  • It's pretty frustrating looking at the backend code and thinking "this would be so much more tractable in D".
  • The dmc++ front end is already in D :-) and it's a nice win.
  • I'm good with going with as new a DMD release as possible as the boot compiler.

@ibuclaw
Copy link
Member

ibuclaw commented Apr 4, 2018

There have been codegen improvements in dmd since 2.068 - loop unrolling and SROA. That won't close the gap, but it's progress.

However gdc doesn't have the same pitfalls as dmd (there have never been any assert "helper" templates) so you can use it to build without -betterC enabled.

GDC will hopefully soon have a new release upon which it could be added to the CI pipeline again

You've already switched to Semaphore, which at least has 16.04 running (two years newer than Travis). Have you not considered seeing what the latest version is available there to see what features are enabled? It should be gdc-6 if I have the release schedule right. And 18.04 is right around the corner and will have gdc-8 bundled in, which is straight from master branch at the time of feature freeze.

@wilzbach
Copy link
Contributor

wilzbach commented Apr 4, 2018

Have you not considered seeing what the latest version is available there to see what features are enabled?

> docker run --rm -ti ubuntu:16.04
> apt-get update && apt-get install gdc
> gdc --version
gdc (Ubuntu 5.4.0-6ubuntu1~16.04.9) 5.4.0 20160609
> echo "pragma(msg, __VERSION);" > foo.d && gdc -c foo.d
2067L

And 18.04 is right around the corner and will have gdc-8 bundled in, which is straight from master branch at the time of feature freeze.

> docker run --rm -ti ubuntu:18.04
> apt-get update && apt-get install gdc
> gdc --version
gdc (Ubuntu 8-20180402-1ubuntu1) 8.0.1 20180402 (experimental) [trunk revision 259004]
> echo "pragma(msg, __VERSION__);" > foo.d && gdc -c foo.d
2068L

Anyhow the install script fetches the binaries from https://gdcproject.org/downloads (or http://gdcproject.org/downloads/LATEST in particular). That one still points to 4.8.5.

(see also: https://github.com/dlang/installer/blob/master/script/install.sh#L540 and dlang/installer#251)

@ibuclaw
Copy link
Member

ibuclaw commented Apr 4, 2018

I've never actually bothered updating the frontend version because it's not strictly true to do so. If I were to give it a version, GDC master would be 2068+really2076-withoutstaticforeach-andothermissingbackportsthatarentneededforpassingtestsuite-c++. So to call it 2076 would be a stretch of the imagination, but its certainly far closer to 2076 than 2068, despite the frontend still implemented in C++. :-)

@WalterBright
Copy link
Member Author

DAutoTest (due to digger bootstrapping from 2.067 - the last C++ release, CC @CyberShadow)

I understand the need for that bootstrap. But I think it's time to make it two stage instead of one stage. First, boot to 2.079. Then use 2.079 to boot to the latest. @CyberShadow can you make this happen?

@CyberShadow
Copy link
Member

@CyberShadow can you make this happen?

Of course, no problem. This is why I suggested that we use the latest version as of now (2.079), to minimize the number of steps.

I updated Digger and DAutoTest to use 2.079.0 if -mv= exists in the makefile (we'll probably want to update that condition should we want to get rid of -mv=), so the DAutoTest CI should in theory become green on the next rebuild. However, I don't know what needs to be done with the other failures.

@wilzbach
Copy link
Contributor

wilzbach commented Apr 9, 2018

I updated Digger and DAutoTest to use 2.079.0
However, I don't know what needs to be done with the other failures.

Ping @braddr - could you please update the build machines to 2.079.0 too then?

So to call it 2076 would be a stretch of the imagination, but its certainly far closer to 2076 than 2068, despite the frontend still implemented in C++. :-)

Okay so from what I gather is that there's no easy way to grab a recent GDC binary that runs on SemaphoreCI (correct @ibuclaw ?), so I guess I will try compiling GDC from master, s.t. we have a binary that SemaphoreCI can run.

@dnadlinger
Copy link
Contributor

While I don't have a stake in this discussion myself (LDC supports the "worse D" switch because users expect it, even though I still think it's a fundamentally flawed design), there is one point I'd like to raise:

Is it really true that converting the backend to D "requires" these features? As in, is this a statement that could be defended in good faith in a discussion among competent engineers?

Features needed:

  1. support -mv compiler switch
  2. support -betterC switch
  3. send assert failures to C's assert failure function

-mv can trivially be worked around. If organising the files in the default way (i.e. as people reading the code expect them to be laid out) is really too big of a burden, then just move them into the root module and use -Isrc/dmd/backend for DMD (and -Idm/src/dmc for DMC).

I don't see a reason for -betterC itself to be required. The runtime could always be excluded on compilers that support this for size reasons, but that's just an optimisation.

And assert() uses can trivially be searched/replaced with a custom function that aborts instead of unwinding.


TL;DR: Some of these features might be nice to have, but there is no reason they should block any work on the compiler backend.

@jacob-carlborg
Copy link
Contributor

Some of these features might be nice to have, but there is no reason they should block any work on the compiler backend.

I agree. These features are not a technical requirement.

@wilzbach
Copy link
Contributor

wilzbach commented May 2, 2018

If I read the auto-tester logs correctly, @braddr has silently been preparing a first may present for us, i.e. almost some hosts have been updated to 2.079.0:

HOST_DMD(/home/ec2-user/sandbox/at-client/release-build/dmd-2.079.0/linux/bin64/dmd): DMD64 D Compiler v2.079.0 Copyright (C) 1999-2018 by The D Language Foundation, All Rights Reserved written by Walter Bright

which means only the OSX hosts need to be updated :)

@wilzbach
Copy link
Contributor

wilzbach commented May 3, 2018

CC @schveiguy for upgrading the auto-tester OSX hosts.

@wilzbach
Copy link
Contributor

So any updates on upgrading the OSX hosts at auto-tester to 2.079?
Alternatively, for hosts building with 2.068 the -betterC flag could be removed in the Makefile as it has been proposed in the last PR.

@schveiguy
Copy link
Member

@wilzbach #8243

@WalterBright
Copy link
Member Author

So any updates on upgrading the OSX hosts at auto-tester to 2.079?

Currently blocked on OSX by #8243

@WalterBright
Copy link
Member Author

@braddr can we try again to upgrade OSX boxes to 2.079?

Copy link
Member

@rainers rainers left a comment

Choose a reason for hiding this comment

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

The title and description are a bit misleading now.
This just restores the asserts that were already there before the conversion to D. Would be good to reflect that in the commit message.

Copy link
Contributor

@wilzbach wilzbach left a comment

Choose a reason for hiding this comment

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

Great to see that finally all boxes have been upgraded :)
FWIW we still use "latest" GDC (currently 4.8.5 - based on 2.068.2) to compile DMD on Semaphore to check for being able to compile dmd with gdc.
So it would be great to update that one too, s.t. we can take advantage of all the great features (and frontend fixes) that have been committed since.

While it's on my TODO list to compile GDC for Ubuntu 14.04 (Trusty), I sadly won't get to this this month. I think the latest Ubuntu does ship with the latest/greatest GDC, but IIRC only GCC 4.9 is still supported whereas Trusty only supports GCC 4.8.
So if anyone wants to see this happen, all that's needed is a tar ball similar to the ones from gdcproject.org

@schveiguy
Copy link
Member

Are we waiting on something here? I think this should be merged.

@wilzbach wilzbach merged commit 5a8162c into dlang:master Jun 22, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

9 participants