Re: ppc64 __attribute__((visibility ("hidden"))) and multiple TOCs
> Jakub Jelinek writes: Jakub> I guess the right thing to do would be to replace the current Jakub> 3 uses of SYMBOL_REF_LOCAL_P (x) macro in config/rs6000/*.md Jakub> with Jakub> SYMBOL_REF_LOCAL_P (x) && (!TARGET_ARCH64 || !SYMBOL_REF_EXTERNAL_P (x)) Jakub> where TARGET_ARCH64 is replaced by whatever ppc* subarches need multi-toc. The only one that needs to be changed in the current_file_function_operand predicate in rs6000/predicates.md I think the test needs to be SYMBOL_REF_LOCAL_P (x) && (DEFAULT_ABI != ABI_AIX || !SYMBOL_REF_EXTERNAL_P (x)) David
Re: ppc64 __attribute__((visibility ("hidden"))) and multiple TOCs
On Mon, Jun 25, 2007 at 12:48:10PM -0400, Mark Mitchell wrote: > I was suggesting leaving the hook alone, but modifying the test for the > sibling call optimization in rs6000_function_ok_for_sibcall to: > > !DECL_EXTERNAL (decl) && binds_local_p (decl) > > In other words, per Jakub's argument, consider binds_local_p to be > correct (even on Power), but just note in the back end that > binds_local_p isn't a sufficient test for being safe for a sibling call. I guess the right thing to do would be to replace the current 3 uses of SYMBOL_REF_LOCAL_P (x) macro in config/rs6000/*.md with SYMBOL_REF_LOCAL_P (x) && (!TARGET_ARCH64 || !SYMBOL_REF_EXTERNAL_P (x)) where TARGET_ARCH64 is replaced by whatever ppc* subarches need multi-toc. SYMBOL_REF_LOCAL_P is used even in generic code and there the current definition is ok - SYMBOL_REF_LOCAL_P symbols won't need global relocations. Similarly other uses of binds_local_p I have checked. Jakub
Re: ppc64 __attribute__((visibility ("hidden"))) and multiple TOCs
David Edelsohn wrote: >> Mark Mitchell writes: > > Mark> Good point -- if there's no definition in the current translation unit, > Mark> then I guess we aren't going to make any bad assumptions about the > Mark> contents of the function. So, I guess that just means that the Power > Mark> back end needs to check for !DECL_EXTERNAL in addition to binds_local_p? > > Are you suggesting that the rs6000 port should override the hook > or just swap the order of the tests in the default hook? I was suggesting leaving the hook alone, but modifying the test for the sibling call optimization in rs6000_function_ok_for_sibcall to: !DECL_EXTERNAL (decl) && binds_local_p (decl) In other words, per Jakub's argument, consider binds_local_p to be correct (even on Power), but just note in the back end that binds_local_p isn't a sufficient test for being safe for a sibling call. -- Mark Mitchell CodeSourcery [EMAIL PROTECTED] (650) 331-3385 x713
Re: ppc64 __attribute__((visibility ("hidden"))) and multiple TOCs
> Mark Mitchell writes: Mark> Good point -- if there's no definition in the current translation unit, Mark> then I guess we aren't going to make any bad assumptions about the Mark> contents of the function. So, I guess that just means that the Power Mark> back end needs to check for !DECL_EXTERNAL in addition to binds_local_p? Are you suggesting that the rs6000 port should override the hook or just swap the order of the tests in the default hook? Thanks, David
Re: ppc64 __attribute__((visibility ("hidden"))) and multiple TOCs
Jakub Jelinek wrote: > For replacability the current definition is just fine. Weak functions > must be assumed to be always replaceable and non-weak functions which are > known to bind within the same executable resp. shared library are not > replaceable - linker will issue error if two non-weak symbols with the > same name are linked into the same executable resp. shared library and > when linking a non-weak symbol and weak symbol the non-weak one will > win. Good point -- if there's no definition in the current translation unit, then I guess we aren't going to make any bad assumptions about the contents of the function. So, I guess that just means that the Power back end needs to check for !DECL_EXTERNAL in addition to binds_local_p? -- Mark Mitchell CodeSourcery [EMAIL PROTECTED] (650) 331-3385 x713
Re: ppc64 __attribute__((visibility ("hidden"))) and multiple TOCs
On Mon, Jun 25, 2007 at 10:15:55AM -0400, Mark Mitchell wrote: > Daniel Jacobowitz wrote: > > >> I think the DECL_EXTERNAL case should go before the visibility checks in > >> default_binds_local_p_1. A DECL_EXTERNAL entity never binds locally. > > > > That isn't the meaning that most callers of this function want, > > however. They want same shared object, which is what it currently > > returns; that's what I think of when you ask me if something binds > > "locally", too... > > I dunno about "most", but at least some want to know "can this > definition be replaced by another one". For example, DECL_REPLACEABLE_P > and cgraph_variable_initializer_availability (which quite probably > should be using DECL_REPLACEABLE_P). For replacability the current definition is just fine. Weak functions must be assumed to be always replaceable and non-weak functions which are known to bind within the same executable resp. shared library are not replaceable - linker will issue error if two non-weak symbols with the same name are linked into the same executable resp. shared library and when linking a non-weak symbol and weak symbol the non-weak one will win. Jakub
Re: ppc64 __attribute__((visibility ("hidden"))) and multiple TOCs
Daniel Jacobowitz wrote: >> I think the DECL_EXTERNAL case should go before the visibility checks in >> default_binds_local_p_1. A DECL_EXTERNAL entity never binds locally. > > That isn't the meaning that most callers of this function want, > however. They want same shared object, which is what it currently > returns; that's what I think of when you ask me if something binds > "locally", too... I dunno about "most", but at least some want to know "can this definition be replaced by another one". For example, DECL_REPLACEABLE_P and cgraph_variable_initializer_availability (which quite probably should be using DECL_REPLACEABLE_P). So, perhaps binds_local_p needs to return a tri-state value. -- Mark Mitchell CodeSourcery [EMAIL PROTECTED] (650) 331-3385 x713
Re: ppc64 __attribute__((visibility ("hidden"))) and multiple TOCs
On Mon, Jun 25, 2007 at 07:06:01AM -0400, Jakub Jelinek wrote:
> Hi!
>
> extern void bar (void) __attribute__((visibility ("hidden")));
> void foo (void)
> {
> bar ();
> bar ();
> }
> compiled on ppc64-linux with -O2 -m64 -mminimal-toc
> leads to bl bar without nop in the following instruction
> and to sibling call.
> Now, when this together with bar's definition is linked
> into a big binary and foo and bar need to have different TOCs,
> ld issues error:
> sibling call optimization to `bar' does not allow automatic multiple TOCs;
> recompile with -mminimal-toc or -fno-optimize-sibling-calls, or make `foo'
> extern
Ouch.
> Shouldn't -mminimal-toc also forbid omitting nops if
> the target call isn't defined in the same file and forbid
> sibcalls to such functions? Or do we need another switch?
> In this case -mminimal-toc, nor -fno-optimize-sibling-calls
> helps ATM, only removing the hidden visibility.
A TARGET_BINDS_LOCAL_P for powerpc64-linux is the easiest way to fix
this. It should return false for any function not in the same file,
and *not* be controlled by -mminimal-toc. -mminimal-toc is actually
orthogonal to the linker's toc splitting at file boundaries.
--
Alan Modra
IBM OzLabs - Linux Technology Centre
Re: ppc64 __attribute__((visibility ("hidden"))) and multiple TOCs
On Mon, Jun 25, 2007 at 09:31:14AM -0400, Mark Mitchell wrote: > David Edelsohn wrote: > > > /* If defined in this object and visibility is not default, must be > > local. */ > > else if (DECL_VISIBILITY (exp) != VISIBILITY_DEFAULT) > > local_p = true; > > > > Why does binds_local_p return true for non-default visibility? > > I was just about to ask that. > > It's an intermediate case: more local than default visibility, but not > *that* local. If the function is defined, then it probably does bind > locally, in that nothing can come along later and replace the definition > we saw. > > I think the DECL_EXTERNAL case should go before the visibility checks in > default_binds_local_p_1. A DECL_EXTERNAL entity never binds locally. That isn't the meaning that most callers of this function want, however. They want same shared object, which is what it currently returns; that's what I think of when you ask me if something binds "locally", too... -- Daniel Jacobowitz CodeSourcery
Re: ppc64 __attribute__((visibility ("hidden"))) and multiple TOCs
> Mark Mitchell writes: Mark> I think the DECL_EXTERNAL case should go before the visibility checks in Mark> default_binds_local_p_1. A DECL_EXTERNAL entity never binds locally. Jakub, Do you want to follow up with a patch to change the ordering of tests in default_binds_local_p_1()? David
Re: ppc64 __attribute__((visibility ("hidden"))) and multiple TOCs
David Edelsohn wrote: > /* If defined in this object and visibility is not default, must be > local. */ > else if (DECL_VISIBILITY (exp) != VISIBILITY_DEFAULT) > local_p = true; > > Why does binds_local_p return true for non-default visibility? I was just about to ask that. It's an intermediate case: more local than default visibility, but not *that* local. If the function is defined, then it probably does bind locally, in that nothing can come along later and replace the definition we saw. I think the DECL_EXTERNAL case should go before the visibility checks in default_binds_local_p_1. A DECL_EXTERNAL entity never binds locally. -- Mark Mitchell CodeSourcery [EMAIL PROTECTED] (650) 331-3385 x713
Re: ppc64 __attribute__((visibility ("hidden"))) and multiple TOCs
On Mon, Jun 25, 2007 at 09:15:40AM -0400, David Edelsohn wrote: > Emitting a NOP depends on SYMBOL_FLAG_LOCAL. > >if (targetm.binds_local_p (decl)) > flags |= SYMBOL_FLAG_LOCAL; > > PPC64 uses the default binds_local_p() hook, default_binds_local_p_1(): > > /* If defined in this object and visibility is not default, must be > local. */ > else if (DECL_VISIBILITY (exp) != VISIBILITY_DEFAULT) > local_p = true; > > Why does binds_local_p return true for non-default visibility? Because local in binds_local_p's definition is "defined in the same executable or shared library" and that's true for hidden visibility. ppc64 in this case needs more strict definition of what local is though. Jakub
Re: ppc64 __attribute__((visibility ("hidden"))) and multiple TOCs
Emitting a NOP depends on SYMBOL_FLAG_LOCAL. if (targetm.binds_local_p (decl)) flags |= SYMBOL_FLAG_LOCAL; PPC64 uses the default binds_local_p() hook, default_binds_local_p_1(): /* If defined in this object and visibility is not default, must be local. */ else if (DECL_VISIBILITY (exp) != VISIBILITY_DEFAULT) local_p = true; Why does binds_local_p return true for non-default visibility? David
Re: ppc64 __attribute__((visibility ("hidden"))) and multiple TOCs
> Mark Mitchell writes: Mark> That would be my recommendation: limit optimizations that require a Mark> short branch to calls to functions in the same translation unit, not Mark> just in the same shared object. But, that's just my two cents; the Mark> Power maintainers might have a different take. The rs6000 port already does that, but the port thinks the call is local. David
Re: ppc64 __attribute__((visibility ("hidden"))) and multiple TOCs
Jakub Jelinek wrote:
> Hi!
>
> extern void bar (void) __attribute__((visibility ("hidden")));
> void foo (void)
> {
> bar ();
> bar ();
> }
> compiled on ppc64-linux with -O2 -m64 -mminimal-toc
> leads to bl bar without nop in the following instruction
> and to sibling call.
> Shouldn't -mminimal-toc also forbid omitting nops if
> the target call isn't defined in the same file and forbid
> sibcalls to such functions?
That would be my recommendation: limit optimizations that require a
short branch to calls to functions in the same translation unit, not
just in the same shared object. But, that's just my two cents; the
Power maintainers might have a different take.
--
Mark Mitchell
CodeSourcery
[EMAIL PROTECTED]
(650) 331-3385 x713
Re: ppc64 __attribute__((visibility ("hidden"))) and multiple TOCs
On Mon, Jun 25, 2007 at 07:32:48AM -0400, David Edelsohn wrote: > > Jakub Jelinek writes: > > Jakub> compiled on ppc64-linux with -O2 -m64 -mminimal-toc > Jakub> leads to bl bar without nop in the following instruction > Jakub> and to sibling call. > Jakub> Now, when this together with bar's definition is linked > Jakub> into a big binary and foo and bar need to have different TOCs, > > Just out of curiosity, are you using a version of GCC with or > without the section anchor support? Is the application still overrunning > the TOC with sectcion anchors? Without (4.1.2). The program being linked is frysk and has quite a lot of Java stuff linked into it. Jakub
Re: ppc64 __attribute__((visibility ("hidden"))) and multiple TOCs
On 6/25/07, David Edelsohn <[EMAIL PROTECTED]> wrote: Just out of curiosity, are you using a version of GCC with or without the section anchor support? Is the application still overrunning the TOC with sectcion anchors? I have access to programs that overflow the TOC even with section anchors (this is with a modified PPC64 linux ABI in that pointers are 32bits so it takes even more to overflow the TOC). I have noticed that the linker does not combine duplicated TOC entries across objects which makes it even easier to overflow the TOC. Thanks, Andrew Pinski
Re: ppc64 __attribute__((visibility ("hidden"))) and multiple TOCs
> Jakub Jelinek writes: Jakub> compiled on ppc64-linux with -O2 -m64 -mminimal-toc Jakub> leads to bl bar without nop in the following instruction Jakub> and to sibling call. Jakub> Now, when this together with bar's definition is linked Jakub> into a big binary and foo and bar need to have different TOCs, Just out of curiosity, are you using a version of GCC with or without the section anchor support? Is the application still overrunning the TOC with sectcion anchors? Thanks, David
