Second round of makefile/build.d consolidation #10212

marler8997 · 2019-07-24T02:52:30Z

This round I'm moving the generation of the backend library to build.d.

dlang-bot · 2019-07-24T02:52:31Z

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

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

thewilsonator

Otherwise looks great!

src/dmd/backend/cgobj.d

PetarKirov · 2019-07-24T06:22:30Z

GDC9 on SemaphoreCI fails with:

...
gdmd-9 -of../generated/linux/release/64/dmd -m64 -vtls -J../generated/linux/release/64 -J../res  -version=MARS -fPIC -J../generated/linux/release/64 -w -de -O -release -inline dmd/access.d dmd/aggregate.d dmd/aliasthis.d dmd/apply.d dmd/argtypes.d dmd/argtypes_sysv_x64.d dmd/arrayop.d dmd/arraytypes.d dmd/astcodegen.d dmd/ast_node.d dmd/attrib.d dmd/builtin.d dmd/canthrow.d dmd/cli.d dmd/clone.d dmd/compiler.d dmd/complex.d dmd/cond.d dmd/constfold.d dmd/cppmangle.d dmd/cppmanglewin.d dmd/ctfeexpr.d dmd/ctorflow.d dmd/dcast.d dmd/dclass.d dmd/declaration.d dmd/delegatize.d dmd/denum.d dmd/dimport.d dmd/dinifile.d dmd/dinterpret.d dmd/dmacro.d dmd/dmangle.d dmd/dmodule.d dmd/doc.d dmd/dscope.d dmd/dstruct.d dmd/dsymbol.d dmd/dsymbolsem.d dmd/dtemplate.d dmd/dversion.d dmd/escape.d dmd/expression.d dmd/expressionsem.d dmd/func.d dmd/hdrgen.d dmd/id.d dmd/impcnvtab.d dmd/imphint.d dmd/init.d dmd/initsem.d dmd/inline.d dmd/inlinecost.d dmd/intrange.d dmd/json.d dmd/lambdacomp.d dmd/lib.d dmd/libelf.d dmd/libmach.d dmd/link.d dmd/mars.d dmd/mtype.d dmd/nogc.d dmd/nspace.d dmd/objc.d dmd/opover.d dmd/optimize.d dmd/parse.d dmd/permissivevisitor.d dmd/sapply.d dmd/templateparamsem.d dmd/sideeffect.d dmd/statement.d dmd/staticassert.d dmd/target.d dmd/typesem.d dmd/traits.d dmd/transitivevisitor.d dmd/parsetimevisitor.d dmd/visitor.d dmd/typinf.d dmd/utils.d dmd/scanelf.d dmd/scanmach.d dmd/statement_rewrite_walker.d dmd/statementsem.d dmd/staticcond.d dmd/safe.d dmd/blockexit.d dmd/printast.d dmd/semantic2.d dmd/semantic3.d dmd/irstate.d dmd/toctype.d dmd/glue.d dmd/gluelayer.d dmd/todt.d dmd/tocsym.d dmd/toir.d dmd/dmsc.d dmd/tocvdebug.d dmd/s2ir.d dmd/toobj.d dmd/e2ir.d dmd/eh.d dmd/iasm.d dmd/iasmdmd.d dmd/iasmgcc.d dmd/objc_glue.d dmd/backend/cc.d dmd/backend/cdef.d dmd/backend/cgcv.d dmd/backend/code.d dmd/backend/cv4.d dmd/backend/dt.d dmd/backend/el.d dmd/backend/global.d dmd/backend/obj.d dmd/backend/oper.d dmd/backend/outbuf.d dmd/backend/rtlsym.d dmd/backend/code_x86.d dmd/backend/iasm.d dmd/backend/codebuilder.d dmd/backend/ty.d dmd/backend/type.d dmd/backend/exh.d dmd/backend/mach.d dmd/backend/mscoff.d dmd/backend/dwarf.d dmd/backend/dwarf2.d dmd/backend/xmm.d dmd/backend/dlist.d dmd/backend/melf.d dmd/backend/varstats.di dmd/root/aav.d dmd/root/array.d dmd/root/ctfloat.d dmd/root/file.d dmd/root/filename.d dmd/root/man.d dmd/root/outbuffer.d dmd/root/port.d dmd/root/response.d dmd/root/rmem.d dmd/root/rootobject.d dmd/root/speller.d dmd/root/longdouble.d dmd/root/strtold.d dmd/root/stringtable.d dmd/root/hash.d dmd/root/string.d ../generated/linux/release/64/lexer.a ../generated/linux/release/64/backend.o
In function ‘path’,
    inlined from ‘ensurePathExists’ at dmd/root/filename.d:862:22:
