Discussion:
A possible bug
Ivo Doko
2014-10-07 05:00:24 UTC
Permalink
The OS I am using is Windows 7 64-bit. I am using Code::Blocks 13.12
with MinGW-w64 (x86_64-4.9.1-posix-seh-rt_v3-rev1). (I already asked in
mingw-w64-public and was told to ask here instead.)
For this particular instance I've used these compiler options:
-O2 -std=c++11 -Wextra -Wall -march=amdfam10 -pipe -lm -lstdc++

Now that that's out of the way, I have this:
https://sourceforge.net/projects/xorshift-cpp/files/

Compiling the "main.cpp" is fine, unless I remove the "-O2" flag (i.e.
disable compiler optimisation). Then I get the following build log:
http://pastebin.com/LXRX4Pq4

The same thing happens if I remove "-march=amdfam10".

What the linker seems to specifically be complaining about:
undefined reference to `xorshift_engine<unsigned long long, 64ull,
(signed char)-25, (signed char)3, (signed char)49,
8372773778140471301ull>::shift_3'
undefined reference to `xorshift_engine<unsigned long long, 64ull,
(signed char)-25, (signed char)3, (signed char)49,
8372773778140471301ull>::shift_2'

Which makes absolutely no sense, not only because those linker errors
don't come up when compiling even with just "-O", but because, in
"xorshift.h", three constexprs in the xorshift_engine class are defined
like so:

static constexpr int_fast8_t shift_1 = a >= 0 ? a : -a;
static constexpr int_fast8_t shift_2 = b >= 0 ? b : -b;
static constexpr int_fast8_t shift_3 = c >= 0 ? c : -c;

yet shift_1 is ok, but shift_2 and shift_3 somehow aren't...? (There is
no function which does not use all three of these, in order.)

This smells like a bug in the compiler to me, but I am not sure. Can
someone please help me?
--
Ivo Doko
Ivo Doko
2014-10-07 08:36:45 UTC
Permalink
Post by Ivo Doko
The same thing happens if I remove "-march=amdfam10".
Apologies for the spam, but this was worded poorly.

What I meant was that without both "-O" and "-march=amdfam10" the error
still appears. However, if I remove "-march=amdfam10" but still have at
least "-O", there is no linker error.

--
Ivo Doko
Jonathan Wakely
2014-10-07 08:57:37 UTC
Permalink
This smells like a bug in the compiler to me, but I am not sure. Can someone
please help me?
It's probably simply
https://gcc.gnu.org/wiki/VerboseDiagnostics#missing_static_const_definition
Ivo Doko
2014-10-07 09:15:47 UTC
Permalink
Post by Jonathan Wakely
It's probably simply
https://gcc.gnu.org/wiki/VerboseDiagnostics#missing_static_const_definition
Thanks for the reply.

What you propose makes sense. At least, it definitely explains why the
error appears only without compiler optimisation (that was driving me
insane).

However, the only functions which access those static constexpr
variables are member functions of the very class in which those
variables are defined (or actually, there is only one function, because
only one overload of xorshift_engine::xorshiftgen (in file xorshift.h)
will be enabled at compile-time, depending on the class templete
parameters). As far as I can see, the problem should happen only if
static const(expr) variables are accessed outside the class/struct body.

Besides, this doesn't explain why the error happens for shift_2 and
shift_3 but not for shift_1 (keep in mind - function
xorshift_engine::xorshiftgen always accesses each of these three static
constexpr variables, in order).
Jonathan Wakely
2014-10-07 10:45:15 UTC
Permalink
Post by Ivo Doko
As far as I can
see, the problem should happen only if static const(expr) variables are
accessed outside the class/struct body.
No, that's completely irrelevant. I don't know how you would reach
that conclusion form the reference I gave, which says nothing of the
sort.

What matters is whether you use the variable in such a way that the
compiler needs a definition of the variable, rather than just needing
to know its value.
Ivo Doko
2014-10-07 09:28:18 UTC
Permalink
Post by Jonathan Wakely
It's probably simply
https://gcc.gnu.org/wiki/VerboseDiagnostics#missing_static_const_definition
Once again I must apologise for spam, but this helped me solve the
issue. Changing the declaration of function xorshift_engine::shift from

