Merge lp:~raof/mir/fix-and-enable-lto into lp:mir
| Status: | Merged |
|---|---|
| Approved by: | Alan Griffiths on 2016-03-01 |
| Approved revision: | 3350 |
| Merged at revision: | 3353 |
| Proposed branch: | lp:~raof/mir/fix-and-enable-lto |
| Merge into: | lp:mir |
| Diff against target: |
593 lines (+144/-59) 30 files modified
CMakeLists.txt (+7/-0) benchmarks/frame-uniformity/CMakeLists.txt (+6/-0) cmake/FindGtestGmock.cmake (+1/-1) cmake/MirCommon.cmake (+42/-9) debian/rules (+7/-1) examples/CMakeLists.txt (+0/-2) examples/animated_cursor_demo_client.c (+0/-2) src/client/buffer_stream.cpp (+1/-1) src/client/buffer_stream.h (+1/-1) src/client/client_buffer_stream.h (+25/-2) src/client/lttng/CMakeLists.txt (+5/-0) src/client/screencast_stream.cpp (+1/-1) src/client/screencast_stream.h (+1/-1) src/common/sharedlibrary/module_deleter.cpp (+0/-3) src/common/thread/thread_name.cpp (+0/-3) src/include/client/mir/client_platform.h (+3/-1) src/platforms/android/client/android_client_platform.cpp (+3/-6) src/platforms/android/client/android_client_platform.h (+1/-1) src/platforms/common/client/mir/aging_buffer.h (+3/-3) src/platforms/mesa/client/client_platform.cpp (+3/-6) src/platforms/mesa/client/client_platform.h (+1/-1) src/server/report/lttng/CMakeLists.txt (+5/-0) tests/acceptance-tests/throwback/test_client_library_errors.cpp (+1/-1) tests/include/mir_test_framework/client_platform_factory.h (+20/-2) tests/include/mir_test_framework/stub_client_platform_factory.h (+1/-1) tests/mir_test/current_thread_name.cpp (+0/-3) tests/mir_test_framework/stub_client_platform_factory.cpp (+2/-3) tests/unit-tests/client/test_client_buffer_stream.cpp (+2/-2) tests/unit-tests/client/test_client_platform.cpp (+1/-1) tests/unit-tests/client/test_mir_connection.cpp (+1/-1) |
| To merge this branch: | bzr merge lp:~raof/mir/fix-and-enable-lto |
| Related bugs: |
| Reviewer | Review Type | Date Requested | Status |
|---|---|---|---|
| Mir CI Bot | continuous-integration | 2016-01-11 | Approve on 2016-03-01 |
| Alan Griffiths | Approve on 2016-02-29 | ||
| Cemil Azizoglu (community) | Approve on 2016-02-26 | ||
| PS Jenkins bot | continuous-integration | 2016-01-11 | Needs Fixing on 2016-02-24 |
| Andreas Pokorny (community) | 2016-01-11 | Approve on 2016-01-21 | |
|
Review via email:
|
|||
This proposal supersedes a proposal from 2015-12-22.
Commit Message
Fix LTO build, and enable it for Debian packages.
Description of the Change
Fix LTO build, and enable it for Debian packages.
| Chris Halse Rogers (raof) wrote : | # |
/usr/include/
struct stat
^
/usr/include/
struct stat
^
/usr/include/
__ino_t st_ino; /* File serial number. */
^
/usr/include/
__ino_t __st_ino; /* 32bit file serial number. */
Interesting... this seems to be a genuine error; we've included stat.h with twice, with different compile options, really giving us two different definitions of struct stat.
| Andreas Pokorny (andreas-pokorny) wrote : | # |
hmm will gcc-nm and gcc-ar work for cross compilation?
| Chris Halse Rogers (raof) wrote : | # |
That's a fair question. Looks like it works, which somewhat surprised me :).
Of course, this also means that a cross-compile doesn't hit the issue that hit mediumtests-
| Andreas Pokorny (andreas-pokorny) wrote : | # |
ah the job of nm and ar is more elf-ish, tar-ish and less-ish so they might not need to interfere with the actual target format
| Alan Griffiths (alan-griffiths) wrote : | # |
Not sure why LP has this a "Merged" - it clearly isn't.
| PS Jenkins bot (ps-jenkins) wrote : | # |
FAILED: Continuous integration, rev:3236
http://
Executed test runs:
FAILURE: http://
SUCCESS: http://
FAILURE: http://
SUCCESS: http://
deb: http://
FAILURE: http://
FAILURE: http://
Click here to trigger a rebuild:
http://
| PS Jenkins bot (ps-jenkins) wrote : | # |
FAILED: Continuous integration, rev:3236
http://
Executed test runs:
FAILURE: http://
SUCCESS: http://
FAILURE: http://
SUCCESS: http://
deb: http://
FAILURE: http://
FAILURE: http://
Click here to trigger a rebuild:
http://
| Mir CI Bot (mir-ci-bot) wrote : | # |
PASSED: Continuous integration, rev:3236
https:/
Executed test runs:
None: https:/
Click here to trigger a rebuild:
https:/
| PS Jenkins bot (ps-jenkins) wrote : | # |
FAILED: Continuous integration, rev:3236
http://
Executed test runs:
FAILURE: http://
SUCCESS: http://
FAILURE: http://
SUCCESS: http://
deb: http://
FAILURE: http://
FAILURE: http://
Click here to trigger a rebuild:
http://
| PS Jenkins bot (ps-jenkins) wrote : | # |
FAILED: Continuous integration, rev:3238
http://
Executed test runs:
SUCCESS: http://
SUCCESS: http://
FAILURE: http://
SUCCESS: http://
deb: http://
SUCCESS: http://
deb: http://
SUCCESS: http://
deb: http://
FAILURE: http://
SUCCESS: http://
Click here to trigger a rebuild:
http://
| Mir CI Bot (mir-ci-bot) wrote : | # |
PASSED: Continuous integration, rev:3238
https:/
Executed test runs:
None: https:/
Click here to trigger a rebuild:
https:/
| Andreas Pokorny (andreas-pokorny) wrote : | # |
https:/
[1452488342.218071] mirserver: Selected driver: android (version 0.19.0)
ERROR: Dynamic exception type: St12system_error
std::exception:
Do we need to clutter cmake with more pthread flags?
Apart from that. I can live with that fancy solution.
| Alan Griffiths (alan-griffiths) wrote : | # |
Apart from the android failure looks good.
Should we also force use of gold with LTO?
| PS Jenkins bot (ps-jenkins) wrote : | # |
FAILED: Continuous integration, rev:3238
http://
Executed test runs:
FAILURE: http://
None: http://
None: http://
FAILURE: http://
FAILURE: http://
FAILURE: http://
Click here to trigger a rebuild:
http://
| Andreas Pokorny (andreas-pokorny) wrote : | # |
There is now a conflict, when resolved there is a chance that the problem on vivid/armhf is gone.
| PS Jenkins bot (ps-jenkins) wrote : | # |
FAILED: Continuous integration, rev:3238
http://
Executed test runs:
FAILURE: http://
FAILURE: http://
FAILURE: http://
FAILURE: http://
FAILURE: http://
FAILURE: http://
Click here to trigger a rebuild:
http://
| Mir CI Bot (mir-ci-bot) wrote : | # |
FAILED: Continuous integration, rev:3332
https:/
Executed test runs:
FAILURE: https:/
FAILURE: https:/
FAILURE: https:/
FAILURE: https:/
FAILURE: https:/
FAILURE: https:/
FAILURE: https:/
FAILURE: https:/
Click here to trigger a rebuild:
https:/
| Mir CI Bot (mir-ci-bot) wrote : | # |
FAILED: Continuous integration, rev:3340
https:/
Executed test runs:
FAILURE: https:/
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
FAILURE: https:/
FAILURE: https:/
deb: https:/
FAILURE: https:/
FAILURE: https:/
Click here to trigger a rebuild:
https:/
| PS Jenkins bot (ps-jenkins) wrote : | # |
FAILED: Continuous integration, rev:3340
http://
Executed test runs:
SUCCESS: http://
SUCCESS: http://
FAILURE: http://
FAILURE: http://
FAILURE: http://
FAILURE: http://
Click here to trigger a rebuild:
http://
| Mir CI Bot (mir-ci-bot) wrote : | # |
PASSED: Continuous integration, rev:3341
https:/
Executed test runs:
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
Click here to trigger a rebuild:
https:/
| Alan Griffiths (alan-griffiths) wrote : | # |
+static_assert(
+ std::is_
+ sizeof(
+ "Oh, oh. EGLNativeWindowType isn't losslessly convertible to void*.\n"
+ "The ClientBufferStream requires that EGLNativeWindowType be convertible to void*");
It seems odd that this is needed for the LTO MP and that it is needed in two headers (out of the five that #include <EGL/eglplatfor
What's the reasoning for this approach and should we have a single wrapper header for all uses?
| Chris Halse Rogers (raof) wrote : | # |
Moved the static_assert() check to a more relevant header, and gave it a more explicit comment.
It's here because the LTO turned out to allow a check which found that we were returning different types from different virtual overrides, because EGLNativeWindowType is a different type on different EGL platforms.
| Mir CI Bot (mir-ci-bot) wrote : | # |
FAILED: Continuous integration, rev:3348
https:/
Executed test runs:
FAILURE: https:/
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
FAILURE: https:/
SUCCESS: https:/
deb: https:/
FAILURE: https:/
FAILURE: https:/
SUCCESS: https:/
deb: https:/
Click here to trigger a rebuild:
https:/
| Chris Halse Rogers (raof) wrote : | # |
Ah, there it is.
Different layouts for BufferStream because EGLNativeWindowType differs...
- 3349. By Chris Halse Rogers on 2016-02-26
-
Remove the platform-specific EGLNativeWindowType from the generic interface
The EGLNativeWindowType differs per EGL platform, which makes it a poor
choice for platform-independent interfaces :). In practise it's always a pointer or a pointer-sized integer, so use a
void* for the interface.
| Mir CI Bot (mir-ci-bot) wrote : | # |
FAILED: Continuous integration, rev:3349
https:/
Executed test runs:
FAILURE: https:/
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
FAILURE: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
Click here to trigger a rebuild:
https:/
| Chris Halse Rogers (raof) wrote : | # |
I'm going to disable LTO on the clang build; it's too difficult to debug with clang being all not working in recent releases.
| Mir CI Bot (mir-ci-bot) wrote : | # |
PASSED: Continuous integration, rev:3349
https:/
Executed test runs:
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
Click here to trigger a rebuild:
https:/
| Alan Griffiths (alan-griffiths) wrote : | # |
+ -DMIR_LINK_
While this is probably right, we ought to be explicit about which builds we want this optimization for (as it can slow things quite a bit).
I'd suggest not for the "normal" CI and developer builds where fast turnaround is desirable.
I think problems will be rare enough that enabling for autolanding would be fine as a final check. And for release.
Is that what happens?
(It also suggests the question of any compile-time optimizations we might want to enable.)
| Cemil Azizoglu (cemil-azizoglu) wrote : | # |
> I think problems will be rare enough that enabling for autolanding would be
> fine as a final check. And for release.
+1
- 3350. By Chris Halse Rogers on 2016-02-28
-
Only enable MIR_LINK_
TIME_OPTIMIZATI ON when DEB_BUILD_OPTIONS does not contain noopt
| Chris Halse Rogers (raof) wrote : | # |
Ok. I've gated MIR_LINK_
I've also started to hook this up to CI, so that we pass DEB_BUILD_
Sadly CI is currently broken, having run out of disc space, and I don't think I have the ability to fix it without a shell on that machine.
| Mir CI Bot (mir-ci-bot) wrote : | # |
FAILED: Continuous integration, rev:3350
https:/
Executed test runs:
FAILURE: https:/
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
FAILURE: https:/
deb: https:/
Click here to trigger a rebuild:
https:/
| Mir CI Bot (mir-ci-bot) wrote : | # |
PASSED: Continuous integration, rev:3350
https:/
Executed test runs:
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
Click here to trigger a rebuild:
https:/
FAILED: Continuous integration, rev:3218
http://
Executed test runs:
SUCCESS: http://
SUCCESS: http://
FAILURE: http://
SUCCESS: http://
deb: http://
FAILURE: http://
FAILURE: http://
Click here to trigger a rebuild:
http://