dmd/root/filename.d:325:9: error: ‘__builtin_memcpy’ specified bound 18446744073709551615 exceeds maximum object size 9223372036854775807 [-Werror=stringop-overflow=]
  325 |         memcpy(path, str.ptr, pathlen);
      |         ^
In function ‘path’,
    inlined from ‘ensurePathToNameExists’ at dmd/utils.d:100:18:
dmd/root/filename.d:325:9: error: ‘__builtin_memcpy’ specified bound 18446744073709551615 exceeds maximum object size 9223372036854775807 [-Werror=stringop-overflow=]
  325 |         memcpy(path, str.ptr, pathlen);
      |         ^
d21: all warnings being treated as errors
make: *** [../generated/linux/release/64/dmd] Error 1

For more info:
https://gcc.gnu.org/onlinedocs/gcc/Warning-Options.html#index-Wstringop-overflow

wilzbach · 2019-07-28T03:25:19Z

src/build.d

    };
+    sources.backend ~=  ((env["OS"] == "windows" && env["MODEL"] == "64") ? ["strtold.d", "longdouble.d"] : [])


Why not use a good old if?

wilzbach · 2019-07-28T03:26:46Z

src/build.d

+/// Returns the first argument if the given version is true, otherwise, returns the second argument.
+auto versionTernary(string version_, T)(T trueCase, T falseCase)
+{
+    mixin("version(" ~ version_ ~ ") { return trueCase; }");


Can't we look up the OS in env?

wilzbach · 2019-08-11T00:02:27Z

src/build.d

+            cod3.d cv8.d dcgcv.d pdata.d util2.d var.d md5.d backconfig.d ph2.d drtlsym.d dwarfeh.d ptrntab.d
+            dvarstats.d dwarfdbginf.d cgen.d os.d goh.d barray.d cgcse.d elpicpie.d
+            ".split
+            ~ ( (env["OS"] == "osx") ? ["machobj.d"] : ["elfobj.d"] )


I presume you copied this from the posix makefil, but as there's windows too shouldn't it be sth. like:

( (env["OS"] == "osx") ? ["machobj.d"] : [] ) ( (env["OS"] == "linux" || env["OS"] == "freebsd") ? ["elfobj.d"] : [] )

Though you can skip all of this logic, because these files have version (OS) headers, so there's no problem in keeping things simple and including all.

wilzbach · 2019-08-11T00:06:15Z

Windows failure on Azure is not very insightful:

Test runnable\test18772.d failed.  The logged output:
..\generated\windows\release\64\dmd.exe -conf= -m64 -Irunnable   -L/OPT:NOICF  -odtest_results\runnable -oftest_results\runnable\test18772_0.exe  runnable\test18772.d  -map nul.map


Assertion failed: stackused == 0, file D:\a\1\s\src\dmd\backend\cgcod.d, line 697


==============================

Test runnable\test18772.d failed: expected rc == 0, exited with rc == 9

CC @rainers

edit: saw that #10231 or #10247 would resolve this.

wilzbach

.

Edit: GH mobile false review.

PetarKirov · 2019-08-29T13:18:58Z

@thewilsonator @marler8997
This PR broke CircleCI for phobos. The difference between the dmd CircleCI build and the phobos CircleCI build comes down to:

make -f posix.mak HOST_DMD=dmd (dmd)
make -f posix.mak HOST_DMD=dmd BUILD=debug all (phobos)

I.e. the BUILD=debug build type of posix.mak is broken. Compare:

git checkout 1309c79ec # The merge commit of this PR
make -f posix.mak HOST_DMD=dmd clean
make -f posix.mak HOST_DMD=dmd BUILD=debug all # fails

git checkout 1309c79ec^ # Checkout dmd before this PR was merged
make -f posix.mak HOST_DMD=dmd clean
make -f posix.mak HOST_DMD=dmd BUILD=debug all # succeeds

…posix.mak PR dlang/dmd#10212 broke building `dmd/src/posix.mak` with `BUILD=debug`. Use `build.d` instead.

marler8997 requested review from MartinNowak and wilzbach as code owners July 24, 2019 02:52

marler8997 force-pushed the makefileRemovals2 branch 2 times, most recently from 8f44a0b to b420999 Compare July 24, 2019 02:55

thewilsonator approved these changes Jul 24, 2019

View reviewed changes

src/dmd/backend/cgobj.d Outdated Show resolved Hide resolved

marler8997 force-pushed the makefileRemovals2 branch 9 times, most recently from e4fedad to 7a035fa Compare July 24, 2019 05:22

marler8997 force-pushed the makefileRemovals2 branch 2 times, most recently from 4421de2 to 0cbd257 Compare July 26, 2019 02:13

wilzbach reviewed Jul 28, 2019

View reviewed changes

marler8997 force-pushed the makefileRemovals2 branch 9 times, most recently from cf332f4 to 3a787aa Compare July 30, 2019 14:38

marler8997 mentioned this pull request Jul 31, 2019

Fix issue 20089: Handle extra case in fixresult_complex87 #10247

Merged

marler8997 force-pushed the makefileRemovals2 branch from 3a787aa to 406d77f Compare July 31, 2019 04:39

marler8997 force-pushed the makefileRemovals2 branch from 406d77f to 1dbae43 Compare August 3, 2019 22:34

wilzbach reviewed Aug 11, 2019

View reviewed changes

marler8997 force-pushed the makefileRemovals2 branch 3 times, most recently from 3a4c3e2 to a05801f Compare August 20, 2019 13:09

thewilsonator added the auto-merge label Aug 20, 2019

marler8997 force-pushed the makefileRemovals2 branch from a05801f to a5fb684 Compare August 21, 2019 04:23

dlang-bot removed the auto-merge label Aug 21, 2019

Second round of makefile/build.d consolidation

66b0ede

marler8997 force-pushed the makefileRemovals2 branch from a5fb684 to 66b0ede Compare August 24, 2019 06:57

Merge branch 'master' into makefileRemovals2

46b13c0

thewilsonator added the auto-merge label Aug 27, 2019

dlang-bot merged commit 1309c79 into dlang:master Aug 27, 2019

wilzbach reviewed Aug 29, 2019

View reviewed changes

PetarKirov mentioned this pull request Aug 29, 2019

Fix CircleCI build dlang/phobos#7160

Merged

PetarKirov added a commit to dlang/phobos that referenced this pull request Aug 29, 2019

[circleci] Fix building dmd - use dmd/src/build.d instead of dmd/src/…

6a6bdc3

…posix.mak PR dlang/dmd#10212 broke building `dmd/src/posix.mak` with `BUILD=debug`. Use `build.d` instead.

PetarKirov added a commit to dlang/phobos that referenced this pull request Aug 29, 2019

[circleci] Fix building dmd - use dmd/src/build.d instead of dmd/src/…

7e15364

…posix.mak PR dlang/dmd#10212 broke building `dmd/src/posix.mak` with `BUILD=debug`. Use `build.d` instead.

PetarKirov added a commit to dlang/phobos that referenced this pull request Aug 29, 2019

[circleci] Fix building dmd - use dmd/src/build.d instead of dmd/src/…

de6c26e

…posix.mak PR dlang/dmd#10212 broke building `dmd/src/posix.mak` with `BUILD=debug`. Use `build.d` instead.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Second round of makefile/build.d consolidation #10212

Second round of makefile/build.d consolidation #10212

marler8997 commented Jul 24, 2019 •

edited

Loading

dlang-bot commented Jul 24, 2019

thewilsonator left a comment

PetarKirov commented Jul 24, 2019 •

edited

Loading

wilzbach Jul 28, 2019

wilzbach Jul 28, 2019

wilzbach Aug 11, 2019

wilzbach commented Aug 11, 2019 •

edited

Loading

wilzbach left a comment •

edited

Loading

PetarKirov commented Aug 29, 2019 •

edited

Loading

		};
		sources.backend ~= ((env["OS"] == "windows" && env["MODEL"] == "64") ? ["strtold.d", "longdouble.d"] : [])

Second round of makefile/build.d consolidation #10212

Second round of makefile/build.d consolidation #10212

Conversation

marler8997 commented Jul 24, 2019 • edited Loading

dlang-bot commented Jul 24, 2019

Bugzilla references

Testing this PR locally

thewilsonator left a comment

Choose a reason for hiding this comment

PetarKirov commented Jul 24, 2019 • edited Loading

wilzbach Jul 28, 2019

Choose a reason for hiding this comment

wilzbach Jul 28, 2019

Choose a reason for hiding this comment

wilzbach Aug 11, 2019

Choose a reason for hiding this comment

wilzbach commented Aug 11, 2019 • edited Loading

wilzbach left a comment • edited Loading

Choose a reason for hiding this comment

PetarKirov commented Aug 29, 2019 • edited Loading

marler8997 commented Jul 24, 2019 •

edited

Loading

PetarKirov commented Jul 24, 2019 •

edited

Loading

wilzbach commented Aug 11, 2019 •

edited

Loading

wilzbach left a comment •

edited

Loading

PetarKirov commented Aug 29, 2019 •

edited

Loading