RFC: Cleaning up macro naming, possibly switch to inline functions

This is to capture #269 (comment) and following discussion.

Summary:

Macros with THEIR CAPS break continuity in API. Suggested fix: downcase names of macros which are part of mp_obj_* API.
As an extra step, consider leveraging C99 inline functions - both to replace macros and have inlinability guarantees for speed-critical accessors.

CAPS are good because they tell you it's a macro. I would rather make something a function than keep it a macro with lower case letters.

Are C99 inline functions mature/good enough to replace macros? (At least macros that don't use ## etc).

CAPS are good because they tell you it's a macro.

But why anyone would bother to know it's a macro? The only reason, as mentioned in previous discussion, would be "caution, this thing may eval args with side-effects twice", but then macro should be written to be free from this double-eval (and most in uPy are likely already are).

Are C99 inline functions mature/good enough to replace macros?

With GCC, depending on optimization level, you may get inline functions not being inlined, which may be a feature or misfeature. To get 100% inlinability, one must use __attribute__((always_inline)), which then must be abstracted to clean looking define like INLINE or STRONG_INLINE or ALWAYS_INLINE. (And inline functions of course needs to be defined in headers.) So, there're warts, but otherwise, inlines are cool.

Okay, I'm happy to inline the MP_OBJ_IS_* functions.

For MP_OBJ_NEW_SMALL_INT, what if it's one day used to initialise static const data? Eg a static const list of uPy objects, some of which are strs, some ints?

For MP_OBJ_NEW_SMALL_INT, what if it's one day used to initialise static const data?

Good question, worth trying, but as C++11 introduced "constexpr" modifier for that to work, I guess it's not going to work with inline functions. That means we should seriously consider just downcasing, and leave inlines for cool new usages, when such cool new usages arise ;-).

Okay, done some testing with macros vs inline functions. I considered the 3 "functions" MP_OBJ_IS_TYPE, MP_OBJ_IS_INT and MP_OBJ_IS_STR. I tried different ways of compiling them, and got the following binary size for stmhal:

as a true function: 209808 bytes
as a macro: 209824
as a macros w/ single eval of all args: 209900
as an inline static function: 210376
as an inline static function w/ always_inline attribute: 211372 bytes.

So, it's almost 1500 bytes extra to inline, but only 16 bytes extra to make macros!

Pretty weird and something unright there. Dunno what. Switching to plain inlines also broke DEBUG=1 build. Using static inlines fixes that.

Yeah, I don't trust/understand the C compiler with it's use of inline.

The static inline I understand: see eg http://stackoverflow.com/questions/216510/extern-inline/216546#216546

So, it's almost 1500 bytes extra to inline

I mentioned in some other comment that looking at some code for some port I got an impression that compiler may generate per-object instance of (non-inlined) inline function, i.e. the function body may be repeated multiple times in a final executable.

I just got reminded about it while reading "man gcov":

If the function is not inlined, the compiler must emit an out of line copy of the function, in any object file that needs it. If fileA.o and fileB.o both contain out of line bodies of a particular inlineable function, they will also both contain coverage counts for that function. When fileA.o and fileB.o are linked together, the linker will, on many systems, select one of those out of line bodies for all calls to that function, and remove or ignore the other.

So, what I saw is linker not removing other copies. Let this be anecdotal experience for now, but something to check on the next close look at the issue.

Is it possible this has to do with compiler/linker options? Maybe one needs to tell gcc optimize for size or so?

I just tried the same with msvc (replace MP_OBJ_IS_TYPE, MP_OBJ_IS_INT and MP_OBJ_IS_STR with normal inline functions), and the executable size stays exactly the same since the generated assembly is also exactly the same. Which is sort of logical, for such simple functions you'd expect the optimizer can see through the function calls and just replace it with bare instructions.

I use the inline declaration to make what are essentially macros public, without having to expose all macros, etc of the entire module.

I just ran into an issue where I tried to use inline function definitions and it wouldn't compile under the unix port. Are inline functions completely turned off? They definitely have uses!

If I change this line in both the header and C file:

mp_uint_t mem_first() ...

to

inline mp_uint_t mem_first() ...

The compiler fails with this error

CC ../py/gc.c
In file included from ../py/gc.c:53:0:
../py/mem.h:11:25: error: inline function ‘mem_first’ declared but never defined [-Werror]
 inline mp_uint_t        mem_first();
                         ^
../py/mem.h:11:25: error: inline function ‘mem_first’ declared but never defined [-Werror]
cc1: all warnings being treated as errors
make: *** [build/py/gc.o] Error 1

Are inline functions turned off?

@cloudformdesign you need to write static inline ....

no! what I needed was to declare it as inline in the .c file and use the extern keyword in the .h file. That didn't happen with my compiler.

See: http://www.greenend.org.uk/rjk/tech/inline.html

looking at this reference more, I think that both the .c and .h files are supposed to use extern inline. I'll have to experiment with this more.

I tried again to convert macros to inline functions. Using gcc 5.2.0. I can get a decrease of around 500 bytes on 64-bit unix port when using inline functions, and the same size (+/- 10 bytes) for stmhal. Everything except MP_OBJ_IS_TYPE is inlined, since inlining this adds around 500 bytes to stmhal.

Also, we can't inline MP_OBJ_NEW_SMALL_INT and MP_OBJ_NEW_QSTR because they are used to initialise constant data it many places.

So maybe we can downcase everything except those 2 above that must remain macros to initialise static data? And inline everything except MP_OBJ_IS_TYPE (which would become mp_obj_is_type)?

Downcase - sure.

But to inline, relying on novel gcc version doesn't sound like a good idea. Nobody uses 5.2.0 currently and it will be few years till whole world starts to use it. So, if going back to inlining ideas, it would be good to understand what went right in the previous attempt, to be able to control the situation. (And I don't think it's a priority - there're lot of other things to do.)

But to inline, relying on novel gcc version doesn't sound like a good idea.

Yeah, I also don't like how inconsistent it is.

Anyway, main thing is that we have a plan to eventually work towards -- ie downcase and inline.

This is pretty much addressed by #4446, which converted most macros/inline functions to lowercase.

Navigation Menu

RFC: Cleaning up macro naming, possibly switch to inline functions #293

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

RFC: Cleaning up macro naming, possibly switch to inline functions #293

Description

Activity

dpgeorge commented on Feb 15, 2014

pfalcon commented on Feb 15, 2014

dpgeorge commented on Feb 15, 2014

pfalcon commented on Feb 15, 2014

dpgeorge commented on Apr 11, 2014

pfalcon commented on Apr 11, 2014

dpgeorge commented on Apr 11, 2014

pfalcon commented on Mar 13, 2015

stinos commented on Mar 26, 2015

vitiral commented on May 6, 2015

dpgeorge commented on May 6, 2015

vitiral commented on May 6, 2015

dpgeorge commented on Oct 9, 2015

pfalcon commented on Oct 11, 2015

dpgeorge commented on Oct 12, 2015

dpgeorge commented on Feb 12, 2019

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions