espressif / crosstool-NG

crosstool-NG with support for Xtensa
Other
121 stars 63 forks source link

C++: In std=gnu++2b size-optimized builds std::string methods are inlined #67

Open rojer opened 4 days ago

rojer commented 4 days ago

Checklist

How often does this bug occurs?

always

Expected behavior

I expected frequently used functions such as std::string::append not to be inlined, at least not in -Os mode.

Actual behavior (suspected bug)

Instead, when switching from gnu++17 to gnu++2b standard std::string functions such as append, resize, instead appear to be inlined. This alone is responsible for significant binary size increase for our application.

Error logs or terminal output

No response

Steps to reproduce the behavior

test file: https://gist.github.com/rojer/81900a923fdfa7e6853afe72fa2ea87b

to change the standard: edit tools/cmake/build.cmake, change the cxx_std variable.

it's obvious that in c++17 mode std::string methods are invoked while in c++2b mode they are inlined.

Project release version

14.2.0_20240906

System architecture

other (details in Additional context)

Operating system

Linux

Operating system version

Ubuntu 24.04

Shell

Bash

Additional context

IDF 5.2.2

Lapshin commented 2 days ago

Hi @rojer, I appreciate your efforts in size optimization!

I took a look at your examples and that what I found - gnu++2b compared to gnu++17 has constexpr specifier for the most methods in stdlib. See macro _GLIBCXX20_CONSTEXPR

constexpr can explicitly inline functions and the issue appeared when the method called once in translation unit, otherwise gcc is smart enough to not inline the method and make a call... But yes, it's still an issue if you have lot of such cases. The first thing comes to my head is to redefine _GLIBCXX20_CONSTEXPR.

And your example could be built with xtensa-esp32-elf-c++ -Os test.cpp -std=gnu++2b -D_GLIBCXX20_CONSTEXPR="__attribute__((noinline))" -D__cpp_lib_constexpr_string="" -D__cpp_lib_string_resize_and_overwrite="". But I'm not sure that this approach is good to use...

rojer commented 2 days ago

ok, it's clear now and adheres to the C++20 standard but the tradeoff is definitely wrong for us - being able to construct std::strings in constexpr functions is not worth the bloat, at all.

i tried adding these flags to an IDF project build but the compiler got very angry at me :) probably some issue with quoting or something... can you help me apply this to an IDF project, just to see what effect it would have?

also, would you consider patching libstdc++ to add a proper way to override this during build?

something like

#ifndef GLIBCXX_STRING_CONSTEXPR
#define GLIBCXX_STRING_CONSTEXPR _GLIBCXX20_CONSTEXPR
#endif

and then use GLIBCXX_STRING_CONSTEXPR instead of _GLIBCXX20_CONSTEXPR directly. seems like a pretty straightforward change.

we'd then be able to add -DGLIBCXX_STRING_CONSTEXPR= to get back to C++17 behavior.

Lapshin commented 2 days ago

@rojer , to apply flags to your projects please update your main/CMakeFiles.txt file with the following lines (insert after the line idf_component_register):

idf_build_set_property(CXX_COMPILE_OPTIONS "-D_GLIBCXX20_CONSTEXPR=__attribute__((noinline))" APPEND)
idf_build_set_property(CXX_COMPILE_OPTIONS "-D__cpp_lib_constexpr_string=" APPEND)
idf_build_set_property(CXX_COMPILE_OPTIONS "-D__cpp_lib_string_resize_and_overwrite=" APPEND)

I'm going to prepare a patch and open a ticket on GCC bugzilla to discuss the changes

Lapshin commented 2 days ago

@rojer , I'm also interested to know if you've observed binary size increases only for string methods. It seems to be a problem for most std functions

rojer commented 1 day ago

you are most likely right, std::string was just the first thing i found and reported.

ok, i tried with the settings you suggested but surprisingly the size of binary went up, and by quite a lot (gcc14_cxx2b_opt):