shift(const result_type& val, const int_fast8_t& sh)

to

shift(const result_type& val, int_fast8_t sh)

fixes the linker problem with optimisation disabled.

Still doesn't explain why the error otherwise happens only for two out
of the three variables, but at least I know what I did wrong.

Thank you very much for the help!

--
Ivo Doko
Jonathan Wakely
2014-10-07 10:48:33 UTC
Permalink
Post by Jonathan Wakely
It's probably simply
https://gcc.gnu.org/wiki/VerboseDiagnostics#missing_static_const_definition
Once again I must apologise for spam, but this helped me solve the issue.
Changing the declaration of function xorshift_engine::shift from
shift(const result_type& val, const int_fast8_t& sh)
to
shift(const result_type& val, int_fast8_t sh)
fixes the linker problem with optimisation disabled.
That's almost certainly an improvement anyway, the compiler can pass
an int_fast8_t in a register more efficiently than passing by
reference.

But you haven't actually fixed the root cause of the problem, which is
that you haven't defined your static variables.
Ivo Doko
2014-10-07 11:47:31 UTC
Permalink
Post by Jonathan Wakely
That's almost certainly an improvement anyway, the compiler can pass
an int_fast8_t in a register more efficiently than passing by
reference.
But you haven't actually fixed the root cause of the problem, which is
that you haven't defined your static variables.
Well, where exactly should I "define" them?

Inside the body of the xorshift_engine class I have:

static constexpr size_t word_size =
std::numeric_limits<UIntType>::digits;
static constexpr size_t state_size = n;
static constexpr int_fast8_t shift_1 = a >= 0 ? a : -a;
static constexpr int_fast8_t shift_2 = b >= 0 ? b : -b;
static constexpr int_fast8_t shift_3 = c >= 0 ? c : -c;
static constexpr result_type multiplier = m;
static constexpr result_type default_seed = 5489u;

I don't use these anywhere but inside the bodies of xorshift_engine
member functions.

I fail to see where else I am supposed to define these variables for use
inside the class.
Ivo Doko
2014-10-07 11:48:37 UTC
Permalink
Post by Ivo Doko
static constexpr int_fast8_t shift_1 = a >= 0 ? a : -a;
static constexpr int_fast8_t shift_2 = b >= 0 ? b : -b;
static constexpr int_fast8_t shift_3 = c >= 0 ? c : -c;
Oh, of course, I should have mentioned that a, b and c are template
parameters.

Sorry about that.
Jonathan Wakely
2014-10-07 12:24:55 UTC
Permalink
Post by Ivo Doko
I fail to see where else I am supposed to define these variables for use
inside the class.
Provide an out-of-class definition.

Please read https://gcc.gnu.org/wiki/VerboseDiagnostics#missing_static_const_definition
more carefully, all the way to the end of that section. Also try
reading the http://www.stroustrup.com/bs_faq2.html#in-class page it
links to.
Andrew Haley
2014-10-07 12:25:21 UTC
Permalink
Post by Ivo Doko
I don't use these anywhere but inside the bodies of xorshift_engine
member functions.
I fail to see where else I am supposed to define these variables for use
inside the class.
In xorshift.cpp. But really, we are well outside the scope of gcc-help
now.

Andrew.
Ivo Doko
2014-10-07 13:54:38 UTC
Permalink
Post by Jonathan Wakely
Provide an out-of-class definition.
I've defined other functions (operator>> and operator<<) outside the
class, but others are tiny, easily inlinable functions for which it
seems silly to do so.
Post by Jonathan Wakely
Please read
https://gcc.gnu.org/wiki/VerboseDiagnostics#missing_static_const_definition
Post by Jonathan Wakely
more carefully, all the way to the end of that section. Also try
reading the http://www.stroustrup.com/bs_faq2.html#in-class page it
links to.
I have read both, as well as
https://gcc.gnu.org/onlinedocs/gcc/Static-Definitions.html

The static variables *are* defined (not just declared) - inside the
class body. The issue was that I was passing them by reference to
function xorshift_engine::shift, which has been fixed.

But anyway...
Post by Jonathan Wakely
But really, we are well outside the scope of gcc-help now.
Agreed.

Continue reading on narkive:
Loading...