On 06/11/2013 10:03 AM, Jakub Jelinek wrote: > On Tue, Jun 11, 2013 at 09:56:28AM -0700, Richard Henderson wrote: >> I'm having trouble picking out what's supposed to be different between this >> function and default_binds_local_p_1. It appears to be the same set of >> tests, >> just rearranged into a possibly more efficient form requiring less runtime >> evaluation. > > What is different is > 1) weakref/ifunc handling without !TREE_PUBLIC (not sure about that though; > the old version would return true for !TREE_PUBLIC right away, but > e.g. default_binds_local_p_1 checks TREE_PUBLIC only after testing > for weakref)
This sounds wrong even for binds_local_p. (1a) The documentation says that weakrefs must be static, which implies that TREE_PUBLIC must be 0. I guess that assumption could be invalidated by manipulations in the code, but I don't see them in handle_weakref_attribute. If someone knows better, please chime in... (1b) One *can* declare a static ifunc, but that only means that the resolver isn't reachable from outside the translation unit. The resolver, being user code, is free to return absolutely anything, so we can't be sure about whence it binds. > 2) DECL_COMMON with non-default visibility - the old version would return > true, now it returns false unless linker plugin tells us the current > common was used > > Or do you think we should just do what we did before and just > handle the 1) and 2) cases explicitly in the decl_binds_to_current_def_p > function? This (2) sounds like something that we could handle in d_b_t_c_d_p without having to duplicate all of binds_local_p. r~