$ ls -la build_Pro1.*/objs/Pro1.bin
-rw-r--r-- 1 rojer rojer 1867712 Nov 13 14:25 build_Pro1.gcc13_cxx17/objs/Pro1.bin
-rw-r--r-- 1 rojer rojer 1848880 Nov 13 14:23 build_Pro1.gcc14_cxx17/objs/Pro1.bin
-rw-r--r-- 1 rojer rojer 1864384 Nov 13 14:18 build_Pro1.gcc14_cxx2b/objs/Pro1.bin
-rw-r--r-- 1 rojer rojer 2004832 Nov 13 14:16 build_Pro1.gcc14_cxx2b_opt/objs/Pro1.bin
Lapshin commented 1 day ago

@rojer , that's interesting. What result will be if you change __attribute__((noinline)) to __attribute__((cold)) ?

rojer commented 1 day ago

much better:

$ ll build_Pro1.*/objs/Pro1.bin
-rw-r--r-- 1 rojer rojer 1867712 Nov 13 14:25 build_Pro1.gcc13_cxx17/objs/Pro1.bin
-rw-r--r-- 1 rojer rojer 1848880 Nov 13 14:23 build_Pro1.gcc14_cxx17/objs/Pro1.bin
-rw-r--r-- 1 rojer rojer 1864384 Nov 13 14:18 build_Pro1.gcc14_cxx2b/objs/Pro1.bin
-rw-r--r-- 1 rojer rojer 1848720 Nov 13 19:00 build_Pro1.gcc14_cxx2b_opt_cold/objs/Pro1.bin
-rw-r--r-- 1 rojer rojer 2004832 Nov 13 14:16 build_Pro1.gcc14_cxx2b_opt/objs/Pro1.bin

even better than cxx17

Lapshin commented 1 day ago

@rojer , thank you for sharing the results of the real application.

Could you please check one more option, change the strings:

-idf_build_set_property(CXX_COMPILE_OPTIONS "-D_GLIBCXX20_CONSTEXPR=__attribute__((cold))" APPEND)
-idf_build_set_property(CXX_COMPILE_OPTIONS "-D__cpp_lib_constexpr_string=" APPEND)
-idf_build_set_property(CXX_COMPILE_OPTIONS "-D__cpp_lib_string_resize_and_overwrite=" APPEND)
+idf_build_set_property(CXX_COMPILE_OPTIONS "-D_GLIBCXX20_CONSTEXPR=constexpr __attribute__((cold))" APPEND)
rojer commented 22 hours ago

first, apologies: previous test was done incorrect, std was actually set to 17, so i renamed it to gcc14_cxx17_opt_cold. i re-did the test and cold result is a bit better than default but worse than 17. constexpr+cold (gcc14_cxx2b_opt_constexpr_cold) is worse.

$ ll build_Pro1*/objs/Pro1.bin
-rw-r--r-- 1 rojer rojer 1867712 Nov 13 14:25 build_Pro1.gcc13_cxx17/objs/Pro1.bin
-rw-r--r-- 1 rojer rojer 1848880 Nov 13 14:23 build_Pro1.gcc14_cxx17/objs/Pro1.bin
-rw-r--r-- 1 rojer rojer 1848720 Nov 13 19:00 build_Pro1.gcc14_cxx17_opt_cold/objs/Pro1.bin
-rw-r--r-- 1 rojer rojer 1864384 Nov 13 14:18 build_Pro1.gcc14_cxx2b/objs/Pro1.bin
-rw-r--r-- 1 rojer rojer 1858336 Nov 14 12:47 build_Pro1.gcc14_cxx2b_opt_cold/objs/Pro1.bin
-rw-r--r-- 1 rojer rojer 1863200 Nov 14 12:52 build_Pro1.gcc14_cxx2b_opt_constexpr_cold/objs/Pro1.bin
-rw-r--r-- 1 rojer rojer 2004832 Nov 13 14:16 build_Pro1.gcc14_cxx2b_opt/objs/Pro1.bin