On the use of `constexpr` constants in header files

Technical Feedback
1

[Edit] There’s more nuance here than first meets the eye when it comes to determining if the data is actually duplicated in the final binary. Please see the comments for further discussion on the matter. Large constant tables defined in headers can probably still use inline constexpr rather than static const or static constexpr but single-value variables should probably not use either inline or static; just constexpr is fine.

As we move more code to modern C++, constexpr constants are becoming more prevelant as we replace their old #define counterparts.

However, there’s a bit of ceremony in how to define them “correctly”. constexpr types have storage, they take up space in the binary, you can take their address. This means defining them in headers brings traditional header problems as illustrated below.

TL;DR: use inline constexpr when defining constant variables, both single values and arrays, in header files.

Below is an example application showing the differences between various ways of declaring the constant variables. It contains a header.hh header declaring the constants and 3 implementation files using that header. Each implementation will print what they believe the address of the variables to be.

Notice that only the inline constexpr variant results in the same address in the binary across all translation units. The other forms lead to duplication and potential ODR violations (albeit harmless in this case).

Compiler Explorer link: Compiler Explorer
Example output:

main.cpp - var_constexpr               0x402270
main.cpp - var_static_constexpr        0x40226c
main.cpp - var_inline_static_constexpr 0x402268
main.cpp - var_inline_constexpr        0x402244
----
impl_1.cpp - var_constexpr               0x402250
impl_1.cpp - var_static_constexpr        0x40224c
impl_1.cpp - var_inline_static_constexpr 0x402248
impl_1.cpp - var_inline_constexpr        0x402244
----
impl_2.cpp - var_constexpr               0x40225c
impl_2.cpp - var_static_constexpr        0x402258
impl_2.cpp - var_inline_static_constexpr 0x402254
impl_2.cpp - var_inline_constexpr        0x402244
----
1 Like
2

Why not a static constexpr? I mean usually to you don’t takes and address to a compile-time objects, right?

3

The point is to use the correct source to begin with, to guarantee no duplication in the final binary. It is semantically more correct. Printing the address above just proves that duplication has occurred. There is no reason to use static here - it does nothing.

If your linker is good enough maybe it will throw out the duplicate data if it finds that no address has been taken. Maybe not.

Using an array, though, is nothing more than (address + offset) so you do use the address in many cases like if you have a table of values that you made constexpr for example.

4

I mean that constexpr varible is not expected to be used as something taken by address. I mean, constexpr float PI = 3.14f; in a header is to be inlined in a code and used in constexpr functions and other expressions, like format. In case you’ll take address of such constant this would mean you did something wrong since this is not a thing that has benefit to be inlined.
Where and why you created constexpr array?

Not checked, but for me additional reason to reduce non-static linkage of things in headers is about faster linkage after compiling\

5

Thanks for writing this up! The meaning of inline feels so backwards to me. I’ll keep this in mind though, I just added a constexpr variable in a header today so I’ll start by fixing that.

6

Sounds correct and indeed probably needs to be fixed in some places. Just wanted to mention one more related detail that I ran into a couple of times in the past: When defining static data in a class/struct inline is already implied by constexpr.

I updated your code to show that: Compiler Explorer.

A constexpr specifier used in a function or static data member(since C++17) declaration implies inline (Source).

7

I looked through the uses of constexpr in headers and hopefully found all the fixes: #131433 - Cleanup: Use "inline constexpr" in headers outside of classes - blender - Blender Projects

This seems to actually increase binary size though (using GCC and mold here):

Before: 224643336
After:  224643448
8

static constexpr int NODE_DEFAULT_MAX_WIDTH = 700; - thing that can be inlined where is used (inlined in mean like func(NODE_DEFAULT_MAX_WIDTH) → func(700)), but instead its marked as something with external linkage so each translation unit now has its own non-inlined (in mean as actuall variable) version of NODE_DEFAULT_MAX_WIDTH so linker have to spent time and eliminate all this copies/do link non-inlined usage by pointers to NODE_DEFAULT_MAX_WIDTH.

9

A bit unexpected. Though the eevee_shadow.hh changes looks like you left static in the declarations for those tables (and in a few other cases too?). Static needs to be removed for those.

10

You’re right, my bad. But uh oh, now it’s even worse, the change makes Blender 312 bytes bigger!

Before: 224760584
After:  224760896
11

Alright this is no good then. I don’t know what the cause of this might be.

Locally, when I convert a decently sized constexpr static or static const array to an inline constexpr array, in a 3 file example similar to the example above, it yields a good results:

-rwxr-xr-x 1 deadpin deadpin 24504 Dec  5 19:05 ./a.out
vs.
-rwxr-xr-x 1 deadpin deadpin 20304 Dec  5 19:07 ./a.out

I was hoping to keep the “rule” simple, just use inline constexpr everywhere, but maybe the main benefit is for arrays rather than simple constants. How about I amend the top discussion noting that there seems to be more nuance here and then not worry about your PR for now? Operate under status quo until someone roots out the ultimate issue here (if there even is one).

I primarily decided to bring this up only due to ff_compat_crop_tab table which might benefit from the inline treatment. The SizeBench app on windows notes that it gets duplicated for 9k wasted space (our largest offender).