merge_llvm_401 #5400
merge_llvm_401 #5400
Conversation
|
This is to match when emscripten-core/emscripten-fastcomp#191 lands. |
src/library.js
Outdated
| @@ -1464,6 +1464,8 @@ LibraryManager.library = { | |||
|
|
|||
| llvm_lifetime_start: function() {}, | |||
| llvm_lifetime_end: function() {}, | |||
| llvm_lifetime_start_p0i8: function() {}, | |||
| llvm_lifetime_end_p0i8: function() {}, | |||
There was a problem hiding this comment.
I think these should be implemented in CallHanders.h in fastcomp, so that we emit no-ops for them (instead of emitting ffi calls that are then no-ops).
There was a problem hiding this comment.
That is a great idea. Applied that treatment to all of these no-ops.
| # For fastcomp backend, no LLVM IR functions should ever be annotated 'optnone', because that would skip running the SimplifyCFG pass on them, which is required to always run to | ||
| # clean up LandingPadInst instructions that are not needed. | ||
| if not shared.Settings.WASM_BACKEND: | ||
| args += ['-Xclang', '-disable-O0-optnone'] |
There was a problem hiding this comment.
Are you saying that llvm changed semantics so that the SimplifyCFG pass will not run on optnone methods now? Or is optnone a new thing in this merge?
Perhaps there are other consequences to not doing optnone? Like slower compile times for -O0?
There was a problem hiding this comment.
Are you saying that llvm changed semantics so that the SimplifyCFG pass will not run on optnone methods now?
That semantic has not changed, all optimization passes have a test for if (func is optnone) skip this pass on this function;
Or is optnone a new thing in this merge?
The new thing is that the optnone attribute is now getting slapped to all functions when compiling code with -O0 optimization level, whereas it used to not be added. When this change was done, the -disable-O0-optnone flag was added to allow reverting to the earlier behavior of not producing all functions with optnone.
There was a problem hiding this comment.
This does still worry me, but the only concrete concern I have is compile times. Can you please check if -O0 compile times vary much with and without that flag? (on a file where it doesn't crash the compiler to not run that pass that optnone disables).
There was a problem hiding this comment.
Check, running timing benchmarks now
There was a problem hiding this comment.
Running default suite with -disable-O0-optnone present gives 1367 seconds, and running default suite without -disable-O0-optnone gives 1303 seconds (-4.68%), though that suite fails on 7 tests, so not sure if that has a bearing.
However, I don't think it's fair to compare compile times to the version where the optnone flag is present, since that is not the original status quo - we have never generated code with that attribute applied to all functions. I.e. we used to behave as if optnone did not exist. See this commit that introduced the -disable-O0-optnone flag: juj/emscripten-fastcomp-clang@7a45503 It is that commit which also introduced the sprinkling of optnone attribute everywhere in -O0.
There was a problem hiding this comment.
Running a comparison against pre-merge branch next.
There was a problem hiding this comment.
Ran default suite in current incoming branch, getting 1433 seconds as runtime. So at least the update will not regress overall compilation performance, but improves it by a little (-4.61% less time).
tests/core/test_llvm_intrinsics.out
Outdated
| @@ -62,12 +62,12 @@ llvm_floor_f64 -2.0 | |||
| llvm_round_f64 20.0 | |||
| llvm_round_f64 21.0 | |||
| llvm_round_f64 42.0 | |||
| llvm_round_f64 -20.0 | |||
| llvm_round_f64 -21.0 | |||
There was a problem hiding this comment.
i'm a little worried about changing rounding semantics as part of this merge. optimally we'd do it separately.
I think I remember you having good reasons for this change, where are they documented?
There was a problem hiding this comment.
Ok, sounds good. But please add to that test, as before it showed the decision point (one result was -20, one was -21). I.e., can add
printf("llvm_round_f32 %.1f\n", llvm_round_f32(-20.49)); // new line, should have different result
printf("llvm_round_f32 %.1f\n", llvm_round_f32(-20.5));
printf("llvm_round_f32 %.1f\n", llvm_round_f32(-20.51));
juj
force-pushed
the
merge_llvm_401
branch
2 times, most recently
from
936a6e4
to
be8dee2
Compare
…om zero, independent of current active rounding mode, and rint() family should respect the active rounding mode (we only implement one - FE_TONEAREST i.e. round-to-even, or banker's rounding)
…s/LangRef.html#id508 states that llvm.round 'returns the same values as the libm round functions would', and http://en.cppreference.com/w/c/numeric/math/round says that round should always round away from zero, regardless of the current rounding mode.
…LVM 4.0.1 tagged release in LLVM upstream SVN has
…n -O0 optimization level is present, since optnone would prevent SimplifyCFG pass from being run, which is needed in fastcomp to remove LandingPadInsts when exception catching is not enabled.
Update from LLVM 4.0 to LLVM 4.0.1, add new llvm_lifetime_start and _end intrinsics.