On 17/11/15 12:00, Charles Baylis wrote: > On 16 November 2015 at 22:24, Kugan <kugan.vivekanandara...@linaro.org> wrote: > >> Please note that we have a sibcall from "broken" to "indirect". >> >> "direct" is variadic function so it is conforming to AAPCS base standard. >> >> "broken" is a non-variadic function and will return the value in >> floating point register for TARGET_HARD_FLOAT. Thus we should not be >> doing sibcall here. >> >> Attached patch fixes this. Bootstrap and regression testing is ongoing. >> Is this OK if no issues with the testing? > > Hi Kugan, > > It looks like this patch should work, but I think this is an overly > conservative fix, as it prevents all sibcalls for hardfloat targets. > It would be better if only variadic sibcalls were prevented on > hardfloat. You can check for variadic calls by checking the > function_type in the call expression (exp) using stdarg_p(). > > As an example to show how to test for variadic function calls, this is > how to test it in gdb: > > (gdb) b arm_function_ok_for_sibcall > Breakpoint 1 at 0xdae59c: file > /home/cbaylis/srcarea/gcc/gcc-git/gcc/config/arm/arm.c, line 6634. > (gdb) r > <snip>... > Breakpoint 1, arm_function_ok_for_sibcall (decl=0x0, exp=0x7ffff6104ce8) > at /home/cbaylis/srcarea/gcc/gcc-git/gcc/config/arm/arm.c:6634 > 6634 if (cfun->machine->sibcall_blocked) > (gdb) print debug_tree(exp) > <call_expr 0x7ffff6104ce8 > type <real_type 0x7ffff62835e8 double sizes-gimplified DF > size <integer_cst 0x7ffff626cf18 constant 64> > unit size <integer_cst 0x7ffff626cf30 constant 8> > align 64 symtab 0 alias set -1 canonical type 0x7ffff62835e8 > precision 64 > pointer_to_this <pointer_type 0x7ffff62837e0>> > side-effects addressable > fn <ssa_name 0x7ffff6275708 > type <pointer_type 0x7ffff60e9540 type <function_type 0x7ffff60e9348> > <snip>... > (gdb) print stdarg_p((tree)0x7ffff60e9348) <--- from function_type ^^^^^ > $2 = true >
Hi Charles, I wrongly thought that for indirect call we wouldn't know if it is variadic or not. I should check stdarg_p here. But we should really fix aapcs_allocate_return_reg as it is simply setting pcs_variant = arm_pcs_default without checking if this is stdarg_p. I will send an updated patch. Thanks, Kugan
