cmake: Compile applications with O1 optimization level (sel4bench/sel4test) #243
Conversation
Signed-off-by: Jakub Duchniewicz <[email protected]>
JDuchniewicz
force-pushed
the
benchmark_fixes
branch
from
9493479 to
ef5163d
Compare
Indanz
left a comment
Indanz
left a comment
There was a problem hiding this comment.
Does this override the setting configured with CMAKE_C_FLAGS by the user, or is that applied later so it can still overrule this setting?
If no, that's a problem. If yes, isn't this just being overruled by the default value of CMAKE_C_FLAGS* anyway?
In other words, shouldn't you change the default values of CMAKE_C_FLAGS_DEBUG, CMAKE_C_FLAGS_RELEASE etc. instead of this?
| if(NOT (("${CMAKE_BUILD_TYPE}" STREQUAL "Release") AND ("${CMAKE_BUILD_TYPE}" STREQUAL | ||
| "MinSizeRel")) | ||
| ) | ||
| add_compile_options(-O1) |
There was a problem hiding this comment.
If this is the style the syntax checker wants it, then we really should disable it for CMake. It wants to make everything extremely ugly.
Can you please simplify the check to just if(NOT "${CMAKE_BUILD_TYPE}" STREQUAL "MinSizeRel")?
Please separate content changes and style changes in different commits (and I personally would omit the style fixes altogether as a matter of principle).
|
I just realised that you are changing the default compile option to O1 for all applications using cmake-tool/helpers, not just sel4bench. Please don't do that, that will cause a regression for all current other users of seL4_tools. |
|
I would also not want sel4test to be compiled with -O1, it should be testing realistic user-level settings. |
|
Okay, I will restrict it to sel4bench only in such case. |
3 participants
To make results in sel4bench less noisy, we reduce the build optimization level to
O1. This solves one of the issues raised in: seL4/sel4bench#65