Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Don't use "mostly debug runtime" with release configurations #4112

Merged
merged 7 commits into from
May 13, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
60 changes: 30 additions & 30 deletions .cirrus.yml
Original file line number Diff line number Diff line change
Expand Up @@ -31,18 +31,18 @@ task:
upload_caches:
- libs

release_configure_script:
- make configure arch=x86-64 config=release
release_build_script:
- make build config=release
release_test_script:
- make test-ci config=release
debug_configure_script:
- make configure arch=x86-64 config=debug
debug_build_script:
- make build config=debug
debug_test_script:
- make test-ci config=debug
release_configure_script:
- make configure arch=x86-64 config=release
release_build_script:
- make build config=release
release_test_script:
- make test-ci config=release

task:
only_if: $CIRRUS_PR != ''
Expand All @@ -66,18 +66,18 @@ task:
upload_caches:
- libs

release_configure_script:
- make configure arch=armv8-a config=release
release_build_script:
- make build config=release
release_test_script:
- make test-ci config=release
debug_configure_script:
- make configure arch=armv8-a config=debug
debug_build_script:
- make build config=debug
debug_test_script:
- make test-ci config=debug
release_configure_script:
- make configure arch=armv8-a config=release
release_build_script:
- make build config=release
release_test_script:
- make test-ci config=release

task:
only_if: $CIRRUS_PR != ''
Expand Down Expand Up @@ -107,18 +107,18 @@ task:
upload_caches:
- libs

release_configure_script:
- gmake configure arch=x86-64 config=release
release_build_script:
- gmake build config=release
release_test_script:
- gmake test-ci config=release
debug_configure_script:
- gmake configure arch=x86-64 config=debug
debug_build_script:
- gmake build config=debug
debug_test_script:
- gmake test-ci config=debug
release_configure_script:
- gmake configure arch=x86-64 config=release
release_build_script:
- gmake build config=release
release_test_script:
- gmake test-ci config=release

task:
only_if: $CIRRUS_PR != ''
Expand All @@ -140,18 +140,18 @@ task:
upload_caches:
- libs

release_configure_script:
- make configure arch=x86-64 config=release
release_build_script:
- make build config=release
release_test_script:
- make test-ci config=release
debug_configure_script:
- make configure arch=x86-64 config=debug
debug_build_script:
- make build config=debug
debug_test_script:
- make test-ci config=debug
release_configure_script:
- make configure arch=x86-64 config=release
release_build_script:
- make build config=release
release_test_script:
- make test-ci config=release

task:
only_if: $CIRRUS_PR != ''
Expand All @@ -173,18 +173,18 @@ task:
upload_caches:
- libs

release_configure_script:
- make configure arch=armv8 config=release
release_build_script:
- make build arch=armv8 config=release
release_test_script:
- make test-ci config=release
debug_configure_script:
- make configure arch=armv8 config=debug
debug_build_script:
- make build arch=armv8 config=debug
debug_test_script:
- make test-ci config=debug
release_configure_script:
- make configure arch=armv8 config=release
release_build_script:
- make build arch=armv8 config=release
release_test_script:
- make test-ci config=release

task:
only_if: $CIRRUS_PR != ''
Expand Down
7 changes: 7 additions & 0 deletions .release-notes/4112.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
## Don't include debug information in release versions of ponyc

At some point in the past, we turned on an option PONY_ALWAYS_ASSERT when building ponyc and the runtime. The result of this option was to not only turn on all debug assertions, but also, turn on almost all debug code in the runtime and the ponyc compiler.

The inclusion of assertions was great for error reports from users for compiler bugs. However, it was also including code that would make the compiler slower and use more memory. It was also including code that made the runtime slower for all compiled Pony programs.

We've turned off PONY_ALWAYS_ASSERT. Programs will be somewhat faster, the compiler will be a little faster, and the compiler will use a little less memory. In return, if you report a compiler bug, we'll definitely need a minimal reproduction to have any idea what is causing your bug.
4 changes: 0 additions & 4 deletions src/libponyc/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -98,10 +98,6 @@ add_library(libponyc STATIC
verify/type.c
)

target_compile_definitions(libponyc PRIVATE
PONY_ALWAYS_ASSERT
)

