| Message ID | 53849277.7020201@codesourcery.com |
|---|---|
| State | New |
| Headers | show |
On Tue, May 27, 2014 at 3:26 PM, Bernd Schmidt <bernds@codesourcery.com> wrote: > When putting a constant into the constant pool, we make a copy of the tree > in build_constant_desc. This caused me some problems with the ptx backend, > which has a pass to modify the address spaces of all variables and constants > to match the requirements of the backend. It would be nice for it to be able > to find them all by walking the varpool and the gimple of all functions, and > I ran into some testcases where these copies can slip through. > > After a while it occurred to me that the copy is probably a relic of old > obstack times and not necessary anymore. And indeed, in 2.7.2.3 the only > call to it looked like this: > push_obstacks_nochange (); > suspend_momentary (); > p->exp = copy_constant (exp); > pop_obstacks (); > > It seems unlikely that anything in the compiler would want to modify a > constant after it has been put into the constant pool, and even if so, it > should have the responsibility of making the copy itself. So, as an > experiment I've bootstrapped and tested the following patch (x86_64-linux), > and that seemed to work. Ok to apply along with removal of the now unused > copy_constant? Ok if nobody raises issues within 24h. Thanks, Richard. > > Bernd >
diff --git a/gcc/varasm.c b/gcc/varasm.c index 6781096..3469384 100644 --- a/gcc/varasm.c +++ b/gcc/varasm.c @@ -3205,7 +3205,7 @@ build_constant_desc (tree exp) tree decl; desc = ggc_alloc<constant_descriptor_tree> (); - desc->value = copy_constant (exp); + desc->value = exp; /* Create a string containing the label name, in LABEL. */ labelno = const_labelno++;
When putting a constant into the constant pool, we make a copy of the tree in build_constant_desc. This caused me some problems with the ptx backend, which has a pass to modify the address spaces of all variables and constants to match the requirements of the backend. It would be nice for it to be able to find them all by walking the varpool and the gimple of all functions, and I ran into some testcases where these copies can slip through. After a while it occurred to me that the copy is probably a relic of old obstack times and not necessary anymore. And indeed, in 2.7.2.3 the only call to it looked like this: push_obstacks_nochange (); suspend_momentary (); p->exp = copy_constant (exp); pop_obstacks (); It seems unlikely that anything in the compiler would want to modify a constant after it has been put into the constant pool, and even if so, it should have the responsibility of making the copy itself. So, as an experiment I've bootstrapped and tested the following patch (x86_64-linux), and that seemed to work. Ok to apply along with removal of the now unused copy_constant? Bernd