Re: [PATCH 6/6] [RS6000] inline plt call sequences

[Segher Boessenkool <segher at kernel dot crashing dot org>]

Hi Alan,

On Tue, Nov 13, 2018 at 11:23:43PM +1030, Alan Modra wrote:
> Finally, the point of the previous patches in this series, support for
> inline PLT calls, keyed off -fno-plt.  This emits code using new
> relocations that tie all insns in the sequence together, so that the
> linker can edit the sequence back to a direct call should the call
> target turn out to be local.  An example of ELFv2 code to call puts is
> as follows:
> 
>      .reloc .,R_PPC64_PLTSEQ,puts
>         std 2,24(1)
>      .reloc .,R_PPC64_PLT16_HA,puts
>         addis 12,2,0
>      .reloc .,R_PPC64_PLT16_LO_DS,puts
>         ld 12,0(12)
>      .reloc .,R_PPC64_PLTSEQ,puts
>         mtctr 12
>      .reloc .,R_PPC64_PLTCALL,puts
>         bctrl
>         ld 2,24(1)
> 
> "addis 12,2,puts@plt@ha" and "ld 12,puts@plt@l(12)" are also supported
> by the assembler.  gcc instead uses the explicit R_PPC64_PLT16_HA and
> R_PPC64_PLT16_LO_DS relocs because when the call is to __tls_get_addr
> an extra reloc is emitted at every place where one is shown above, to
> specify the __tls_get_addr arg.  The linker expects the extra reloc to
> come first.  .reloc enforces that ordering.
> 
> The patch also changes code emitted for longcalls if the assembler
> supports the new marker relocs, so that these too can be edited.  One
> side effect of longcalls using PLT16 relocs is that they can now be
> resolved lazily by ld.so.
> 
> I don't support lazy inline PLT calls for ELFv1, because ELFv1 would
> need barriers to reliably load both the function address and toc
> pointer from the PLT.  ELFv1 -fno-plt uses the longcall sequence
> instead, which isn't edited by GNU ld.

That all sounds great :-)  Has this been tested with older binutils, too?


> 	* config.in (HAVE_AS_PLTSEQ): Add.
> 	* config/rs6000/predicates.md (indirect_call_operand): New.
> 	* config/rs6000/rs6000-protos.h (rs6000_pltseq_template),
> 	(rs6000_sibcall_sysv): Declare.
> 	* config/rs6000/rs6000.c (init_cumulative_args): Set cookie
> 	CALL_LONG for -fno-plt.
> 	(print_operand <T, z, 0>): Handle UNSPEC_PLTSEQ.
> 	(rs6000_indirect_call_template_1): Emit .reloc directives for
> 	UNSPEC_PLTSEQ calls.
> 	(rs6000_pltseq_template): New function.
> 	(rs6000_longcall_ref): Add arg parameter.  Use PLT16 insns if
> 	relocs supported by assembler.  Move SYMBOL_REF test to callers.
> 	(rs6000_call_aix): Adjust rs6000_longcall_ref call.  Package
> 	insns in UNSPEC_PLTSEQ, preserving original func_desc.
> 	(rs6000_call_sysv): Likewise.
> 	(rs6000_sibcall_sysv): New function.
> 	* config/rs6000/rs6000.h (HAVE_AS_PLTSEQ): Provide default.
> 	* config/rs6000/rs6000.md (UNSPEC_PLTSEQ, UNSPEC_PLT16_HA,
> 	UNSPEC_PLT16_LO): New.
> 	(pltseq_tocsave, pltseq_plt16_ha, pltseq_plt16_lo, pltseq_mtctr): New.
> 	(call_indirect_nonlocal_sysv): Don't differentiate zero from non-zero
> 	cookie in constraints.  Test explicitly for flags in length attr.
> 	Handle unspec operand 1.
> 	(call_value_indirect_nonlocal_sysv): Likewise.
> 	(call_indirect_aix, call_value_indirect_aix): Handle unspec operand 1.
> 	(call_indirect_elfv2, call_value_indirect_elfv2): Likewise.
> 	(sibcall, sibcall_value): Use rs6000_sibcall_sysv.
> 	(sibcall_indirect_nonlocal_sysv): New pattern.
> 	(sibcall_value_indirect_nonlocal_sysv): Likewise.
> 	(sibcall_nonlocal_sysv, sibcall_value_nonlocal_sysv): Remove indirect
> 	call alternatives.
> 	* configure.ac: Check for gas plt sequence marker support.
> 	* configure: Regenerate.


> @@ -10727,10 +10727,20 @@ init_cumulative_args (CUMULATIVE_ARGS *cum, tree fntype,
>      cum->nargs_prototype = n_named_args;
>  
>    /* Check for a longcall attribute.  */
> -  if ((!fntype && rs6000_default_long_calls)
> -      || (fntype
> -	  && lookup_attribute ("longcall", TYPE_ATTRIBUTES (fntype))
> -	  && !lookup_attribute ("shortcall", TYPE_ATTRIBUTES (fntype))))
> +  if (((!fntype && rs6000_default_long_calls)
> +       || (fntype
> +	   && lookup_attribute ("longcall", TYPE_ATTRIBUTES (fntype))
> +	   && !lookup_attribute ("shortcall", TYPE_ATTRIBUTES (fntype))))
> +      || (DEFAULT_ABI != ABI_DARWIN
> +	  && !(fndecl
> +	       && !DECL_EXTERNAL (fndecl)
> +	       && !DECL_WEAK (fndecl)
> +	       && (*targetm.binds_local_p) (fndecl))
> +	  && (flag_plt
> +	      ? (fntype
> +		 && lookup_attribute ("noplt", TYPE_ATTRIBUTES (fntype)))
> +	      : !(fntype
> +		  && lookup_attribute ("plt", TYPE_ATTRIBUTES (fntype))))))
>      cum->call_cookie |= CALL_LONG;

Could you split this into a few cases?  Maybe with some extra locals.  It
is hard to understand the code as it is.

I mean like

  if (...)
    cum->call_cookie |= CALL_LONG;
  if (...)
    cum->call_cookie |= CALL_LONG;
  if (...)
    cum->call_cookie |= CALL_LONG;


> +(define_insn "*sibcall_indirect_nonlocal_sysv<mode>"

> +   (set (attr "length")
> +	(cond [(and (and (match_test "!rs6000_speculate_indirect_jumps")
> +			 (match_test "which_alternative != 1"))
> +		    (match_test "(INTVAL (operands[2]) & (CALL_V4_SET_FP_ARGS | CALL_V4_CLEAR_FP_ARGS))"))
> +		  (const_string "12")
> +	       (ior (and (match_test "!rs6000_speculate_indirect_jumps")
> +			 (match_test "which_alternative != 1"))
> +		   (match_test "(INTVAL (operands[2]) & (CALL_V4_SET_FP_ARGS | CALL_V4_CLEAR_FP_ARGS))"))
> +		  (const_string "8")]
> +	      (const_string "4")))])

Please use set_attr_alternative for these.


Looks fine modulo those nits.  Okay for trunk.  Thanks!


Segher