win: precompile=yes #17179

vtjnash · 2016-06-29T03:42:46Z

This should be the last item for making the sys.dll image reliable on x86_64. I didn't test x86, so for all I know, it's probably been doing just fine with --precompile=yes (since the OS is less demanding).

(Personality is a technical term ≠ Personification)

also fix the test for this to actually test for this, instead of injecting a non-default build configuration in order to test for something that bootstrapping is already testing fix #16953

tkelman · 2016-06-29T03:58:50Z

it's probably been doing just fine

It has not, it's missing type info. This would be a regression in backtrace quality on win32 which is still somewhat widely used.

tkelman · 2016-06-29T04:00:15Z

src/signals-win.c

@@ -278,8 +278,7 @@ static LONG WINAPI exception_handler(struct _EXCEPTION_POINTERS *ExceptionInfo)
 }

 #if defined(_CPU_X86_64_)
-EXCEPTION_DISPOSITION _seh_exception_handler(PEXCEPTION_RECORD ExceptionRecord, void *EstablisherFrame, PCONTEXT ContextRecord, void *DispatcherContext)
-{
+JL_DLLEXPORT EXCEPTION_DISPOSITION __julia_personality(PEXCEPTION_RECORD ExceptionRecord, void *EstablisherFrame, PCONTEXT ContextRecord, void *DispatcherContext) {


this should be wrapped

vtjnash · 2016-06-29T04:00:26Z

This would be a regression in backtrace quality on win32 which is still somewhat widely used.

technically this isn't a regression, since it's a new feature introduced in the current development cycle

tkelman · 2016-06-29T04:01:07Z

.travis.yml

@@ -56,7 +56,6 @@ notifications:
          - http://julia.mit.edu:8000/travis-hook
 before_install:
    - make check-whitespace
-    - JULIA_SYSIMG_BUILD_FLAGS="--output-ji ../usr/lib/julia/sys.ji"


bootstrapping is NOT testing the case where you have a [sys.]ji file but not a dll file, which systems without linkers need to continue testing

and I added a separate test of --precompiled=no in cmdlineargs, these are testing separate things

changes to tests and CI configuration really do not belong in the same commit as changing the default value here

bootstrapping is NOT testing the case where you have a ji file but not a dll file

that's exactly what bootstrapping tests.

which systems without linkers need to continue testing

now that's just a red herring, since this is make, we just ran the linker

changes to tests and CI configuration really do not belong in the same commit as changing the default value here

the changes are inter-related (aside from the part where this test has severely bit-rotted)

bootstrapping doesn't have a sys.ji - this test is to be sure build_sysimg keeps working, where we may not have a linker

I'm well aware of how bootstrapping works, and that it cycles through several names (not currently including sys.ji). The codepath that was being tested for in build_sysimg doesn't exist anymore. But forcing a non-default build configuration here has the potential to cause us to miss bugs (#16907) and is preventing me from testing other more interesting code paths (#16059).

Now that --precompiled=no is tested separately we wouldn't have missed #16907 locally. If this doesn't need to be deleted to add new functionality or fix the exact bug that this PR fixes, don't delete it now, in this PR. The use case of running build_sysimg from a binary without having a toolchain present certainly still exists, and should be tested. We could instead add a separate makefile target for a ji-only sys.ji output and have that build in parallel on CI if that's in any way better. Can you leave this alone for now and address it elsewhere?

I think we're talking past each other. I don't care how it gets built, we can remove support for JULIA_SYSIMG_BUILD_FLAGS if you want (better in #16059 though, not here). As long as a no-linker ~~.ji~~ sys.ji file gets built somehow, separately from sys.dll is fine (parallel make to save CI time), and we test that Julia can start with it, I'm satisfied.

We do. It's a standard step in the build process.

It's also not what this test was written to test (since that's unnecessarily redundant and not quite correct). This was originally written to test the --precompile=no configuration, but was later mistakenly altered to its current form instead.

I'm confused - we only emit inference.ji and sys.o in a default build. I was specifically referring to sys.ji and should have spelled that out.

Right, when the --precompiled=no configuration changed forms to being bundled inside sys.dll (and introduced as that flag) in a default build, the remaining purpose of this test was then only about the no-linker build_sysimg use case.

nalimilan · 2016-06-29T07:51:22Z

This would be a regression in backtrace quality on win32 which is still somewhat widely used.

@tkelman Is 32-bit Windows so common among Julia users? On Linux, 32-bit seems to be very rare according to the RPM download stats. I couldn't find data regarding Windows, but for example this survey among gamers (OK, a very specific population) show very few 32-bit machines: http://store.steampowered.com/hwsurvey

tkelman · 2016-06-29T12:32:32Z

32 bit OS is not so common, but downloading and using the 32 bit binaries on a 64 bit OS is still common, about a third of the win64 downloads last I checked. Will try to get more recent numbers.

nalimilan · 2016-06-29T14:41:20Z

Maybe we should make it more obvious that 64-bit is recommended? Unless there can be bugs when interacting with other programs of course.

tkelman · 2016-06-29T14:58:08Z

If you need to call into a 32 bit Python or other libraries where you might only have access to 32 bit binaries via ccall or similar, you need to use the same bitness.

tkelman · 2016-06-29T17:23:02Z

.travis.yml

-    - cp /tmp/julia/lib/julia/sys.ji local.ji && /tmp/julia/bin/julia -J local.ji -e 'true' &&
-        /tmp/julia/bin/julia-debug -J local.ji -e 'true' && rm local.ji
+    - /tmp/julia/bin/julia --precompiled=no -e 'true' &&
+      /tmp/julia/bin/julia-debug --precompiled=no -e 'true'


leave the indent otherwise it's not immediately obvious this is a line continuation

Keno · 2016-07-01T18:02:54Z

Cxx.jl has custom personalities on functions that it generates. Just want to make sure those don't get overriden.

Keno · 2016-07-01T18:05:42Z

Error during bootstrap after merging this:

Referencing personality function in another module!
%jl_value_t* (%jl_value_t*, i64)* @julia_getindex_3251
i32 (...)* @__julia_personality

vtjnash · 2016-07-01T19:01:32Z

Ah, of course. Will need to teach merge module about this.

Keno · 2016-07-01T23:26:28Z

Fix for the assertion failure as discussed. Breadcrumbs for issues this needed to avoid:
llvm-mirror/llvm@48f4451
llvm-mirror/llvm@fdf52d3

tkelman · 2016-07-01T23:28:48Z

src/jitlayers.cpp

+            // Add unwind exception personalities to functions to handle async exceptions
+            assert(!juliapersonality_func || juliapersonality_func->getParent() == shadow_output);
+            if (Function *F = dyn_cast<Function>(G))
+               F->setPersonalityFn(juliapersonality_func);


vtjnash added 3 commits June 28, 2016 23:29

win: cleanup build warnings

558f44a

win64: add Julia personality

bf3b97e

set precompiled=yes by default on windows

9ad1357

also fix the test for this to actually test for this, instead of injecting a non-default build configuration in order to test for something that bootstrapping is already testing fix #16953

tkelman reviewed Jun 29, 2016
View reviewed changes

yuyichao mentioned this pull request Jul 1, 2016

Wine errors #17232

Closed

Keno mentioned this pull request Jul 1, 2016

win: cleanup build warnings #17233

Merged

kshyatt added the system:windows Affects only Windows label Jul 1, 2016

tkelman reviewed Jul 1, 2016
View reviewed changes

Fix assertion failure on LLVM 3.8+

5f31073

Keno force-pushed the jn/personality branch from 28d7c3d to 5f31073 Compare July 1, 2016 23:33

vtjnash merged commit 2e1bcc6 into master Jul 3, 2016

vtjnash deleted the jn/personality branch July 3, 2016 00:01

vtjnash mentioned this pull request Jul 3, 2016

win32 missing linfo lookup #17251

Closed

tkelman mentioned this pull request Jul 26, 2016

Add a few tests for backtrace quality #17628

Merged

tkelman mentioned this pull request Apr 14, 2017

Exit calls in build scripts no longer build to abort #20070

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

win: precompile=yes #17179

win: precompile=yes #17179

vtjnash commented Jun 29, 2016

tkelman commented Jun 29, 2016

tkelman Jun 29, 2016

vtjnash commented Jun 29, 2016 •

edited

Loading

tkelman Jun 29, 2016 •

edited

Loading

vtjnash Jun 29, 2016

tkelman Jun 29, 2016

vtjnash Jun 29, 2016

tkelman Jun 29, 2016

tkelman Jun 29, 2016 •

edited

Loading

vtjnash Jun 29, 2016

vtjnash Jun 29, 2016

tkelman Jun 29, 2016

tkelman Jun 29, 2016 •

edited

Loading

nalimilan commented Jun 29, 2016

tkelman commented Jun 29, 2016

nalimilan commented Jun 29, 2016

tkelman commented Jun 29, 2016

tkelman Jun 29, 2016

Keno commented Jul 1, 2016

Keno commented Jul 1, 2016

vtjnash commented Jul 1, 2016

Keno commented Jul 1, 2016

tkelman Jul 1, 2016

win: precompile=yes #17179

win: precompile=yes #17179

Conversation

vtjnash commented Jun 29, 2016

tkelman commented Jun 29, 2016

Choose a reason for hiding this comment

vtjnash commented Jun 29, 2016 • edited Loading

tkelman Jun 29, 2016 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tkelman Jun 29, 2016 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tkelman Jun 29, 2016 • edited Loading

Choose a reason for hiding this comment

nalimilan commented Jun 29, 2016

tkelman commented Jun 29, 2016

nalimilan commented Jun 29, 2016

tkelman commented Jun 29, 2016

Choose a reason for hiding this comment

Keno commented Jul 1, 2016

Keno commented Jul 1, 2016

vtjnash commented Jul 1, 2016

Keno commented Jul 1, 2016

Choose a reason for hiding this comment

vtjnash commented Jun 29, 2016 •

edited

Loading

tkelman Jun 29, 2016 •

edited

Loading

tkelman Jun 29, 2016 •

edited

Loading

tkelman Jun 29, 2016 •

edited

Loading