Re: [PATCH] Trust TREE_ADDRESSABLE
On Mon, 23 Jun 2014, Jan Hubicka wrote:
On Fri, 13 Jun 2014, Jan Hubicka wrote:
When you extract the address and use it. For example when you
do auto-parallelization and outline a part of your function it
passes arrays as addresses.
Or if you start to introduce address induction variables like
the vectorizer or IVOPTs does.
I see, nothing really done by current early/IPA optimizers and in those
cases
we also want to set TREE_ADDRESSABLE bit, too I suppose.
Do you think I should make patch for setting the NOVOPS bits in ipa
code?
No, please don't introduce new users of NOVOPS (it's a quite broken
hack - it's sth like a 'const' function with side-effects so we should
have instead used 'const' and some kind of volatile flag). We're
not using NOVOPS much and that's good (I think handling of such
function calls are somewhat broken).
I meant DECL_NONALIASED. I will test the patch and lets see.
Hi,
this patch adds the discussed code to set DECL_NOALIASED so we get better AA
with partitioning. We probably also can sed DECL_NOALIASED for variables
whose address is passed only to external calls that do not capture the
parameters (i.e. memset).
I suppose I can teach ipa-ref code about this, but will need a bit of
extra infrastructure for that, since currently REF_ADDR does not associate
any information about these.
Martin, this is related to your controlled uses. What do you think about
adding
stable UIDs into ipa_ref datastructure and then adding a vector into cgraph
edges
that describe what REFs are directly used as parameters of a given callsite?
It will take some work to maintain these, but we will be able to remove them
when
call site or parameter was eliminated in a more general way.
I suppose we could also use these to associate REFs with given use in the
satement or constructor (i.e. have pointer to statement as well as pointer to
specific use within the statement). With this we will be able to redirect
references same way as we redirect callgraph edges now. This is something I
need to for the ipa-visibility optimizations.
I don't like this very much. It's fragile and it will be very hard to
detect bugs caused by it.
Please don't spread uses of the DECL_NONALIASED hack.
If we are only concerned about LTO I'd rather have a in_lto_p check
in may_be_aliased and trust TREE_ADDRESSABLE there.
Richard.
Honza
Bootstrapped/regtested and lto-bootstrapped x86_64-linux, will commit it
shortly.
* ipa.c (clear_addressable_bit): Set also DECL_NONALIASED.
(ipa_discover_readonly_nonaddressable_var): Compute also NONALIASED.
Index: ipa.c
===
--- ipa.c (revision 211881)
+++ ipa.c (working copy)
@@ -669,6 +669,10 @@ clear_addressable_bit (varpool_node *vno
{
vnode-address_taken = false;
TREE_ADDRESSABLE (vnode-decl) = 0;
+ /* Set also non-aliased bit. In LTO, when program is partitioned, we no
longer
+ trust TREE_ADDRESSABLE for TREE_PUBLIC variables and then
DECL_NONALIASED is
+ useful to improve code. */
+ DECL_NONALIASED (vnode-decl) = 1;
return false;
}
@@ -690,6 +694,7 @@ ipa_discover_readonly_nonaddressable_var
FOR_EACH_VARIABLE (vnode)
if (!vnode-alias
(TREE_ADDRESSABLE (vnode-decl)
+ || !DECL_NONALIASED (vnode-decl)
|| !vnode-writeonly
|| !TREE_READONLY (vnode-decl)))
{
@@ -703,8 +708,8 @@ ipa_discover_readonly_nonaddressable_var
continue;
if (!address_taken)
{
- if (TREE_ADDRESSABLE (vnode-decl) dump_file)
- fprintf (dump_file, %s (non-addressable), vnode-name ());
+ if ((TREE_ADDRESSABLE (vnode-decl) || !DECL_NONALIASED
(vnode-decl)) dump_file)
+ fprintf (dump_file, %s (non-addressable non-aliased),
vnode-name ());
varpool_for_node_and_aliases (vnode, clear_addressable_bit, NULL,
true);
}
if (!address_taken !written
--
Richard Biener rguent...@suse.de
SUSE / SUSE Labs
SUSE LINUX Products GmbH - Nuernberg - AG Nuernberg - HRB 16746
GF: Jeff Hawn, Jennifer Guild, Felix Imendorffer
Re: [PATCH] Trust TREE_ADDRESSABLE
Hi,
On Mon, Jun 23, 2014 at 04:55:36AM +0200, Jan Hubicka wrote:
On Fri, 13 Jun 2014, Jan Hubicka wrote:
When you extract the address and use it. For example when you
do auto-parallelization and outline a part of your function it
passes arrays as addresses.
Or if you start to introduce address induction variables like
the vectorizer or IVOPTs does.
I see, nothing really done by current early/IPA optimizers and in those
cases
we also want to set TREE_ADDRESSABLE bit, too I suppose.
Do you think I should make patch for setting the NOVOPS bits in ipa
code?
No, please don't introduce new users of NOVOPS (it's a quite broken
hack - it's sth like a 'const' function with side-effects so we should
have instead used 'const' and some kind of volatile flag). We're
not using NOVOPS much and that's good (I think handling of such
function calls are somewhat broken).
I meant DECL_NONALIASED. I will test the patch and lets see.
Hi,
this patch adds the discussed code to set DECL_NOALIASED so we get better AA
with partitioning. We probably also can sed DECL_NOALIASED for variables
whose address is passed only to external calls that do not capture the
parameters (i.e. memset).
I suppose I can teach ipa-ref code about this, but will need a bit of
extra infrastructure for that, since currently REF_ADDR does not associate
any information about these.
Martin, this is related to your controlled uses. What do you think about
adding
stable UIDs into ipa_ref datastructure and then adding a vector into cgraph
edges
that describe what REFs are directly used as parameters of a given callsite?
It will take some work to maintain these, but we will be able to remove them
when
call site or parameter was eliminated in a more general way.
I'm still recovering from getting up at six in the morning today so I
may be a bit slow but: the big patch already assigns (per-function)
UIDs to interesting DECLs and then maintains this information in jump
functions. The only advantage of reference UIDs I can think of now is
that we would stop treating all references from and to same things as
equal (because currently we delete the first one we find). Is that
what you want to achieve?
And by the way, if we add support for nocapture calls like memeset
that you described above, the big ipa-prop noescape patch will
actually directly calculate the nonaliased flag. Perhaps it should be
even called that and not noescape.
I suppose we could also use these to associate REFs with given use in the
satement or constructor (i.e. have pointer to statement as well as pointer to
specific use within the statement). With this we will be able to redirect
references same way as we redirect callgraph edges now. This is something I
need to for the ipa-visibility optimizations.
I see. I will think about this some more (and will be happy to chat on IRC).
Thanks,
Martin
Honza
Bootstrapped/regtested and lto-bootstrapped x86_64-linux, will commit it
shortly.
* ipa.c (clear_addressable_bit): Set also DECL_NONALIASED.
(ipa_discover_readonly_nonaddressable_var): Compute also NONALIASED.
Index: ipa.c
===
--- ipa.c (revision 211881)
+++ ipa.c (working copy)
@@ -669,6 +669,10 @@ clear_addressable_bit (varpool_node *vno
{
vnode-address_taken = false;
TREE_ADDRESSABLE (vnode-decl) = 0;
+ /* Set also non-aliased bit. In LTO, when program is partitioned, we no
longer
+ trust TREE_ADDRESSABLE for TREE_PUBLIC variables and then
DECL_NONALIASED is
+ useful to improve code. */
+ DECL_NONALIASED (vnode-decl) = 1;
return false;
}
@@ -690,6 +694,7 @@ ipa_discover_readonly_nonaddressable_var
FOR_EACH_VARIABLE (vnode)
if (!vnode-alias
(TREE_ADDRESSABLE (vnode-decl)
+ || !DECL_NONALIASED (vnode-decl)
|| !vnode-writeonly
|| !TREE_READONLY (vnode-decl)))
{
@@ -703,8 +708,8 @@ ipa_discover_readonly_nonaddressable_var
continue;
if (!address_taken)
{
- if (TREE_ADDRESSABLE (vnode-decl) dump_file)
- fprintf (dump_file, %s (non-addressable), vnode-name ());
+ if ((TREE_ADDRESSABLE (vnode-decl) || !DECL_NONALIASED
(vnode-decl)) dump_file)
+ fprintf (dump_file, %s (non-addressable non-aliased),
vnode-name ());
varpool_for_node_and_aliases (vnode, clear_addressable_bit, NULL,
true);
}
if (!address_taken !written
Re: [PATCH] Trust TREE_ADDRESSABLE
I don't like this very much. It's fragile and it will be very hard to detect bugs caused by it. Please don't spread uses of the DECL_NONALIASED hack. If we are only concerned about LTO I'd rather have a in_lto_p check in may_be_aliased and trust TREE_ADDRESSABLE there. I do not like it ether, but I tought it was outcome of the discussion to use it. I do not see how in_lto_p helps here, but we probably want to go for the altnerative where ipa-visibility sets TREE_ADDRESSABLE for all external variables and then we trust it unconditonally? Honza
Re: [PATCH] Trust TREE_ADDRESSABLE
On June 23, 2014 6:15:10 PM CEST, Jan Hubicka hubi...@ucw.cz wrote: I don't like this very much. It's fragile and it will be very hard to detect bugs caused by it. Please don't spread uses of the DECL_NONALIASED hack. If we are only concerned about LTO I'd rather have a in_lto_p check in may_be_aliased and trust TREE_ADDRESSABLE there. I do not like it ether, but I tought it was outcome of the discussion to use it. I do not see how in_lto_p helps here, but we probably want to go for the If we are sure it is correctly set in lto1 it helps. altnerative where ipa-visibility sets TREE_ADDRESSABLE for all external variables and then we trust it unconditonally? That works for me, too. But at least add a checking assert that may-be-aliased is not invoked before that. Richard. Honza
Re: [PATCH] Trust TREE_ADDRESSABLE
On June 23, 2014 6:15:10 PM CEST, Jan Hubicka hubi...@ucw.cz wrote: I don't like this very much. It's fragile and it will be very hard to detect bugs caused by it. Please don't spread uses of the DECL_NONALIASED hack. If we are only concerned about LTO I'd rather have a in_lto_p check in may_be_aliased and trust TREE_ADDRESSABLE there. I do not like it ether, but I tought it was outcome of the discussion to use it. I do not see how in_lto_p helps here, but we probably want to go for the If we are sure it is correctly set in lto1 it helps. altnerative where ipa-visibility sets TREE_ADDRESSABLE for all external variables and then we trust it unconditonally? That works for me, too. But at least add a checking assert that may-be-aliased is not invoked before that. OK, I suppose can check cgraph_state for that (after construction it will be all set). Will prepapre patch tonight. Honza Richard. Honza
Re: [PATCH] Trust TREE_ADDRESSABLE
On Fri, 13 Jun 2014, Jan Hubicka wrote:
When you extract the address and use it. For example when you
do auto-parallelization and outline a part of your function it
passes arrays as addresses.
Or if you start to introduce address induction variables like
the vectorizer or IVOPTs does.
I see, nothing really done by current early/IPA optimizers and in those
cases
we also want to set TREE_ADDRESSABLE bit, too I suppose.
Do you think I should make patch for setting the NOVOPS bits in ipa code?
No, please don't introduce new users of NOVOPS (it's a quite broken
hack - it's sth like a 'const' function with side-effects so we should
have instead used 'const' and some kind of volatile flag). We're
not using NOVOPS much and that's good (I think handling of such
function calls are somewhat broken).
I meant DECL_NONALIASED. I will test the patch and lets see.
Hi,
this patch adds the discussed code to set DECL_NOALIASED so we get better AA
with partitioning. We probably also can sed DECL_NOALIASED for variables
whose address is passed only to external calls that do not capture the
parameters (i.e. memset).
I suppose I can teach ipa-ref code about this, but will need a bit of
extra infrastructure for that, since currently REF_ADDR does not associate
any information about these.
Martin, this is related to your controlled uses. What do you think about adding
stable UIDs into ipa_ref datastructure and then adding a vector into cgraph
edges
that describe what REFs are directly used as parameters of a given callsite?
It will take some work to maintain these, but we will be able to remove them
when
call site or parameter was eliminated in a more general way.
I suppose we could also use these to associate REFs with given use in the
satement or constructor (i.e. have pointer to statement as well as pointer to
specific use within the statement). With this we will be able to redirect
references same way as we redirect callgraph edges now. This is something I
need to for the ipa-visibility optimizations.
Honza
Bootstrapped/regtested and lto-bootstrapped x86_64-linux, will commit it
shortly.
* ipa.c (clear_addressable_bit): Set also DECL_NONALIASED.
(ipa_discover_readonly_nonaddressable_var): Compute also NONALIASED.
Index: ipa.c
===
--- ipa.c (revision 211881)
+++ ipa.c (working copy)
@@ -669,6 +669,10 @@ clear_addressable_bit (varpool_node *vno
{
vnode-address_taken = false;
TREE_ADDRESSABLE (vnode-decl) = 0;
+ /* Set also non-aliased bit. In LTO, when program is partitioned, we no
longer
+ trust TREE_ADDRESSABLE for TREE_PUBLIC variables and then DECL_NONALIASED
is
+ useful to improve code. */
+ DECL_NONALIASED (vnode-decl) = 1;
return false;
}
@@ -690,6 +694,7 @@ ipa_discover_readonly_nonaddressable_var
FOR_EACH_VARIABLE (vnode)
if (!vnode-alias
(TREE_ADDRESSABLE (vnode-decl)
+ || !DECL_NONALIASED (vnode-decl)
|| !vnode-writeonly
|| !TREE_READONLY (vnode-decl)))
{
@@ -703,8 +708,8 @@ ipa_discover_readonly_nonaddressable_var
continue;
if (!address_taken)
{
- if (TREE_ADDRESSABLE (vnode-decl) dump_file)
- fprintf (dump_file, %s (non-addressable), vnode-name ());
+ if ((TREE_ADDRESSABLE (vnode-decl) || !DECL_NONALIASED
(vnode-decl)) dump_file)
+ fprintf (dump_file, %s (non-addressable non-aliased),
vnode-name ());
varpool_for_node_and_aliases (vnode, clear_addressable_bit, NULL,
true);
}
if (!address_taken !written
Re: [PATCH] Trust TREE_ADDRESSABLE
On Fri, 13 Jun 2014, Jan Hubicka wrote: When you extract the address and use it. For example when you do auto-parallelization and outline a part of your function it passes arrays as addresses. Or if you start to introduce address induction variables like the vectorizer or IVOPTs does. I see, nothing really done by current early/IPA optimizers and in those cases we also want to set TREE_ADDRESSABLE bit, too I suppose. Do you think I should make patch for setting the NOVOPS bits in ipa code? No, please don't introduce new users of NOVOPS (it's a quite broken hack - it's sth like a 'const' function with side-effects so we should have instead used 'const' and some kind of volatile flag). We're not using NOVOPS much and that's good (I think handling of such function calls are somewhat broken). Richard.
Re: [PATCH] Trust TREE_ADDRESSABLE
On Fri, 13 Jun 2014, Jan Hubicka wrote: When you extract the address and use it. For example when you do auto-parallelization and outline a part of your function it passes arrays as addresses. Or if you start to introduce address induction variables like the vectorizer or IVOPTs does. I see, nothing really done by current early/IPA optimizers and in those cases we also want to set TREE_ADDRESSABLE bit, too I suppose. Do you think I should make patch for setting the NOVOPS bits in ipa code? No, please don't introduce new users of NOVOPS (it's a quite broken hack - it's sth like a 'const' function with side-effects so we should have instead used 'const' and some kind of volatile flag). We're not using NOVOPS much and that's good (I think handling of such function calls are somewhat broken). I meant DECL_NONALIASED. I will test the patch and lets see. Honza Richard.
Re: [PATCH] Trust TREE_ADDRESSABLE
If we want to give frontends a way to pass information that address of a given global object is not taken (apparently useful for Ada and its alias attribute), then I do not think we are looking for middle-end only solution. I don't feel very confortable with doing that in Ada, since everybody seems to be thinking that TRE_PUBLIC/DECL_EXTERNAL objects are implicitly addressable (see for example Steven's reasoning in an earlier message). If we really do not want to revisit TREE_ADDRESSABLE in frontends, we can do the following: 1) change semantics of addressable flag on global variables in a way Richard did, document it is initialized only after symbol table is built 2) add code to cgraph construction to set TREE_ADDRESSABLE on every global variable it sees. IPA visibility is run before early optimizations. I suppose we can set it there. I.e. in function_and_variable_visibility whenever we set externally_visible and we have !in_lto_p It is bit of hack. 3) perhaps add some way to avoid 2) on objects we want - apparenlty we now have DECL_NONALIASED that may be useful for this. Then how about using DECL_NONALIASED instead of TREE_ADDRESSABLE to achieve the initial goal here? That is to say, may_be_aliased tests DECL_NONALIASED for TREE_PUBLIC/DECL_EXTERNAL DECLs and the LTO front-end sets it properly. -- Eric Botcazou
Re: [PATCH] Trust TREE_ADDRESSABLE
On Thu, 12 Jun 2014, Eric Botcazou wrote: If we want to give frontends a way to pass information that address of a given global object is not taken (apparently useful for Ada and its alias attribute), then I do not think we are looking for middle-end only solution. I don't feel very confortable with doing that in Ada, since everybody seems to be thinking that TRE_PUBLIC/DECL_EXTERNAL objects are implicitly addressable (see for example Steven's reasoning in an earlier message). If we really do not want to revisit TREE_ADDRESSABLE in frontends, we can do the following: 1) change semantics of addressable flag on global variables in a way Richard did, document it is initialized only after symbol table is built 2) add code to cgraph construction to set TREE_ADDRESSABLE on every global variable it sees. IPA visibility is run before early optimizations. I suppose we can set it there. I.e. in function_and_variable_visibility whenever we set externally_visible and we have !in_lto_p It is bit of hack. 3) perhaps add some way to avoid 2) on objects we want - apparenlty we now have DECL_NONALIASED that may be useful for this. Then how about using DECL_NONALIASED instead of TREE_ADDRESSABLE to achieve the initial goal here? That is to say, may_be_aliased tests DECL_NONALIASED for TREE_PUBLIC/DECL_EXTERNAL DECLs and the LTO front-end sets it properly. Btw, may_be_aliased already does that. So yes, when LTO promotes sth from non-public to public but hidden visibility and TREE_ADDRESSABLE was not set it could set DECL_NONALIASED. That would at least preserve the aliasing behavior from without using LTO. If the resolution info from the linker allows us to make initial public variables hidden _and_ some LTO IPA pass proves that the variables address is not taken then that pass can set DECL_NONALIASED as well. Of course one issue is that it's impossible to write a verifier that checks whether DECL_NONALIASED and TREE_ADDRESSABLE are out-of-sync (because by design they can be). So it's a bit more fragile (we could make the operand scanner that updates TREE_ADDRESSABLE also unset DECL_NONALIASED of course). Richard.
Re: [PATCH] Trust TREE_ADDRESSABLE
On Thu, 12 Jun 2014, Eric Botcazou wrote: If we want to give frontends a way to pass information that address of a given global object is not taken (apparently useful for Ada and its alias attribute), then I do not think we are looking for middle-end only solution. I don't feel very confortable with doing that in Ada, since everybody seems to be thinking that TRE_PUBLIC/DECL_EXTERNAL objects are implicitly addressable (see for example Steven's reasoning in an earlier message). If we really do not want to revisit TREE_ADDRESSABLE in frontends, we can do the following: 1) change semantics of addressable flag on global variables in a way Richard did, document it is initialized only after symbol table is built 2) add code to cgraph construction to set TREE_ADDRESSABLE on every global variable it sees. IPA visibility is run before early optimizations. I suppose we can set it there. I.e. in function_and_variable_visibility whenever we set externally_visible and we have !in_lto_p It is bit of hack. 3) perhaps add some way to avoid 2) on objects we want - apparenlty we now have DECL_NONALIASED that may be useful for this. Then how about using DECL_NONALIASED instead of TREE_ADDRESSABLE to achieve the initial goal here? That is to say, may_be_aliased tests DECL_NONALIASED for TREE_PUBLIC/DECL_EXTERNAL DECLs and the LTO front-end sets it properly. Btw, may_be_aliased already does that. So yes, when LTO promotes sth from non-public to public but hidden visibility and TREE_ADDRESSABLE was not set it could set DECL_NONALIASED. That would at least preserve the aliasing behavior from without using LTO. If the resolution info from the linker allows us to make initial public variables hidden _and_ some LTO IPA pass proves that the variables address is not taken then that pass can set DECL_NONALIASED as well. Yep, I suppose each time I clear TREE_ADDRESSABLE flag, i can also set DECL_NONALIASED. Of course one issue is that it's impossible to write a verifier that checks whether DECL_NONALIASED and TREE_ADDRESSABLE are out-of-sync (because by design they can be). So it's a bit more fragile (we could make the operand scanner that updates TREE_ADDRESSABLE also unset DECL_NONALIASED of course). Hmm,when one would unset it? Honza Richard.
Re: [PATCH] Trust TREE_ADDRESSABLE
Btw, may_be_aliased already does that. Indeed, and we could make use of that in Ada, at least in some cases. Of course one issue is that it's impossible to write a verifier that checks whether DECL_NONALIASED and TREE_ADDRESSABLE are out-of-sync (because by design they can be). So it's a bit more fragile (we could make the operand scanner that updates TREE_ADDRESSABLE also unset DECL_NONALIASED of course). IMO it's also more robust because the default (no DECL_NONALIASED) is safe. -- Eric Botcazou
Re: [PATCH] Trust TREE_ADDRESSABLE
On Thu, 12 Jun 2014, Jan Hubicka wrote: On Thu, 12 Jun 2014, Eric Botcazou wrote: If we want to give frontends a way to pass information that address of a given global object is not taken (apparently useful for Ada and its alias attribute), then I do not think we are looking for middle-end only solution. I don't feel very confortable with doing that in Ada, since everybody seems to be thinking that TRE_PUBLIC/DECL_EXTERNAL objects are implicitly addressable (see for example Steven's reasoning in an earlier message). If we really do not want to revisit TREE_ADDRESSABLE in frontends, we can do the following: 1) change semantics of addressable flag on global variables in a way Richard did, document it is initialized only after symbol table is built 2) add code to cgraph construction to set TREE_ADDRESSABLE on every global variable it sees. IPA visibility is run before early optimizations. I suppose we can set it there. I.e. in function_and_variable_visibility whenever we set externally_visible and we have !in_lto_p It is bit of hack. 3) perhaps add some way to avoid 2) on objects we want - apparenlty we now have DECL_NONALIASED that may be useful for this. Then how about using DECL_NONALIASED instead of TREE_ADDRESSABLE to achieve the initial goal here? That is to say, may_be_aliased tests DECL_NONALIASED for TREE_PUBLIC/DECL_EXTERNAL DECLs and the LTO front-end sets it properly. Btw, may_be_aliased already does that. So yes, when LTO promotes sth from non-public to public but hidden visibility and TREE_ADDRESSABLE was not set it could set DECL_NONALIASED. That would at least preserve the aliasing behavior from without using LTO. If the resolution info from the linker allows us to make initial public variables hidden _and_ some LTO IPA pass proves that the variables address is not taken then that pass can set DECL_NONALIASED as well. Yep, I suppose each time I clear TREE_ADDRESSABLE flag, i can also set DECL_NONALIASED. Of course one issue is that it's impossible to write a verifier that checks whether DECL_NONALIASED and TREE_ADDRESSABLE are out-of-sync (because by design they can be). So it's a bit more fragile (we could make the operand scanner that updates TREE_ADDRESSABLE also unset DECL_NONALIASED of course). Hmm,when one would unset it? When you extract the address and use it. For example when you do auto-parallelization and outline a part of your function it passes arrays as addresses. Or if you start to introduce address induction variables like the vectorizer or IVOPTs does. Richard.
Re: [PATCH] Trust TREE_ADDRESSABLE
When you extract the address and use it. For example when you do auto-parallelization and outline a part of your function it passes arrays as addresses. Or if you start to introduce address induction variables like the vectorizer or IVOPTs does. I see, nothing really done by current early/IPA optimizers and in those cases we also want to set TREE_ADDRESSABLE bit, too I suppose. Do you think I should make patch for setting the NOVOPS bits in ipa code? Honza Richard.
Re: [PATCH] Trust TREE_ADDRESSABLE
On Wed, 11 Jun 2014, Jan Hubicka wrote:
Note that I'm happy to revert the change.
I am hesitant to any approach that overloads TREE_ADDRESSABLE even more.
It already is used for two (slightly) different things - first the
old meaning that the address of the symbol is needed, second, that
the symbol is aliased by pointers. Those are of course related, but
as you see they are not 100% equivalent.
An alternative is surely to add a flag to varpool. But again, having several
flags of similar names and slightly different meanings doesn't make things
more
maintaible either.
As I already added DECL_NONALIASED (for VAR_DECLs) to fix that
coverage counter issue (those are TREE_STATIC but they have their
address taken - still we know that no pointers alias the accesses),
we can as well rely on that flag - but then we should set it whenever
a TU-local decl does not have its address taken (!TREE_ADDRESSABLE).
I see, I did not notice this. Will this help me with the situation where
address is taken, but it is only passed to external calls that do not capture
it (i.e. memset), so we know it does not appear in the points-to sets?
Yes. But I think it will be hard to keep this correct - if you consider
that sth could pull out the argument to a pointer. This is why I didn't
bother to try to do sth fancy with DECL_NONALIASED. But the
update-address-taken pass could in theory do sth about this (at least
for automatics).
So it does impose some redundancy and possibility of things to go
out-of-sync.
Btw, the C frontend doesn't call varpool_finalize_decl for externals,
so setting TREE_ADDRESSABLE there doesn't work unfortunately. It
works with doing it in varpool_node_for_decl though.
Patch doing both attached (we may choose to do this in different
places for DECL_EXTERNALs vs. TREE_PUBLIC TREE_STATICs?).
At LTO input time we directly call symtab_register_node which would
side-step this thus an IPA pass could drop TREE_ADDRESSABLE from
those decls.
Sofar untested.
Comments?
I think it may be easier to just set the flag as part of the ipa-visiblity
pass. I.e. at the time we set externally_visile, we can also set
TREE_ADDRESSABLE for variable. We don't use alias machinery before
that, right?
That's the pass_ipa_function_and_variable_visibility pass run during
early small IPA passes right after lowering? No, currently there
are no uses of the alias machinery there - but having an incorrect
state in the IL for quite some time sounds dangerous to me.
I have now reverted the original patch while we discuss things here.
Richard.
2014-06-11 Richard Biener rguent...@suse.de
PR middle-end/61437
Revert
2014-06-04 Richard Biener rguent...@suse.de
* tree.h (may_be_aliased): Trust TREE_ADDRESSABLE from
TREE_PUBLIC and DECL_EXTERNAL decls.
* gcc.dg/torture/20140610-1.c: New testcase.
* gcc.dg/torture/20140610-2.c: Likewise.
Index: gcc/tree.h
===
--- gcc/tree.h (revision 211215)
+++ gcc/tree.h (working copy)
@@ -4506,9 +4506,7 @@ static inline bool
may_be_aliased (const_tree var)
{
return (TREE_CODE (var) != CONST_DECL
- (TREE_PUBLIC (var)
- || DECL_EXTERNAL (var)
- || TREE_ADDRESSABLE (var))
+ TREE_ADDRESSABLE (var)
!((TREE_STATIC (var) || TREE_PUBLIC (var) || DECL_EXTERNAL (var))
((TREE_READONLY (var)
!TYPE_NEEDS_CONSTRUCTING (TREE_TYPE (var)))
Re: [PATCH] Trust TREE_ADDRESSABLE
In non-LTO compilation I think it would allows us to optimize bettter in the case address of the global symbol can not be taken by the other unit or it can not escape back to current unit. For C/C++ this would be for runtime generates symbols, like for gcov runtime (well if our initialization was not implemented in a way taking address of everything). For non-C languages I expect there are cases where you just can't take address of a given object. Yes, that's true in Ada :-), at least theoritically since you need to explicitly mark objects with aliased to be able to take their address. For LTO compilation this is useful for optimizing cases where variable is static at whole program optimization time, but it becomes hidden by partitioning and at whole program time we figure out its address is not taken. Then why not just make the LTO front-end follow the existing semantics of TREE_ADDRESSABLE and clear it when it figures out that the address is not taken? That would seem the natural thing to do here. -- Eric Botcazou
Re: [PATCH] Trust TREE_ADDRESSABLE
Then why not just make the LTO front-end follow the existing semantics of TREE_ADDRESSABLE and clear it when it figures out that the address is not taken? That would seem the natural thing to do here. I meant clear the TREE_PUBLIC/DECL_EXTERNAL flags of course... -- Eric Botcazou
Re: [PATCH] Trust TREE_ADDRESSABLE
Note that I'm happy to revert the change. Thanks. I am hesitant to any approach that overloads TREE_ADDRESSABLE even more. It already is used for two (slightly) different things - first the old meaning that the address of the symbol is needed, second, that the symbol is aliased by pointers. Those are of course related, but as you see they are not 100% equivalent. As I already added DECL_NONALIASED (for VAR_DECLs) to fix that coverage counter issue (those are TREE_STATIC but they have their address taken - still we know that no pointers alias the accesses), we can as well rely on that flag - but then we should set it whenever a TU-local decl does not have its address taken (!TREE_ADDRESSABLE). Why not just make the change to may_be_aliased in LTO mode, with a comment saying that TREE_PUBLIC and DECL_EXTERNAL aren't fully correct any longer? -- Eric Botcazou
Re: [PATCH] Trust TREE_ADDRESSABLE
On Wed, Jun 11, 2014 at 10:57 AM, Eric Botcazou ebotca...@adacore.com wrote: Then why not just make the LTO front-end follow the existing semantics of TREE_ADDRESSABLE and clear it when it figures out that the address is not taken? That would seem the natural thing to do here. I meant clear the TREE_PUBLIC/DECL_EXTERNAL flags of course... But that's not possible - because the symbol may be referred to by another partition. Thus we make the symbols only with hidden visibility but still TREE_PUBLIC. Richard. -- Eric Botcazou
Re: [PATCH] Trust TREE_ADDRESSABLE
On Wed, Jun 11, 2014 at 11:08 AM, Eric Botcazou ebotca...@adacore.com wrote: Note that I'm happy to revert the change. Thanks. I am hesitant to any approach that overloads TREE_ADDRESSABLE even more. It already is used for two (slightly) different things - first the old meaning that the address of the symbol is needed, second, that the symbol is aliased by pointers. Those are of course related, but as you see they are not 100% equivalent. As I already added DECL_NONALIASED (for VAR_DECLs) to fix that coverage counter issue (those are TREE_STATIC but they have their address taken - still we know that no pointers alias the accesses), we can as well rely on that flag - but then we should set it whenever a TU-local decl does not have its address taken (!TREE_ADDRESSABLE). Why not just make the change to may_be_aliased in LTO mode, with a comment saying that TREE_PUBLIC and DECL_EXTERNAL aren't fully correct any longer? Because that's not the point and because it feels like a hack ;) I could do || in_lto_p and make LTO properly set TREE_ADDRESSABLE. But then we can make the IPA machinery properly set TREE_ADDRESSABLE as well, unconditionally, just like with my proposed patch to fix the fallout. Richard. -- Eric Botcazou
Re: [PATCH] Trust TREE_ADDRESSABLE
Because that's not the point and because it feels like a hack ;) Well, if we keep the current semantics of TREE_ADDRESSABLE and decide that the predicate for aliasing is may_be_aliased, the implementation for the latter becomes a detail. And it would seem better/simpler to have the knowledge localized in this single predicate than spread over multiple files and FEs. -- Eric Botcazou
Re: [PATCH] Trust TREE_ADDRESSABLE
On Wed, 11 Jun 2014, Eric Botcazou wrote: Because that's not the point and because it feels like a hack ;) Well, if we keep the current semantics of TREE_ADDRESSABLE and decide that the predicate for aliasing is may_be_aliased, the implementation for the latter becomes a detail. And it would seem better/simpler to have the knowledge localized in this single predicate than spread over multiple files and FEs. Sure. Still currently TREE_ADDRESSABLE on TREE_PUBLIC/DECL_EXTERNAL VAR_DECLs carries no useful information, so I consider the bit unused. I propose to add semantics for it. Btw, the optimization is not restricted to LTO but also applies to -fwhole-program (poor-mans single-TU LTO). So it is a property that could be computed (as Honza says) by the IPA visibility pass (in the -fwhole-program non-LTO case it promotes those decls to !TREE_PUBLIC TREE_STATIC). Richard.
Re: [PATCH] Trust TREE_ADDRESSABLE
Sure. Still currently TREE_ADDRESSABLE on TREE_PUBLIC/DECL_EXTERNAL VAR_DECLs carries no useful information, so I consider the bit unused. I guess that's true for the middle-end in non-LTO mode at this point. But then the new approach shouldn't be make correctness depend on its setting in the front-ends, that's too error-prone IMO. I propose to add semantics for it. Fine with me, as long as it's centralized somewhere in the middle-end. -- Eric Botcazou
Re: [PATCH] Trust TREE_ADDRESSABLE
Sure. Still currently TREE_ADDRESSABLE on TREE_PUBLIC/DECL_EXTERNAL VAR_DECLs carries no useful information, so I consider the bit unused. I guess that's true for the middle-end in non-LTO mode at this point. But then the new approach shouldn't be make correctness depend on its setting in the front-ends, that's too error-prone IMO. I propose to add semantics for it. Fine with me, as long as it's centralized somewhere in the middle-end. If we want to give frontends a way to pass information that address of a given global object is not taken (apparently useful for Ada and its alias attribute), then I do not think we are looking for middle-end only solution. If we really do not want to revisit TREE_ADDRESSABLE in frontends, we can do the following: 1) change semantics of addressable flag on global variables in a way Richard did, document it is initialized only after symbol table is built 2) add code to cgraph construction to set TREE_ADDRESSABLE on every global variable it sees. IPA visibility is run before early optimizations. I suppose we can set it there. I.e. in function_and_variable_visibility whenever we set externally_visible and we have !in_lto_p It is bit of hack. 3) perhaps add some way to avoid 2) on objects we want - apparenlty we now have DECL_NONALIASED that may be useful for this. Honza
Re: [PATCH] Trust TREE_ADDRESSABLE
Why not just make the change to may_be_aliased in LTO mode, with a comment saying that TREE_PUBLIC and DECL_EXTERNAL aren't fully correct any longer? They are fully correct to the partition being compiled BTW. Honza
Re: [PATCH] Trust TREE_ADDRESSABLE
On Mon, 9 Jun 2014, Eric Botcazou wrote:
I wonder if we want toupdate the frontends to set addressable flag with new
sense or we want symtab to simple set addressable on all global symbols or
invent a new flag.
I would preffer the first case - it seems to make most sense to me.
I think you need to explain why this change is desirable/necessary for LTO,
this would be a good starting point. As for setting TREE_ADDRESSABLE on
every
single global symbol in every single front-end, why not, but this seems more
complicated than treating global symbols as so by default (in non-LTO mode).
Note that I'm happy to revert the change.
I am hesitant to any approach that overloads TREE_ADDRESSABLE even more.
It already is used for two (slightly) different things - first the
old meaning that the address of the symbol is needed, second, that
the symbol is aliased by pointers. Those are of course related, but
as you see they are not 100% equivalent.
As I already added DECL_NONALIASED (for VAR_DECLs) to fix that
coverage counter issue (those are TREE_STATIC but they have their
address taken - still we know that no pointers alias the accesses),
we can as well rely on that flag - but then we should set it whenever
a TU-local decl does not have its address taken (!TREE_ADDRESSABLE).
So it does impose some redundancy and possibility of things to go
out-of-sync.
Btw, the C frontend doesn't call varpool_finalize_decl for externals,
so setting TREE_ADDRESSABLE there doesn't work unfortunately. It
works with doing it in varpool_node_for_decl though.
Patch doing both attached (we may choose to do this in different
places for DECL_EXTERNALs vs. TREE_PUBLIC TREE_STATICs?).
At LTO input time we directly call symtab_register_node which would
side-step this thus an IPA pass could drop TREE_ADDRESSABLE from
those decls.
Sofar untested.
Comments?
Thanks,
Richard.
2014-06-10 Richard Biener rguent...@suse.de
* tree.h (TREE_ADDRESSABLE): Clarify.
* varpool.c (varpool_node_for_decl): Mark public or external
variables as TREE_ADDRESSABLE.
* cgraphunit.c (varpool_finalize_decl): Likewise.
* gcc.dg/torture/20140610-1.c: New testcase.
* gcc.dg/torture/20140610-2.c: Likewise.
Index: gcc/tree.h
===
--- gcc/tree.h (revision 211398)
+++ gcc/tree.h (working copy)
@@ -571,8 +571,9 @@ extern void omp_clause_range_check_faile
/* Define many boolean fields that all tree nodes have. */
-/* In VAR_DECL, PARM_DECL and RESULT_DECL nodes, nonzero means address
- of this is needed. So it cannot be in a register.
+/* In VAR_DECL, PARM_DECL and RESULT_DECL nodes, nonzero means the address
+ of this is needed. So it cannot be in a register. If not set, then
+ the address of this cannot be used to initialize an aliasing pointer.
In a FUNCTION_DECL it has no meaning.
In LABEL_DECL nodes, it means a goto for this label has been seen
from a place outside all binding contours that restore stack levels.
Index: gcc/varpool.c
===
--- gcc/varpool.c (revision 211398)
+++ gcc/varpool.c (working copy)
@@ -149,6 +149,8 @@ varpool_node_for_decl (tree decl)
if (node)
return node;
+ if (TREE_PUBLIC (decl) || DECL_EXTERNAL (decl))
+TREE_ADDRESSABLE (decl) = 1;
node = varpool_create_empty_node ();
node-decl = decl;
symtab_register_node (node);
Index: gcc/cgraphunit.c
===
--- gcc/cgraphunit.c(revision 211398)
+++ gcc/cgraphunit.c(working copy)
@@ -818,6 +818,11 @@ varpool_finalize_decl (tree decl)
gcc_assert (TREE_STATIC (decl) || DECL_EXTERNAL (decl));
+ /* Mark all symbols visible to other TUs as possibly having their
+ address taken. */
+ if (TREE_PUBLIC (decl) || DECL_EXTERNAL (decl))
+TREE_ADDRESSABLE (decl) = 1;
+
if (node-definition)
return;
notice_global_symbol (decl);
Index: gcc/testsuite/gcc.dg/torture/20140610-1.c
===
--- gcc/testsuite/gcc.dg/torture/20140610-1.c (revision 0)
+++ gcc/testsuite/gcc.dg/torture/20140610-1.c (working copy)
@@ -0,0 +1,15 @@
+/* { dg-do run } */
+/* { dg-additional-sources 20140610-2.c } */
+
+extern int a;
+extern int *p;
+
+void test (void);
+
+int main ()
+{
+ *p = 0;
+ a = 1;
+ test ();
+ return 0;
+}
Index: gcc/testsuite/gcc.dg/torture/20140610-2.c
===
--- gcc/testsuite/gcc.dg/torture/20140610-2.c (revision 0)
+++ gcc/testsuite/gcc.dg/torture/20140610-2.c (working copy)
@@ -0,0 +1,12 @@
+/* { dg-do compile } */
+
+extern void abort (void);
+
+int a;
+int *p = a;
+
+void test (void)
+{
+ if (a != 1)
+abort ();
+}
Re: [PATCH] Trust TREE_ADDRESSABLE
Note that I'm happy to revert the change.
I am hesitant to any approach that overloads TREE_ADDRESSABLE even more.
It already is used for two (slightly) different things - first the
old meaning that the address of the symbol is needed, second, that
the symbol is aliased by pointers. Those are of course related, but
as you see they are not 100% equivalent.
An alternative is surely to add a flag to varpool. But again, having several
flags of similar names and slightly different meanings doesn't make things more
maintaible either.
As I already added DECL_NONALIASED (for VAR_DECLs) to fix that
coverage counter issue (those are TREE_STATIC but they have their
address taken - still we know that no pointers alias the accesses),
we can as well rely on that flag - but then we should set it whenever
a TU-local decl does not have its address taken (!TREE_ADDRESSABLE).
I see, I did not notice this. Will this help me with the situation where
address is taken, but it is only passed to external calls that do not capture
it (i.e. memset), so we know it does not appear in the points-to sets?
So it does impose some redundancy and possibility of things to go
out-of-sync.
Btw, the C frontend doesn't call varpool_finalize_decl for externals,
so setting TREE_ADDRESSABLE there doesn't work unfortunately. It
works with doing it in varpool_node_for_decl though.
Patch doing both attached (we may choose to do this in different
places for DECL_EXTERNALs vs. TREE_PUBLIC TREE_STATICs?).
At LTO input time we directly call symtab_register_node which would
side-step this thus an IPA pass could drop TREE_ADDRESSABLE from
those decls.
Sofar untested.
Comments?
I think it may be easier to just set the flag as part of the ipa-visiblity
pass. I.e. at the time we set externally_visile, we can also set
TREE_ADDRESSABLE for variable. We don't use alias machinery before
that, right?
Honza
Thanks,
Richard.
2014-06-10 Richard Biener rguent...@suse.de
* tree.h (TREE_ADDRESSABLE): Clarify.
* varpool.c (varpool_node_for_decl): Mark public or external
variables as TREE_ADDRESSABLE.
* cgraphunit.c (varpool_finalize_decl): Likewise.
* gcc.dg/torture/20140610-1.c: New testcase.
* gcc.dg/torture/20140610-2.c: Likewise.
Index: gcc/tree.h
===
--- gcc/tree.h(revision 211398)
+++ gcc/tree.h(working copy)
@@ -571,8 +571,9 @@ extern void omp_clause_range_check_faile
/* Define many boolean fields that all tree nodes have. */
-/* In VAR_DECL, PARM_DECL and RESULT_DECL nodes, nonzero means address
- of this is needed. So it cannot be in a register.
+/* In VAR_DECL, PARM_DECL and RESULT_DECL nodes, nonzero means the address
+ of this is needed. So it cannot be in a register. If not set, then
+ the address of this cannot be used to initialize an aliasing pointer.
In a FUNCTION_DECL it has no meaning.
In LABEL_DECL nodes, it means a goto for this label has been seen
from a place outside all binding contours that restore stack levels.
Index: gcc/varpool.c
===
--- gcc/varpool.c (revision 211398)
+++ gcc/varpool.c (working copy)
@@ -149,6 +149,8 @@ varpool_node_for_decl (tree decl)
if (node)
return node;
+ if (TREE_PUBLIC (decl) || DECL_EXTERNAL (decl))
+TREE_ADDRESSABLE (decl) = 1;
node = varpool_create_empty_node ();
node-decl = decl;
symtab_register_node (node);
Index: gcc/cgraphunit.c
===
--- gcc/cgraphunit.c (revision 211398)
+++ gcc/cgraphunit.c (working copy)
@@ -818,6 +818,11 @@ varpool_finalize_decl (tree decl)
gcc_assert (TREE_STATIC (decl) || DECL_EXTERNAL (decl));
+ /* Mark all symbols visible to other TUs as possibly having their
+ address taken. */
+ if (TREE_PUBLIC (decl) || DECL_EXTERNAL (decl))
+TREE_ADDRESSABLE (decl) = 1;
+
if (node-definition)
return;
notice_global_symbol (decl);
Index: gcc/testsuite/gcc.dg/torture/20140610-1.c
===
--- gcc/testsuite/gcc.dg/torture/20140610-1.c (revision 0)
+++ gcc/testsuite/gcc.dg/torture/20140610-1.c (working copy)
@@ -0,0 +1,15 @@
+/* { dg-do run } */
+/* { dg-additional-sources 20140610-2.c } */
+
+extern int a;
+extern int *p;
+
+void test (void);
+
+int main ()
+{
+ *p = 0;
+ a = 1;
+ test ();
+ return 0;
+}
Index: gcc/testsuite/gcc.dg/torture/20140610-2.c
===
--- gcc/testsuite/gcc.dg/torture/20140610-2.c (revision 0)
+++ gcc/testsuite/gcc.dg/torture/20140610-2.c (working copy)
@@ -0,0 +1,12 @@
+/* { dg-do compile } */
+
+extern void abort (void);
+
+int a;
+int *p = a;
+
+void test (void)
+{
+ if (a != 1)
+
Re: [PATCH] Trust TREE_ADDRESSABLE
It is my fault here - I alwasy interpreted TREE_ADDRESSABLE this way and it seems to work for C/C++ that are the frontends I usually look into. But that's not true, see the obvious C testcase I attached in this thread! -- Eric Botcazou
Re: [PATCH] Trust TREE_ADDRESSABLE
It is my fault here - I alwasy interpreted TREE_ADDRESSABLE this way and it seems to work for C/C++ that are the frontends I usually look into. But that's not true, see the obvious C testcase I attached in this thread! You are right - I never noticed the difference in cases I looked at. I wonder if we want toupdate the frontends to set addressable flag with new sense or we want symtab to simple set addressable on all global symbols or invent a new flag. I would preffer the first case - it seems to make most sense to me. Honza -- Eric Botcazou
Re: [PATCH] Trust TREE_ADDRESSABLE
I wonder if we want toupdate the frontends to set addressable flag with new sense or we want symtab to simple set addressable on all global symbols or invent a new flag. I would preffer the first case - it seems to make most sense to me. I think you need to explain why this change is desirable/necessary for LTO, this would be a good starting point. As for setting TREE_ADDRESSABLE on every single global symbol in every single front-end, why not, but this seems more complicated than treating global symbols as so by default (in non-LTO mode). -- Eric Botcazou
Re: [PATCH] Trust TREE_ADDRESSABLE
I wonder if we want toupdate the frontends to set addressable flag with new sense or we want symtab to simple set addressable on all global symbols or invent a new flag. I would preffer the first case - it seems to make most sense to me. I think you need to explain why this change is desirable/necessary for LTO, this would be a good starting point. As for setting TREE_ADDRESSABLE on every single global symbol in every single front-end, why not, but this seems more complicated than treating global symbols as so by default (in non-LTO mode). In non-LTO compilation I think it would allows us to optimize bettter in the case address of the global symbol can not be taken by the other unit or it can not escape back to current unit. For C/C++ this would be for runtime generates symbols, like for gcov runtime (well if our initialization was not implemented in a way taking address of everything). For non-C languages I expect there are cases where you just can't take address of a given object. For LTO compilation this is useful for optimizing cases where variable is static at whole program optimization time, but it becomes hidden by partitioning and at whole program time we figure out its address is not taken. Honza
Re: [PATCH] Trust TREE_ADDRESSABLE
... In this particular translation unit you mean? Yes, in the translation unit being processed. That would be worthless information for decls also reachable from elsewhere. It's the information: ADDR_EXPR of this DECL is taken somewhere in the IL, it's no more or not less worthless than any other information. Information whether ADDR_EXPR exists on DECL is actually more accurately mainted by ipa-ref code (that is also used to drop TREE_ADDRSSABLE on static vars). Having information whether source language permits taking address of a given object is more useful. So - let's say history is something of the past? Maybe, but it's an annoying precedent: no clear explanation for the change, no testcase and no audit of the affected front-ends (all I guess). That should really have been discussed beforehand. It is my fault here - I alwasy interpreted TREE_ADDRESSABLE this way and it seems to work for C/C++ that are the frontends I usually look into. Sorry for that. Honza -- Eric Botcazou
Re: [PATCH] Trust TREE_ADDRESSABLE
Hmm, I don't see this.
Usual scheduler-sensitive stuff I guess, here's the assembly I have:
movqaliasing3_pkg__pointer(%rip), %rax
testq %rax, %rax
je .L6
cmpl$5, aliasing3_pkg__block(%rip)
movl$5, (%rax)
insns #4 and #5 have been wrongly swapped.
In Ada we don't mark (external) variables as addressable if we don't
see their address taken.
You have to (now). The testing was of course to detect this...
Well, you need to define what TREE_ADDRESSABLE means now, because according to
/* In VAR_DECL, PARM_DECL and RESULT_DECL nodes, nonzero means address
of this is needed. So it cannot be in a register.
[...]
#define TREE_ADDRESSABLE(NODE) ((NODE)-base.addressable_flag)
your change is clearly wrong and the Ada compiler clearly right. And auditing
the various front-ends might also be in order here if they really need to mark
every single external variable as addressable to be safe wrt aliasing.
Obvious testcase for the C compiler attached, compile at -O2.
This looks like a big hack to work around an LTO issue...
--
Eric Botcazouextern int a;
extern int *p;
int main (void)
{
*p = 0;
a = 1;
test ();
}extern void abort (void);
int a;
int *p = a;
void test (void)
{
if (a != 1)
abort ();
}
Re: [PATCH] Trust TREE_ADDRESSABLE
On Sat, Jun 7, 2014 at 12:59 PM, Eric Botcazou wrote: In Ada we don't mark (external) variables as addressable if we don't see their address taken. You have to (now). The testing was of course to detect this... Well, you need to define what TREE_ADDRESSABLE means now, because according to /* In VAR_DECL, PARM_DECL and RESULT_DECL nodes, nonzero means address of this is needed. So it cannot be in a register. [...] #define TREE_ADDRESSABLE(NODE) ((NODE)-base.addressable_flag) your change is clearly wrong and the Ada compiler clearly right. Clearly? An external variable is a VAR_DECL that cannot be in a register. It can be loaded into a register (or stored into), and for that its address is needed. So I would expect an external variable to be marked addressable by default. I was always surprised that this was not the case before Richi's change. And auditing the various front-ends might also be in order here if they really need to mark every single external variable as addressable to be safe wrt aliasing. Right. And this should have been done (clearly ;-) ) before the patch was committed... Ciao! Steven
Re: [PATCH] Trust TREE_ADDRESSABLE
An external variable is a VAR_DECL that cannot be in a register. It can be loaded into a register (or stored into), and for that its address is needed. So I would expect an external variable to be marked addressable by default. address of this is needed historically means ADDR_EXPR of this is taken. -- Eric Botcazou
Re: [PATCH] Trust TREE_ADDRESSABLE
On June 7, 2014 1:54:06 PM CEST, Steven Bosscher stevenb@gmail.com wrote: On Sat, Jun 7, 2014 at 12:59 PM, Eric Botcazou wrote: In Ada we don't mark (external) variables as addressable if we don't see their address taken. You have to (now). The testing was of course to detect this... Well, you need to define what TREE_ADDRESSABLE means now, because according to /* In VAR_DECL, PARM_DECL and RESULT_DECL nodes, nonzero means address of this is needed. So it cannot be in a register. [...] #define TREE_ADDRESSABLE(NODE) ((NODE)-base.addressable_flag) your change is clearly wrong and the Ada compiler clearly right. Clearly? An external variable is a VAR_DECL that cannot be in a register. It can be loaded into a register (or stored into), and for that its address is needed. So I would expect an external variable to be marked addressable by default. I was always surprised that this was not the case before Richi's change. Well, honza was as well, so I thought I must be wrong. And auditing the various front-ends might also be in order here if they really need to mark every single external variable as addressable to be safe wrt aliasing. Right. And this should have been done (clearly ;-) ) before the patch was committed... I thought that bootstrap and regtest plus what I remember from debug sessions was enough. Anyway, an easy fix is to make all globals TREE_ADDRESSABLE somewhere in the varpool code. But I have no idea how to audit the frontends and judge their language specific knowledge they put into deciding whether a deck may have its address taken. Richard. Ciao! Steven
Re: [PATCH] Trust TREE_ADDRESSABLE
On June 7, 2014 2:06:47 PM CEST, Eric Botcazou ebotca...@adacore.com wrote: An external variable is a VAR_DECL that cannot be in a register. It can be loaded into a register (or stored into), and for that its address is needed. So I would expect an external variable to be marked addressable by default. address of this is needed historically means ADDR_EXPR of this is taken. ... In this particular translation unit you mean? That would be worthless information for decls also reachable from elsewhere. So - let's say history is something of the past? Richard.
Re: [PATCH] Trust TREE_ADDRESSABLE
... In this particular translation unit you mean? Yes, in the translation unit being processed. That would be worthless information for decls also reachable from elsewhere. It's the information: ADDR_EXPR of this DECL is taken somewhere in the IL, it's no more or not less worthless than any other information. So - let's say history is something of the past? Maybe, but it's an annoying precedent: no clear explanation for the change, no testcase and no audit of the affected front-ends (all I guess). That should really have been discussed beforehand. -- Eric Botcazou
Re: [PATCH] Trust TREE_ADDRESSABLE
Bootstrapped and tested on x86_64-unknown-linux-gnu for all languages including obj-c++, ada and go (yay), applied. Something went wrong because this nevertheless introduced a regression: FAIL: gnat.dg/aliasing3.adb execution test In Ada we don't mark (external) variables as addressable if we don't see their address taken. -- Eric Botcazou
Re: [PATCH] Trust TREE_ADDRESSABLE
On June 6, 2014 7:15:55 PM CEST, Eric Botcazou ebotca...@adacore.com wrote: Bootstrapped and tested on x86_64-unknown-linux-gnu for all languages including obj-c++, ada and go (yay), applied. Something went wrong because this nevertheless introduced a regression: FAIL: gnat.dg/aliasing3.adb execution test Hmm, I don't see this. In Ada we don't mark (external) variables as addressable if we don't see their address taken. You have to (now). The testing was of course to detect this... Richard.
[PATCH] Trust TREE_ADDRESSABLE
This makes may_be_aliased trust TREE_ADDRESSABLE setting for
exported decls. This should make it possible for LTO to
compute more optimistic aliasing and removes the pessimization
with respective to aliasing that currently LTO brought global
(but hidden visibility) statics cause.
Bootstrapped and tested on x86_64-unknown-linux-gnu for all
languages including obj-c++, ada and go (yay), applied.
Richard.
2014-06-04 Richard Biener rguent...@suse.de
* tree.h (may_be_aliased): Trust TREE_ADDRESSABLE from
TREE_PUBLIC and DECL_EXTERNAL decls.
Index: gcc/tree.h
===
--- gcc/tree.h (revision 211215)
+++ gcc/tree.h (working copy)
@@ -4506,9 +4506,7 @@ static inline bool
may_be_aliased (const_tree var)
{
return (TREE_CODE (var) != CONST_DECL
- (TREE_PUBLIC (var)
- || DECL_EXTERNAL (var)
- || TREE_ADDRESSABLE (var))
+ TREE_ADDRESSABLE (var)
!((TREE_STATIC (var) || TREE_PUBLIC (var) || DECL_EXTERNAL (var))
((TREE_READONLY (var)
!TYPE_NEEDS_CONSTRUCTING (TREE_TYPE (var)))
