On Thu, Jan 29, 2015 at 4:12 PM, Richard Biener <richard.guent...@gmail.com> wrote: > On Thu, Jan 29, 2015 at 4:05 PM, Richard Biener > <richard.guent...@gmail.com> wrote: >> On Thu, Jan 29, 2015 at 3:14 PM, Alan Modra <amo...@gmail.com> wrote: >>> Here is the completed patch. Bootstrapped and regression tested >>> powerpc64-linux. Is this OK to apply? If not now, then when gcc is >>> in stage1 again? >> >> It's ok to apply as it is a wrong-code fix. It would also be ok to backport >> if needed. >> >> Did you check whether other targets have function descriptors (seem >> to remember the Itanic here at least)? >> >> The middle-end changes are ok, I defer to David for the rs6000 changes. >> >> I am also curious of the .029t.ealias dump from >> >> gcc -O -fdump-tree-ealias-details-alias >> >> on (the nonsensical) >> >> void x(void); >> int y; >> int main() >> { >> void *p = x; >> p+=y; >> return *(int *)p; >> } > > Just checked myself with the host gcc 4.8 on gcc111. It looks like > function descriptors are not exposed in GIMPLE: > > void * p; > sizetype y.1; > int y.0; > int _6; > > <bb 2>: > y.0_3 = y; > y.1_4 = (sizetype) y.0_3; > # PT = > p_5 = x + y.1_4; > _6 = MEM[(int *)p_5]; > return _6; > > thus when the function address-taking happens in the same function > as the call there will be no aliasing as it points to nothing (points-to > doesn't track function decls). And if it's flowing in from the outside > you get "all globals". > > This means that you still will be able to create a testcase that is > miscompiled with exposing the address-taking to points-to analysis. > And it means that indirect calls to const functions are severely > pessimized (not that it matters?) as they effectively become pure calls.
Btw, with gimple_call_fntype available I was hoping to eventually get rid of the required conversion between ptr and function ptr: /* We need to take special care recursing to pointed-to types. */ else if (POINTER_TYPE_P (inner_type) && POINTER_TYPE_P (outer_type)) { /* Do not lose casts to function pointer types. */ if ((TREE_CODE (TREE_TYPE (outer_type)) == FUNCTION_TYPE || TREE_CODE (TREE_TYPE (outer_type)) == METHOD_TYPE) && !(TREE_CODE (TREE_TYPE (inner_type)) == FUNCTION_TYPE || TREE_CODE (TREE_TYPE (inner_type)) == METHOD_TYPE)) return false; /* We do not care for const qualification of the pointed-to types as const qualification has no semantic value to the middle-end. */ /* Otherwise pointers/references are equivalent. */ return true; which means call in the testcase in the PR would become (&opd) (dl_hwcap); just as I mentioned in my initial review (you can try removing the above check to see that). I wonder if we have targets where a conversion between type T and a function pointer generates actual code. Richard. > Richard. > >> Thanks, >> Richard. >> >>> gcc/ >>> PR target/64703 >>> * target.def (has_function_descriptors): New hook. >>> * doc/tm.texi.in: Add TARGET_HAS_FUNCTION_DESCRIPTORS. >>> * doc/tc.texi: Regenerate. >>> * tree-ssa-alias.c (pt_solution_includes_base): New function, >>> extracted from.. >>> (ref_maybe_used_by_call_p_1): ..here. Handle potential memory >>> reference by indirect calls on targets using function descriptors. >>> * config/rs6000/rs6000.c (TARGET_HAS_FUNCTION_DESCRIPTORS): Define. >>> (rs6000_has_function_descriptors): New function. >>> gcc/testsuite/ >>> * gcc.target/powerpc/pr64703.c: New. >>> >>> Index: gcc/target.def >>> =================================================================== >>> --- gcc/target.def (revision 220025) >>> +++ gcc/target.def (working copy) >>> @@ -2821,6 +2821,15 @@ The default value of this hook is based on target' >>> bool, (void), >>> default_has_ifunc_p) >>> >>> +/* True if target defines the address of a function as that of a >>> + function descriptor. */ >>> +DEFHOOK >>> +(has_function_descriptors, >>> + "True if target has function descriptors and defines the address\n\ >>> +of a function as that of a function descriptor.", >>> + bool, (void), >>> + hook_bool_void_false) >>> + >>> /* True if it is OK to do sibling call optimization for the specified >>> call expression EXP. DECL will be the called function, or NULL if >>> this is an indirect call. */ >>> Index: gcc/doc/tm.texi.in >>> =================================================================== >>> --- gcc/doc/tm.texi.in (revision 220025) >>> +++ gcc/doc/tm.texi.in (working copy) >>> @@ -8175,6 +8175,8 @@ and the associated definitions of those functions. >>> >>> @hook TARGET_HAS_IFUNC_P >>> >>> +@hook TARGET_HAS_FUNCTION_DESCRIPTORS >>> + >>> @hook TARGET_ATOMIC_ALIGN_FOR_MODE >>> >>> @hook TARGET_ATOMIC_ASSIGN_EXPAND_FENV >>> Index: gcc/doc/tm.texi >>> =================================================================== >>> --- gcc/doc/tm.texi (revision 220025) >>> +++ gcc/doc/tm.texi (working copy) >>> @@ -11510,6 +11510,11 @@ The support includes the assembler, linker and dyn >>> The default value of this hook is based on target's libc. >>> @end deftypefn >>> >>> +@deftypefn {Target Hook} bool TARGET_HAS_FUNCTION_DESCRIPTORS (void) >>> +True if target has function descriptors and defines the address >>> +of a function as that of a function descriptor. >>> +@end deftypefn >>> + >>> @deftypefn {Target Hook} {unsigned int} TARGET_ATOMIC_ALIGN_FOR_MODE >>> (machine_mode @var{mode}) >>> If defined, this function returns an appropriate alignment in bits for an >>> atomic object of machine_mode @var{mode}. If 0 is returned then the >>> default alignment for the specified mode is used. >>> @end deftypefn >>> Index: gcc/tree-ssa-alias.c >>> =================================================================== >>> --- gcc/tree-ssa-alias.c (revision 220025) >>> +++ gcc/tree-ssa-alias.c (working copy) >>> @@ -1532,6 +1532,25 @@ refs_output_dependent_p (tree store1, tree store2) >>> return refs_may_alias_p_1 (&r1, &r2, false); >>> } >>> >>> +/* Return true if the points-to solution *PT includes the object BASE. */ >>> + >>> +static bool >>> +pt_solution_includes_base (struct pt_solution *pt, tree base) >>> +{ >>> + if (DECL_P (base)) >>> + return pt_solution_includes (pt, base); >>> + >>> + if ((TREE_CODE (base) == MEM_REF >>> + || TREE_CODE (base) == TARGET_MEM_REF) >>> + && TREE_CODE (TREE_OPERAND (base, 0)) == SSA_NAME) >>> + { >>> + struct ptr_info_def *pi = SSA_NAME_PTR_INFO (TREE_OPERAND (base, 0)); >>> + if (pi) >>> + return pt_solutions_intersect (pt, &pi->pt); >>> + } >>> + return true; >>> +} >>> + >>> /* If the call CALL may use the memory reference REF return true, >>> otherwise return false. */ >>> >>> @@ -1542,6 +1561,22 @@ ref_maybe_used_by_call_p_1 (gcall *call, ao_ref *r >>> unsigned i; >>> int flags = gimple_call_flags (call); >>> >>> + callee = gimple_call_fn (call); >>> + if (callee && TREE_CODE (callee) == SSA_NAME >>> + && targetm.has_function_descriptors ()) >>> + { >>> + /* Handle indirect call. When a target defines the address of a >>> + function as that of a function descriptor, then dereferencing >>> + a function pointer implicitly references memory. */ >>> + struct ptr_info_def *pi = SSA_NAME_PTR_INFO (callee); >>> + if (pi) >>> + { >>> + base = ao_ref_base (ref); >>> + if (pt_solution_includes_base (&pi->pt, base)) >>> + return true; >>> + } >>> + } >>> + >>> /* Const functions without a static chain do not implicitly use memory. >>> */ >>> if (!gimple_call_chain (call) >>> && (flags & (ECF_CONST|ECF_NOVOPS))) >>> @@ -1564,7 +1599,7 @@ ref_maybe_used_by_call_p_1 (gcall *call, ao_ref *r >>> && !is_global_var (base)) >>> goto process_args; >>> >>> - callee = gimple_call_fndecl (call); >>> + callee = gimple_call_addr_fndecl (callee); >>> >>> /* Handle those builtin functions explicitly that do not act as >>> escape points. See tree-ssa-structalias.c:find_func_aliases >>> @@ -1803,23 +1838,7 @@ ref_maybe_used_by_call_p_1 (gcall *call, ao_ref *r >>> } >>> >>> /* Check if the base variable is call-used. */ >>> - if (DECL_P (base)) >>> - { >>> - if (pt_solution_includes (gimple_call_use_set (call), base)) >>> - return true; >>> - } >>> - else if ((TREE_CODE (base) == MEM_REF >>> - || TREE_CODE (base) == TARGET_MEM_REF) >>> - && TREE_CODE (TREE_OPERAND (base, 0)) == SSA_NAME) >>> - { >>> - struct ptr_info_def *pi = SSA_NAME_PTR_INFO (TREE_OPERAND (base, 0)); >>> - if (!pi) >>> - return true; >>> - >>> - if (pt_solutions_intersect (gimple_call_use_set (call), &pi->pt)) >>> - return true; >>> - } >>> - else >>> + if (pt_solution_includes_base (gimple_call_use_set (call), base)) >>> return true; >>> >>> /* Inspect call arguments for passed-by-value aliases. */ >>> Index: gcc/config/rs6000/rs6000.c >>> =================================================================== >>> --- gcc/config/rs6000/rs6000.c (revision 220025) >>> +++ gcc/config/rs6000/rs6000.c (working copy) >>> @@ -1490,6 +1490,9 @@ static const struct attribute_spec rs6000_attribut >>> #undef TARGET_ASM_CAN_OUTPUT_MI_THUNK >>> #define TARGET_ASM_CAN_OUTPUT_MI_THUNK >>> hook_bool_const_tree_hwi_hwi_const_tree_true >>> >>> +#undef TARGET_HAS_FUNCTION_DESCRIPTORS >>> +#define TARGET_HAS_FUNCTION_DESCRIPTORS rs6000_has_function_descriptors >>> + >>> #undef TARGET_FUNCTION_OK_FOR_SIBCALL >>> #define TARGET_FUNCTION_OK_FOR_SIBCALL rs6000_function_ok_for_sibcall >>> >>> @@ -22099,6 +22102,14 @@ rs6000_return_addr (int count, rtx frame) >>> return get_hard_reg_initial_val (Pmode, LR_REGNO); >>> } >>> >>> +/* Return true if we use function descriptors. */ >>> + >>> +static bool >>> +rs6000_has_function_descriptors (void) >>> +{ >>> + return DEFAULT_ABI == ABI_AIX; >>> +} >>> + >>> /* Say whether a function is a candidate for sibcall handling or not. */ >>> >>> static bool >>> Index: gcc/testsuite/gcc.target/powerpc/pr64703.c >>> =================================================================== >>> --- gcc/testsuite/gcc.target/powerpc/pr64703.c (revision 0) >>> +++ gcc/testsuite/gcc.target/powerpc/pr64703.c (working copy) >>> @@ -0,0 +1,36 @@ >>> +/* { dg-do compile { target { powerpc*-*-* && lp64 } } } */ >>> +/* { dg-skip-if "" { powerpc*-*-darwin* } { "*" } { "" } } */ >>> +/* { dg-options "-O2 -mabi=elfv1" } */ >>> +/* { dg-final { scan-assembler "std .\*,112\\(1\\)" } } */ >>> +/* { dg-final { scan-assembler "std .\*,120\\(1\\)" } } */ >>> +/* { dg-final { scan-assembler "std .\*,128\\(1\\)" } } */ >>> +/* { dg-final { scan-assembler "addi .\*,1,112" } } */ >>> + >>> +/* Testcase taken from glibc, powerpc64 dl-machine.h. */ >>> + >>> +typedef struct { >>> + unsigned long fd_func; >>> + unsigned long fd_toc; >>> + unsigned long fd_aux; >>> +} Elf64_FuncDesc; >>> + >>> +extern unsigned long dl_hwcap; >>> + >>> +unsigned long >>> +resolve_ifunc (unsigned long value, unsigned long adjust) >>> +{ >>> + Elf64_FuncDesc opd; >>> + >>> + if (adjust) >>> + { >>> + Elf64_FuncDesc *func = (Elf64_FuncDesc *) value; >>> + opd.fd_func = func->fd_func + adjust; >>> + opd.fd_toc = func->fd_toc + adjust; >>> + opd.fd_aux = func->fd_aux; >>> + value = (unsigned long) &opd; >>> + } >>> +#if 0 >>> + __asm__ ("#%0" : : "r" (value)); >>> +#endif >>> + return ((unsigned long (*) (unsigned long)) value) (dl_hwcap); >>> +} >>> >>> -- >>> Alan Modra >>> Australia Development Lab, IBM