Reference over temporaries

[jhjourdan]

I have a problem with the way the track and dxxxx functions take their
arguments. Let's take the following example (it uses the same idioms as
CppBugds examples):

MCModelboost::minstd_rand m(model);
m.track(lambda).dgamma(0.1, 0.1);

The dgamma function takes its arguments by reference and do not store a
copy of its arguments. Moreover, 0.1 is stored in a temporary before being
passed to dgamma. Thus, the compiler is allowed to deallocate the memory
used for this temporary just after the call, and use it for something
else. However, this value will be used afterward during the sampling. So,
we have a problem, because the value used during the sampling can be
different.

For some reason, it seems like this problem actually happens only for gcc

= 4.7, with optimisations enabled. That is why the problem has never been
discovered before.

I have different ideas for solving this problem. None of them are
perfect. Please give me your thoughts about them:

1- Ask the user code not to pass temporaries to these functions. This makes
the user code heavier, but there is not change of the library code.

The client code would be:

double zeropointone = 0.1;
m.track(lambda).dgamma(zeropointone, zeropointone);

2- For each of these parameters, give two overloads of the functions. One
will take a pointer to the value, and the other will take the value (not a
reference to it !). The second one first stores the value in a safe place
before calling the first version.

The client code would be:

double zeropointone = 0.1;
m.track(&lambda).dgamma(0.1, 0.1);

Pros: flexibility for the user
Cons: no backward compatibility, somewhat less clean user code (and the
user has not to forget to put the &).

3- Idem as 2-, but with const references in place of pointers. That is, the
copy will take place only when the given expression is either an rvalue or
a const variable. The typical client code would not change:

m.track(lambda).dgamma(0.1, 0.1);

Pro: backward compatibility, basic user code more intuitive.
Cons: even if it seems to be a good criterion for most cases in practice,
there is no good reason for using constness for this purpose. In
particular, the non-copying versions of the dxxx functions will have
non-const parameter but const behaviour.

3bis- Idem as 3, but using rvalue references in place of const references

Pro: backward compatibility, basic user code more intuitive
Cons: using rvalue-references which are a new feature of C++11 that nobody
really understand in details...

Thanks !

Jacques-Henri Jourdan

[armstrtw]

I've seen this w/ gcc 4.7. I had assumed it was a bug in gcc.

Are you sure that gcc w/ optimizations on is really conforming to the
c++ standard?

The idea of using const refs in all of the parameters is so that
future evaluations of the likelihood will pick up changes in the
parameters. Obviously, that is not relevant for the cases in which
the parameters are const values, but the underlying code is the same
in all cases, so it has to be general.

copy of its arguments. Moreover, 0.1 is stored in a temporary before being
passed to dgamma. Thus, the compiler is allowed to deallocate the memory

how can you confirm this? const values are stored in a separate area
of the program. So, all refs to a given const value should actually
point to the same memory address. I've confirmed this behaviour with
strings before but not w/ ints/doubles. It should be easy to test.

-Whit

[armstrtw]

also, which exact gcc version are you on? and this is on Fedora as well?

-Whit

On Sun, Jul 8, 2012 at 2:21 PM, jhjourdan
reply@reply.github.com
wrote:

I have a problem with the way the track and dxxxx functions take their
arguments. Let's take the following example (it uses the same idioms as
CppBugds examples):

MCModelboost::minstd_rand m(model);
m.track(lambda).dgamma(0.1, 0.1);

The dgamma function takes its arguments by reference and do not store a
copy of its arguments. Moreover, 0.1 is stored in a temporary before being
passed to dgamma. Thus, the compiler is allowed to deallocate the memory
used for this temporary just after the call, and use it for something
else. However, this value will be used afterward during the sampling. So,
we have a problem, because the value used during the sampling can be
different.

For some reason, it seems like this problem actually happens only for gcc

= 4.7, with optimisations enabled. That is why the problem has never been
discovered before.

I have different ideas for solving this problem. None of them are
perfect. Please give me your thoughts about them:

1- Ask the user code not to pass temporaries to these functions. This makes
the user code heavier, but there is not change of the library code.

The client code would be:

double zeropointone = 0.1;
m.track(lambda).dgamma(zeropointone, zeropointone);

2- For each of these parameters, give two overloads of the functions. One
will take a pointer to the value, and the other will take the value (not a
reference to it !). The second one first stores the value in a safe place
before calling the first version.

The client code would be:

double zeropointone = 0.1;
m.track(&lambda).dgamma(0.1, 0.1);

Pros: flexibility for the user
Cons: no backward compatibility, somewhat less clean user code (and the
user has not to forget to put the &).

3- Idem as 2-, but with const references in place of pointers. That is, the
copy will take place only when the given expression is either an rvalue or
a const variable. The typical client code would not change:

m.track(lambda).dgamma(0.1, 0.1);

Pro: backward compatibility, basic user code more intuitive.
Cons: even if it seems to be a good criterion for most cases in practice,
there is no good reason for using constness for this purpose. In
particular, the non-copying versions of the dxxx functions will have
non-const parameter but const behaviour.

3bis- Idem as 3, but using rvalue references in place of const references

Pro: backward compatibility, basic user code more intuitive
Cons: using rvalue-references which are a new feature of C++11 that nobody
really understand in details...

Thanks !

Jacques-Henri Jourdan

---

Reply to this email directly or view it on GitHub:
#10

[jhjourdan]

Le 08/07/2012 20:38, Whit Armstrong a écrit :

I've seen this w/ gcc 4.7. I had assumed it was a bug in gcc.

Are you sure that gcc w/ optimizations on is really conforming to the
c++ standard?

There are bugs, but that is what they intend to, yes. At least with -O2,
and the bad behavior exists with this option.

how can you confirm this? const values are stored in a separate area
of the program. So, all refs to a given const value should actually
point to the same memory address. I've confirmed this behaviour with
strings before but not w/ ints/doubles. It should be easy to test.

Actually, this is more complicated than that : the compiler can actually
chose between using a separate memory space for storing value, or
loading constants directly by using immediate values. In my assembly
code for .dgamma(0.1, 0.1), I can find :

movabsq $4591870180066957722, %r10 ; 4591870180066957722 is
; actually the qword bit-equivalent
; to the double 0.1
movabsq $4591870180066957722, %r11 ; the other 0.1
movq %r10, -6440(%rbp) ; loading this value to a temporary
; location in the stack
movq %r11, -6448(%rbp) ; idem

Where 4591870180066957722 is actually the qword bit-equivalent to the
double 0.1.

A few lines later, there is :

movq %r9, -6440(%rbp)

That is, the compiler is using the stack location -6440(%rbp) for
storing something else.

So yes, I can confirm this behavior. And I would be /very/ surprised if
gcc does never do anything similar with ints. For string literals, that
is different : a string literal has the type const char*, and is
pointing to some place that contains the right data. But the literal
actually represent a pointer, whereas int/double literals represent the
actual value.

Jacques-Henri Jourdan

-Whit

---

Reply to this email directly or view it on GitHub:
#10 (comment)

[jhjourdan]

It is gcc 4.7.0, the latest version on Fedora 17.

Jacques-Henri Jourdan

Le 08/07/2012 21:01, Whit Armstrong a écrit :

also, which exact gcc version are you on? and this is on Fedora as well?

-Whit

On Sun, Jul 8, 2012 at 2:21 PM, jhjourdan
reply@reply.github.com
wrote:

I have a problem with the way the track and dxxxx functions take their
arguments. Let's take the following example (it uses the same idioms as
CppBugds examples):

MCModelboost::minstd_rand m(model);
m.track(lambda).dgamma(0.1, 0.1);

The dgamma function takes its arguments by reference and do not store a
copy of its arguments. Moreover, 0.1 is stored in a temporary before being
passed to dgamma. Thus, the compiler is allowed to deallocate the memory
used for this temporary just after the call, and use it for something
else. However, this value will be used afterward during the sampling. So,
we have a problem, because the value used during the sampling can be
different.

For some reason, it seems like this problem actually happens only for gcc

= 4.7, with optimisations enabled. That is why the problem has never been
discovered before.

I have different ideas for solving this problem. None of them are
perfect. Please give me your thoughts about them:

1- Ask the user code not to pass temporaries to these functions. This makes
the user code heavier, but there is not change of the library code.

The client code would be:

double zeropointone = 0.1;
m.track(lambda).dgamma(zeropointone, zeropointone);

2- For each of these parameters, give two overloads of the functions. One
will take a pointer to the value, and the other will take the value (not a
reference to it !). The second one first stores the value in a safe place
before calling the first version.

The client code would be:

double zeropointone = 0.1;
m.track(&lambda).dgamma(0.1, 0.1);

Pros: flexibility for the user
Cons: no backward compatibility, somewhat less clean user code (and the
user has not to forget to put the &).

3- Idem as 2-, but with const references in place of pointers. That is, the
copy will take place only when the given expression is either an rvalue or
a const variable. The typical client code would not change:

m.track(lambda).dgamma(0.1, 0.1);

Pro: backward compatibility, basic user code more intuitive.
Cons: even if it seems to be a good criterion for most cases in practice,
there is no good reason for using constness for this purpose. In
particular, the non-copying versions of the dxxx functions will have
non-const parameter but const behaviour.

3bis- Idem as 3, but using rvalue references in place of const references

Pro: backward compatibility, basic user code more intuitive
Cons: using rvalue-references which are a new feature of C++11 that nobody
really understand in details...

Thanks !

Jacques-Henri Jourdan

---

Reply to this email directly or view it on GitHub:
#10

---

Reply to this email directly or view it on GitHub:
#10 (comment)

[armstrtw]

Thanks for digging into this. can you send or post your test code?

-Whit

On Sun, Jul 8, 2012 at 5:53 PM, jhjourdan
reply@reply.github.com
wrote:

Le 08/07/2012 20:38, Whit Armstrong a écrit :

I've seen this w/ gcc 4.7. I had assumed it was a bug in gcc.

Are you sure that gcc w/ optimizations on is really conforming to the
c++ standard?

There are bugs, but that is what they intend to, yes. At least with -O2,
and the bad behavior exists with this option.

how can you confirm this? const values are stored in a separate area
of the program. So, all refs to a given const value should actually
point to the same memory address. I've confirmed this behaviour with
strings before but not w/ ints/doubles. It should be easy to test.

Actually, this is more complicated than that : the compiler can actually
chose between using a separate memory space for storing value, or
loading constants directly by using immediate values. In my assembly
code for .dgamma(0.1, 0.1), I can find :

movabsq $4591870180066957722, %r10 ; 4591870180066957722 is
; actually the qword bit-equivalent
; to the double 0.1
movabsq $4591870180066957722, %r11 ; the other 0.1
movq %r10, -6440(%rbp) ; loading this value to a temporary
; location in the stack
movq %r11, -6448(%rbp) ; idem

Where 4591870180066957722 is actually the qword bit-equivalent to the
double 0.1.

A few lines later, there is :

movq %r9, -6440(%rbp)

That is, the compiler is using the stack location -6440(%rbp) for
storing something else.

So yes, I can confirm this behavior. And I would be /very/ surprised if
gcc does never do anything similar with ints. For string literals, that
is different : a string literal has the type const char*, and is
pointing to some place that contains the right data. But the literal
actually represent a pointer, whereas int/double literals represent the
actual value.

Jacques-Henri Jourdan

-Whit

---

Reply to this email directly or view it on GitHub:
#10 (comment)

---

Reply to this email directly or view it on GitHub:
#10 (comment)

[armstrtw]

The more I look at this the worse it seems.

There is a snip here, which basically describes how gcc did this to be
more efficient w/ stack usage:
http://gcc.gnu.org/gcc-4.7/changes.html

"G++ now properly re-uses stack space allocated for temporary objects
when their lifetime ends, which can significantly lower stack
consumption for some C++ functions. As a result of this, some code
with undefined behavior will now break"

I'll take a closer look at your proposals. I agree that rvalues my be
a bit much for now.

My main gripe is that there is no way to detect when a literal is
passed (there is no way to detect the condition). So, the current
version of the code will silently break whenever one passes in
literals.

-Whit

On Mon, Jul 9, 2012 at 7:07 AM, Whit Armstrong armstrong.whit@gmail.com wrote:

Thanks for digging into this. can you send or post your test code?

-Whit

On Sun, Jul 8, 2012 at 5:53 PM, jhjourdan
reply@reply.github.com
wrote:

Le 08/07/2012 20:38, Whit Armstrong a écrit :

I've seen this w/ gcc 4.7. I had assumed it was a bug in gcc.

Are you sure that gcc w/ optimizations on is really conforming to the
c++ standard?

There are bugs, but that is what they intend to, yes. At least with -O2,
and the bad behavior exists with this option.

how can you confirm this? const values are stored in a separate area
of the program. So, all refs to a given const value should actually
point to the same memory address. I've confirmed this behaviour with
strings before but not w/ ints/doubles. It should be easy to test.

Actually, this is more complicated than that : the compiler can actually
chose between using a separate memory space for storing value, or
loading constants directly by using immediate values. In my assembly
code for .dgamma(0.1, 0.1), I can find :

movabsq $4591870180066957722, %r10 ; 4591870180066957722 is
; actually the qword bit-equivalent
; to the double 0.1
movabsq $4591870180066957722, %r11 ; the other 0.1
movq %r10, -6440(%rbp) ; loading this value to a temporary
; location in the stack
movq %r11, -6448(%rbp) ; idem

Where 4591870180066957722 is actually the qword bit-equivalent to the
double 0.1.

A few lines later, there is :

movq %r9, -6440(%rbp)

That is, the compiler is using the stack location -6440(%rbp) for
storing something else.

So yes, I can confirm this behavior. And I would be /very/ surprised if
gcc does never do anything similar with ints. For string literals, that
is different : a string literal has the type const char*, and is
pointing to some place that contains the right data. But the literal
actually represent a pointer, whereas int/double literals represent the
actual value.

Jacques-Henri Jourdan

-Whit

---

Reply to this email directly or view it on GitHub:
#10 (comment)

---

Reply to this email directly or view it on GitHub:
#10 (comment)

[jhjourdan]

Do you still want me to provide you an example ? My actual code is quite
big and uses some external libraries, so I would need to do some work in
order to extract a smaller failing example, so...

Le 09/07/2012 20:44, Whit Armstrong a écrit :

My main gripe is that there is no way to detect when a literal is
passed (there is no way to detect the condition).

Well, actually, rvalue references can do this detection (in our case, we
can assimilate rvalues to literals).

Anyway, thanks for thinking about this problem !

If you want, I can implement one of these solution.

Jacques-Henri Jourdan

-Whit

On Mon, Jul 9, 2012 at 7:07 AM, Whit Armstrong armstrong.whit@gmail.com wrote:

Thanks for digging into this. can you send or post your test code?

-Whit

On Sun, Jul 8, 2012 at 5:53 PM, jhjourdan
reply@reply.github.com
wrote:

Le 08/07/2012 20:38, Whit Armstrong a écrit :

I've seen this w/ gcc 4.7. I had assumed it was a bug in gcc.

Are you sure that gcc w/ optimizations on is really conforming to the
c++ standard?

There are bugs, but that is what they intend to, yes. At least with -O2,
and the bad behavior exists with this option.

how can you confirm this? const values are stored in a separate area
of the program. So, all refs to a given const value should actually
point to the same memory address. I've confirmed this behaviour with
strings before but not w/ ints/doubles. It should be easy to test.

Actually, this is more complicated than that : the compiler can actually
chose between using a separate memory space for storing value, or
loading constants directly by using immediate values. In my assembly
code for .dgamma(0.1, 0.1), I can find :

movabsq $4591870180066957722, %r10 ; 4591870180066957722 is
; actually the qword bit-equivalent
; to the double 0.1
movabsq $4591870180066957722, %r11 ; the other 0.1
movq %r10, -6440(%rbp) ; loading this value to a temporary
; location in the stack
movq %r11, -6448(%rbp) ; idem

Where 4591870180066957722 is actually the qword bit-equivalent to the
double 0.1.

A few lines later, there is :

movq %r9, -6440(%rbp)

That is, the compiler is using the stack location -6440(%rbp) for
storing something else.

So yes, I can confirm this behavior. And I would be /very/ surprised if
gcc does never do anything similar with ints. For string literals, that
is different : a string literal has the type const char*, and is
pointing to some place that contains the right data. But the literal
actually represent a pointer, whereas int/double literals represent the
actual value.

Jacques-Henri Jourdan

-Whit

---

Reply to this email directly or view it on GitHub:
#10 (comment)

---

Reply to this email directly or view it on GitHub:
#10 (comment)

---

Reply to this email directly or view it on GitHub:
#10 (comment)

[armstrtw]

I think rvalues are the way to go. However, that could complicate the classes significantly.

All the permutations of rvalues in the hyper-parameters would need to be handled...

for instance, using Normal(x, mu, tau)

One would need
Normal(T& x, U& mu, V& tau);
Normal(T& x, U&& mu, V& tau);
Normal(T& x, U& mu, V&& tau);
Normal(T& x, U&& mu, V&& tau);

and two additional class members
U* mu_p;
V* tau_p;

both initialized to null, both deleted in the destructor, and only assigned a value if the rvalue constructor is called.

For instance (using shorthand, not actual code):

template<T,U,V>
class Normal {
U* mu_p;
V* tau_p;
Normal(T& x, U&& mu, V& tau): mu_p(nullptr), tau_p(nullptr) {
  mu_p = new double;
  *mu_p = mu;
  ...
  ...
}
~Normal() {
  // no-op if these are still null (i.e. no rvalue was passed)
  delete mu_p;
  delete tau_p;
}

};

[jhjourdan]

I implemented it using type traits (see my fork of the repo) and templates. It uses very few copy and paste.

The basic idea is that for any type T, T& && reduces to T&, and the compiler knows that it can instantiate U by T& in a function template taking U&& as argument, so it can pass a lvalue reference to the function. That is a classical trick on rvalue references, for avoiding exponential explosion of overloadings.

Fell free to ask questions for the details !

BR,
Jacques-Henri Jourdan

[armstrtw]

you're way ahead of me! Ok, let me check out your fork.

So, this is working for you in your production code?

-Whit

On Mon, Jul 16, 2012 at 9:03 PM, jhjourdan
reply@reply.github.com
wrote:

I implemented it using type traits (see my fork of the repo) and templates. It uses very few copy and paste.

The basic idea is that for any type T, T& && reduces to T&, and the compiler knows that it can instantiate U by T& in a function template taking U&& as argument, so it can pass a lvalue reference to the function. That is a classical trick on rvalue references, for avoiding exponential explosion of overloadings.

Fell free to ask questions for the details !

BR,
Jacques-Henri Jourdan

---

Reply to this email directly or view it on GitHub:
#10 (comment)

[jhjourdan]

It's hard to say whether it's working on my production code, given that
our prototype is not actually doing what we want, but mainly for some
other reason we do not fully understand (apparently not related to Cppbugs).

Anyway, what I can say is that it does compile and it does not seem to
give different results.

By the way, I have a comment : I removed the MCMCSpecialized class, as
its content (history) makes sense only for dynamic variables, and that
made my work easier.

JH

Le 16/07/2012 18:36, Whit Armstrong a écrit :

you're way ahead of me! Ok, let me check out your fork.

So, this is working for you in your production code?

-Whit

On Mon, Jul 16, 2012 at 9:03 PM, jhjourdan
reply@reply.github.com
wrote:

I implemented it using type traits (see my fork of the repo) and templates. It uses very few copy and paste.

The basic idea is that for any type T, T& && reduces to T&, and the compiler knows that it can instantiate U by T& in a function template taking U&& as argument, so it can pass a lvalue reference to the function. That is a classical trick on rvalue references, for avoiding exponential explosion of overloadings.

Fell free to ask questions for the details !

BR,
Jacques-Henri Jourdan

---

Reply to this email directly or view it on GitHub:
#10 (comment)

---

Reply to this email directly or view it on GitHub:
#10 (comment)