target_include_directories(libponyc
PUBLIC ../common
PRIVATE ../../build/libs/include
Expand Down
1 change: 1 addition & 0 deletions src/libponyc/ast/ast.c
Original file line number Diff line number Diff line change
Expand Up @@ -1612,6 +1612,7 @@ bool ast_is_frozen(ast_t* ast)
#ifndef PONY_NDEBUG
return ast->frozen;
#else
(void)ast;
return false;
#endif
}
Expand Down
3 changes: 2 additions & 1 deletion src/libponyc/ast/token.c
Original file line number Diff line number Diff line change
Expand Up @@ -430,9 +430,10 @@ static void token_docstring_signature_serialise_trace(pony_ctx_t* ctx,
void* object)
{
(void)ctx;
(void)object;

token_t* token = (token_t*)object;

(void)token;
pony_assert(token->id == TK_STRING);
}

Expand Down
1 change: 1 addition & 0 deletions src/libponyc/codegen/gencall.c
Original file line number Diff line number Diff line change
Expand Up @@ -1019,6 +1019,7 @@ static LLVMTypeRef ffi_return_type(compile_t* c, reach_type_t* t,

if(t->underlying == TK_TUPLETYPE)
{
(void)intrinsic;
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i'm not sure how we want to deal with these things that aren't in an #ifdef that exist only for asserts.

this is my "get it to compile solution". any ideas for a better approach?

pony_assert(intrinsic);

// Can't use the named type. Build an unnamed type with the same elements.
Expand Down
1 change: 1 addition & 0 deletions src/libponyc/codegen/genreference.c
Original file line number Diff line number Diff line change
Expand Up @@ -129,6 +129,7 @@ LLVMValueRef gen_fieldembed(compile_t* c, ast_t* ast)
static LLVMValueRef make_tupleelemptr(compile_t* c, LLVMValueRef l_value,
ast_t* l_type, ast_t* right)
{
(void)l_type;
pony_assert(ast_id(l_type) == TK_TUPLETYPE);
int index = (int)ast_int(right)->low;

Expand Down
1 change: 1 addition & 0 deletions src/libponyc/expr/reference.c
Original file line number Diff line number Diff line change
Expand Up @@ -478,6 +478,7 @@ bool expr_dontcareref(pass_opt_t* opt, ast_t* ast)
bool expr_local(pass_opt_t* opt, ast_t* ast)
{
(void)opt;
(void)ast;
pony_assert(ast_type(ast) != NULL);

return true;
Expand Down
1 change: 1 addition & 0 deletions src/libponyc/pass/expr.c
Original file line number Diff line number Diff line change
Expand Up @@ -377,6 +377,7 @@ ast_t* find_antecedent_type(pass_opt_t* opt, ast_t* ast, bool* is_recovered)
case TK_FUN:
{
ast_t* body = ast_childidx(parent, 6);
(void)body;
pony_assert(ast == body);
Comment on lines 379 to 381
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This feels like it should be wrapped in an ifdef so we dont do this extra lookup BUT... eh, does it matter as it is in the compiler and the ifdef would end up being weird.


ast_t* ret_type = ast_childidx(parent, 4);
Expand Down
4 changes: 3 additions & 1 deletion src/libponyc/pass/refer.c
Original file line number Diff line number Diff line change
Expand Up @@ -702,7 +702,9 @@ static bool qualify_typeref(pass_opt_t* opt, ast_t* ast)
ast_setdata(ast, (void*)def);
ast_setid(ast, TK_TYPEREF);

pony_assert(typeref == ast_pop(ast));
ast_t* first_child = ast_pop(ast);
(void)first_child;
pony_assert(typeref == first_child);
ast_t* package = ast_pop(typeref);
ast_t* type_name = ast_pop(typeref);
ast_free(typeref);
Expand Down
1 change: 1 addition & 0 deletions src/libponyc/pkg/program.c
Original file line number Diff line number Diff line change
Expand Up @@ -286,6 +286,7 @@ const char* program_signature(ast_t* program)

blake2b_state hash_state;
int status = blake2b_init(&hash_state, SIGNATURE_LENGTH);
(void)status;
pony_assert(status == 0);

package_group_list_t* iter = data->package_groups;
Expand Down
2 changes: 1 addition & 1 deletion src/libponyrt/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,7 @@ if(PONY_RUNTIME_BITCODE)
#message("${libponyrt_SOURCE_DIR}/${_src_file} -> ${libponyrt_BINARY_DIR}/${_src_file}.bc")
get_filename_component(_src_dir ${_src_file} DIRECTORY)
add_custom_command(
COMMAND mkdir -p "${libponyrt_BINARY_DIR}/${_src_dir}" && ${CMAKE_C_COMPILER} $<IF:$<BOOL:${CMAKE_C_COMPILER_TARGET}>,--target=${CMAKE_C_COMPILER_TARGET},> -DBUILD_COMPILER="${CMAKE_C_COMPILER_VERSION}" -D_FILE_OFFSET_BITS=64 -D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS -D__STDC_LIMIT_MACROS -DLLVM_BUILD_MODE=${PONY_LLVM_BUILD_MODE} -DLLVM_VERSION="${LLVM_VERSION}" -DPONY_ALWAYS_ASSERT -DPONY_COMPILER="${CMAKE_C_COMPILER}" -DPONY_ARCH="${CMAKE_SYSTEM_PROCESSOR}" -DPONY_BUILD_CONFIG="release" -DPONY_USE_BIGINT -DPONY_VERSION="${PONYC_VERSION}" -DPONY_VERSION_STR="${PONYC_VERSION} [release]\\nCompiled with: LLVM ${LLVM_VERSION} -- ${CMAKE_C_COMPILER_ID}-${CMAKE_C_COMPILER_VERSION}-${CMAKE_C_COMPILER_ARCHITECTURE_ID}" -fexceptions -Werror -Wconversion -Wno-sign-conversion -Wno-atomic-alignment -Wextra -Wall ${_c_flags} -I. -I../common -emit-llvm -o "${libponyrt_BINARY_DIR}/${_src_file}.bc" -c ${_src_file}
COMMAND mkdir -p "${libponyrt_BINARY_DIR}/${_src_dir}" && ${CMAKE_C_COMPILER} $<IF:$<BOOL:${CMAKE_C_COMPILER_TARGET}>,--target=${CMAKE_C_COMPILER_TARGET},> -DBUILD_COMPILER="${CMAKE_C_COMPILER_VERSION}" -D_FILE_OFFSET_BITS=64 -D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS -D__STDC_LIMIT_MACROS -DLLVM_BUILD_MODE=${PONY_LLVM_BUILD_MODE} -DLLVM_VERSION="${LLVM_VERSION}" -DPONY_COMPILER="${CMAKE_C_COMPILER}" -DPONY_ARCH="${CMAKE_SYSTEM_PROCESSOR}" -DPONY_BUILD_CONFIG="release" -DPONY_USE_BIGINT -DPONY_VERSION="${PONYC_VERSION}" -DPONY_VERSION_STR="${PONYC_VERSION} [release]\\nCompiled with: LLVM ${LLVM_VERSION} -- ${CMAKE_C_COMPILER_ID}-${CMAKE_C_COMPILER_VERSION}-${CMAKE_C_COMPILER_ARCHITECTURE_ID}" -fexceptions -Werror -Wconversion -Wno-sign-conversion -Wno-atomic-alignment -Wextra -Wall ${_c_flags} -I. -I../common -emit-llvm -o "${libponyrt_BINARY_DIR}/${_src_file}.bc" -c ${_src_file}
WORKING_DIRECTORY ${libponyrt_SOURCE_DIR}
DEPENDS "${libponyrt_SOURCE_DIR}/${_src_file}"
OUTPUT "${libponyrt_BINARY_DIR}/${_src_file}.bc"
Expand Down
4 changes: 0 additions & 4 deletions test/libponyc/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -53,10 +53,6 @@ add_executable(libponyc.tests
verify.cc
)

target_compile_definitions(libponyc.tests PRIVATE
PONY_ALWAYS_ASSERT
)

target_include_directories(libponyc.tests
PRIVATE ../../src/common
PRIVATE ../../src/libponyc
Expand Down