10000 Automatic stack location selection (SYS or HEAP), enable per library AR-chive in arduino build system by d-a-v · Pull Request #5018 · esp8266/Arduino · GitHub
[go: up one dir, main page]
Skip to content

Automatic stack location selection (SYS or HEAP), enable per library AR-chive in arduino build system #5018

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.

Sign up for GitHub

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

Merged
merged 20 commits into from
Aug 20, 2018
Merged

Automatic stack location selection (SYS or HEAP), enable per library AR-chive in arduino build system #5018

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
1f88f28
Automatic stack location selection (SYS or HEAP), remove unnecessary …
d-a-v Aug 8, 2018
60d3b4e
documentation updates
d-a-v Aug 8, 2018
05b2aee
dot_a_linkage=true
d-a-v Aug 9, 2018
6ed54ee
WIP: fixes from @igrr's review
d-a-v Aug 9, 2018
6f7286e
(WIP) one more pass
d-a-v Aug 9, 2018
12f412d
clean (wip'ed) declaration
d-a-v Aug 9, 2018
3259bbc
removed example, added device tests
d-a-v Aug 9, 2018
446220b
minimize commits footprint
d-a-v Aug 9, 2018
33db9a2
Merge branch 'master' into autowps
d-a-v Aug 9, 2018
c64f994
boards gen: deprecated option
d-a-v Aug 9, 2018
cdba9b4
some comment updates and fixes from @devyte's review
d-a-v Aug 10, 2018
d38afda
Merge branch 'master' into autowps
devyte Aug 13, 2018
67405c1
updates from second @igrr's review
d-a-v Aug 16, 2018
7c9d897
Merge branch 'autowps' of github.com:d-a-v/Arduino into autowps
d-a-v Aug 16, 2018
8ec3504
updates from second @igrr's review
d-a-v Aug 16, 2018
77b1b2d
Merge branch 'master' into autowps
d-a-v Aug 16, 2018
28c9c22
Merge branch 'master' into autowps
d-a-v Aug 16, 2018
eb18c5b
remove duplicate -L option
d-a-v Aug 16, 2018
d08728a
Merge branch 'master' into autowps
devyte Aug 19, 2018
8487455
Merge branch 'master' into autowps
igrr Aug 20, 2018
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
Prev Previous commit
Next Next commit
documentation updates
  • Loading branch information
@d-a-v
d-a-v committed Aug 8, 2018
commit 60d3b4eff9723dc96e229c9ea18524aa7dd9d7fb
1 change: 0 additions & 1 deletion cores/esp8266/coredecls.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ extern "C" {

// TODO: put declarations here, get rid of -Wno-implicit-function-declaration


extern bool timeshift64_is_set;

void esp_yield();
Expand Down
2 changes: 0 additions & 2 deletions doc/faq/a05-board-generator.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,8 +48,6 @@ As of today you can:

* increase available flash space by disabling floats in ``*printf`` functions

* enable WPS which is now disabled by default (at the cost of a smaller heap by ~4KB)

* change led pin ``LED_BUILTIN`` for the two generic boards

* change the default lwIP version (1.4 or 2)
Expand Down
16 changes: 10 additions & 6 deletions doc/faq/readme.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,22 +46,26 @@ How can I get some extra KBs in flash ?
* Using ``*printf()`` with floats is enabled by default. Some KBs of flash can
be saved by using the option ``--nofloat`` with the boards generator:

``./tools/boards.txt.py --nofloat --allgen``
``./tools/boards.txt.py --nofloat --boardsgen``

* Use the debug level option ``NoAssert-NDEBUG`` (in the Tools menu)

`Read more <a05-board-generator.rst>`__.

Why can't I use WPS ?
~~~~~~~~~~~~~~~~~~~~~
About WPS
~~~~~~~~~

WPS is disabled by default, this offers an extra 4KB in ram/heap. To enable
WPS (and lose 4KB of useable ram), use this boards generator option:
In release 2.4.2 only, WPS is disabled by default. To enable WPS, use this
boards generator option:

``./tools/boards.txt.py --allowWPS --allgen``
``./tools/boards.txt.py --allowWPS --boardsgen``

`Read more <a05-board-generator.rst>`__.

From release 2.4.2 and ahead, using WPS will reduce available heap space to
user by around 4.5KB. In other words, from release 2.4.2 without using WPS,
Copy link
Member

Choose a reason for hiding this comment

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

Suggest rewording this to make it clear that this is not yet available in 2.4.2. E.g. "starting from 2.5.0, ...".

an extra ~4.5KB is available in user's heap space.

This Arduino library doesn't work on ESP. How do I make it work?
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

Expand Down
2 changes: 1 addition & 1 deletion libraries/ESP8266WiFi/src/ESP8266WiFiSTA.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ class ESP8266WiFiSTAClass {
disable_extra4k_at_link_time(); // this call must always be inlined
Copy link
Member

Choose a reason for hiding this comment

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

as far as i can tell this can never be inlined, unless we enable LTO?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I have updated the code/comment (yet to push) as follow:

    protected:
    
        static void WPSStatusCB(wps_cb_status status);
        bool beginRealWPSConfig(void);

    public:

        inline bool beginWPSConfig(void) __attribute__((always_inline))
        {
            /*
             Some notes:

             This call to disable_extra4k_at_link_time() must not be
             "compiled-in" when WPS is not in use.  Calling it from
             beginRealWPSConfig() would always link it.

             'inline ...  attribute(always_inline)' is not required since gcc
             will optimize this function, but it is left as a reminder.

             Moving beginRealWPSConfig from ESP8266WiFiSTA.cpp to
             ESP8266WiFiSTA-WPS.cpp only allows to same some flash space.
            */
            disable_extra4k_at_link_time();

            return beginRealWPSConfig();
        }

When it is not inlined, or not optimized by gcc, or if disable_extra4k_at_link_time() is called from beginRealWPSConfig(), heap stack is always linked in.

Copy link
Member

Choose a reason for hiding this comment

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

Okay, i finally understood why you were mentioning inlining in other comments and why you needed to make it inline. This is because Arduino IDE does not combine object files from libraries into static archives, it links them directly. This is in contrast with the core object files, which are first placed into an archive.

However, recent Arduino versions support putting library object files into .a archive before linking, which should remove the need for inlining. Please check https://github.com/arduino/Arduino/wiki/Arduino-IDE-1.5:-Library-specification and add dot_a_linkage=true to ESP8266WiFi library.properties file.

Copy link
Collaborator Author
@d-a-v d-a-v Aug 9, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

It works with dot_a_linkage=true and the need for an inline function/method in the class is now useless.

Things are now more clear, thanks for this tip. I was not understanding in the first place why I had to use the inline trick and was wrongly thinking that objects files were stored in a library. I admit I don't often try to read the long-like-hell IDE commands.

But I had to cheat and create an empty archive {tmp}/libraries/ESP8266WiFi/ESP8266WiFi.a at the linker command request, while the build process put everything in {tmp}/arduino.ar.
It seems to be the same bug as this esp32 one. This issue is still present in both latest nightly and latest beta.

edit: now trying to find the arduino environment variable for the AR object
edit2: found it, it's {archive_file_path}. Now arduino.ar no longer exists, digging this out :)
edit3: found this, {...}/arduino.ar needs to be replaced by "{archive_file_path}" "-L{build.path}" (all doc is in your above link)

return ::beginWPSConfig();
}

bool beginSmartConfig();
bool stopSmartConfig();
bool smartConfigDone();
Expand Down
0