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

NOZZLE_CLEAN_FEATURE is no longer dependent of HAS_BED_PROBE #4348

Closed
wants to merge 4 commits into from

Conversation

jbrazio
Copy link
Contributor

@jbrazio jbrazio commented Jul 19, 2016

Followup #4054 .
This PR changes NOZZLE_CLEAN_FEATUREso it is no longer dependent of HAS_BED_PROBE.

@thinkyhead
Copy link
Member

Does declaring a function as inline in a header without including the implementation still allow the compiler to make it inline? Obviously not outside of Marlin_main.cpp… but within that file will it still inline the function sometimes, even though it is now exposed to the linker?

@thinkyhead
Copy link
Member

thinkyhead commented Jul 19, 2016

I'm going with this in Marlin.h instead, to save on program space:

/**
 * Blocking movement and shorthand functions
 */
static void do_blocking_move_to(float x, float y, float z, float fr_mm_m=0.0);
static void do_blocking_move_to_axis_pos(AxisEnum axis, float where, float fr_mm_m=0.0);
static void do_blocking_move_to_x(float x, float fr_mm_m=0.0);
static void do_blocking_move_to_y(float y);
static void do_blocking_move_to_z(float z, float fr_mm_m=0.0);
static void do_blocking_move_to_xy(float x, float y, float fr_mm_m=0.0);

@Blue-Marlin
Copy link
Contributor

Blue-Marlin commented Jul 19, 2016

For me this seemed always to be a bit random. My best description would be:
If the 'compiler' is able to inline the function everywhere it does not produce a function we can call from outside. This is later realised by the 'linker' and we get an error.

@thinkyhead
Copy link
Member

The "shorthand" functions, we presume, will be inlined because all they do is pass-through their arguments. A compiler should catch that right away and not generate two function calls.

@jbrazio
Copy link
Contributor Author

jbrazio commented Jul 19, 2016

Does declaring a function as inline in a header without including the implementation still allow the compiler to make it inline

I would think so, otherwise gcc would bark at us for sure for violating some C spec.
-edit- in fact this is one way to do it on classes.

And we must never forget that with -O2 the compiler is far more smarter than us concerning inlining, this means that even if we want to force something to inline is ultimately a compiler decision what to do.

@jbrazio
Copy link
Contributor Author

jbrazio commented Jul 19, 2016

I kept out void do_blocking_move_to(float x, float y, float z, float fr_mm_m=0.0); because it was static and I didn't want to get someone angry at me. :-D

@thinkyhead
Copy link
Member

thinkyhead commented Jul 19, 2016

This isn't really performance-intensive code, so reducing size is more important. Inline functions of course cost more storage space. Compiling with NOZZLE_CLEAN_FEATURE enabled…

  • With simple inline specifiers on all: 99,728
  • Adding FORCE_INLINE to all: 100,020 (+292)
  • Define do_blocking_move_to_axis_pos static: 99,764 (+36)
  • Define all as static in the header file: 99,692 (-36)
  • Declare in header, but define in cpp file: 99,732 (+4)
  • Declare static in header, define in .cpp: 99,474 (-254)

Apparently declaring them static in the header while defining them in the .cpp file is the cheapest option, size-wise. The downside is one or two more call-hops when we use these shorthand functions.

@thinkyhead thinkyhead closed this Jul 19, 2016
@jbrazio jbrazio deleted the rework-g12 branch July 19, 2016 22:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants