armstrtw
Reference over temporaries
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<Gamma>(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<Gamma>(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<Gamma>(&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<Gamma>(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
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
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 [email protected] 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<Gamma>(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<Gamma>(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<Gamma>(&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<Gamma>(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: https://github.com/armstrtw/CppBugs/issues/10
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: https://github.com/armstrtw/CppBugs/issues/10#issuecomment-6833461
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 [email protected] 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<Gamma>(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<Gamma>(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<Gamma>(&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<Gamma>(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: https://github.com/armstrtw/CppBugs/issues/10
Reply to this email directly or view it on GitHub: https://github.com/armstrtw/CppBugs/issues/10#issuecomment-6833663
Thanks for digging into this. can you send or post your test code?
-Whit
On Sun, Jul 8, 2012 at 5:53 PM, jhjourdan [email protected] 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: https://github.com/armstrtw/CppBugs/issues/10#issuecomment-6833461
Reply to this email directly or view it on GitHub: https://github.com/armstrtw/CppBugs/issues/10#issuecomment-6835306
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 [email protected] 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 [email protected] 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: https://github.com/armstrtw/CppBugs/issues/10#issuecomment-6833461
Reply to this email directly or view it on GitHub: https://github.com/armstrtw/CppBugs/issues/10#issuecomment-6835306
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 [email protected] 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 [email protected] 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: https://github.com/armstrtw/CppBugs/issues/10#issuecomment-6833461
Reply to this email directly or view it on GitHub: https://github.com/armstrtw/CppBugs/issues/10#issuecomment-6835306
Reply to this email directly or view it on GitHub: https://github.com/armstrtw/CppBugs/issues/10#issuecomment-6855692
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;
}
};
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
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 [email protected] 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: https://github.com/armstrtw/CppBugs/issues/10#issuecomment-7024150
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 [email protected] 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: https://github.com/armstrtw/CppBugs/issues/10#issuecomment-7024150
Reply to this email directly or view it on GitHub: https://github.com/armstrtw/CppBugs/issues/10#issuecomment-7024546