[PATCH 2/6] [RS6000] rs6000_output_indirect_call

Bill Schmidt wschmidt@linux.ibm.com
Mon Nov 12 19:44:00 GMT 2018


On 11/6/18 11:37 PM, Alan Modra wrote:
> Like the last patch for external calls, now handle most assembly code
> for indirect calls in one place.  The patch also merges some insns,
> correcting some !rs6000_speculate_indirect_jumps cases branching to
> LR, which don't require a speculation barrier.
>
> 	* config/rs6000/rs6000-protos.h (rs6000_output_indirect_call): Declare.
> 	* config/rs6000/rs6000.c (rs6000_output_indirect_call): New function.
> 	* config/rs6000/rs6000.md (call_indirect_nonlocal_sysv): Use
> 	rs6000_output_indirect_call.
> 	(call_value_indirect_nonlocal_sysv, sibcall_nonlocal_sysv): Likewise.
> 	(call_indirect_aix, call_value_indirect_aix,
> 	call_indirect_elfv2, call_value_indirect_elfv2): Likewise, and
> 	handle both speculation and non-speculation cases.
> 	(call_indirect_aix_nospec, call_value_indirect_aix_nospec): Delete.
> 	(call_indirect_elfv2_nospec, call_value_indirect_elfv2_nospec): Delete.
>
> diff --git a/gcc/config/rs6000/rs6000-protos.h b/gcc/config/rs6000/rs6000-protos.h
> index f1a421dde16..493cfe6ba2b 100644
> --- a/gcc/config/rs6000/rs6000-protos.h
> +++ b/gcc/config/rs6000/rs6000-protos.h
> @@ -112,6 +112,7 @@ extern void rs6000_output_function_entry (FILE *, const char *);
>  extern void print_operand (FILE *, rtx, int);
>  extern void print_operand_address (FILE *, rtx);
>  extern const char *rs6000_output_call (rtx *, unsigned int, bool, const char *);
> +extern const char *rs6000_output_indirect_call (rtx *, unsigned int, bool);
>  extern enum rtx_code rs6000_reverse_condition (machine_mode,
>  					       enum rtx_code);
>  extern rtx rs6000_emit_eqne (machine_mode, rtx, rtx, rtx);
> diff --git a/gcc/config/rs6000/rs6000.c b/gcc/config/rs6000/rs6000.c
> index b22cae55a0d..bf1551746d5 100644
> --- a/gcc/config/rs6000/rs6000.c
> +++ b/gcc/config/rs6000/rs6000.c
> @@ -21411,6 +21411,69 @@ rs6000_output_call (rtx *operands, unsigned int fun, bool sibcall,
>    return str;
>  }
>
> +/* As above, for indirect calls.  */
> +
> +const char *
> +rs6000_output_indirect_call (rtx *operands, unsigned int fun, bool sibcall)
> +{
> +  /* -Wformat-overflow workaround, without which gcc thinks that %u
> +      might produce 10 digits.  FUN is 0 or 1 as of 2018-03.  */
> +  gcc_assert (fun <= 6);
> +
> +  static char str[144];
> +  const char *ptrload = TARGET_64BIT ? "d" : "wz";
> +
> +  bool speculate = (rs6000_speculate_indirect_jumps
> +		    || (REG_P (operands[fun])
> +			&& REGNO (operands[fun]) == LR_REGNO));

Wouldn't hurt to have a comment here, indicating that we only have to generate
inefficient, speculation-inhibiting code when speculation via the count cache
has been disabled by switch, and this doesn't apply to indirect calls via the
link register.  This changes behavior of the code from before, but appears to
be safe and more correct.

> +
> +  if (DEFAULT_ABI == ABI_AIX)
> +    {
> +      if (speculate)
> +	sprintf (str,
> +		 "l%s 2,%%%u\n\t"
> +		 "b%%T%ul\n\t"
> +		 "l%s 2,%%%u(1)",
> +		 ptrload, fun + 2, fun, ptrload, fun + 3);
> +      else
> +	sprintf (str,
> +		 "crset 2\n\t"
> +		 "l%s 2,%%%u\n\t"
> +		 "beq%%T%ul-\n\t"
> +		 "l%s 2,%%%u(1)",
> +		 ptrload, fun + 2, fun, ptrload, fun + 3);
> +    }
> +  else if (DEFAULT_ABI == ABI_ELFv2)
> +    {
> +      if (speculate)
> +	sprintf (str,
> +		 "b%%T%ul\n\t"
> +		 "l%s 2,%%%u(1)",
> +		 fun, ptrload, fun + 2);
> +      else
> +	sprintf (str,
> +		 "crset 2\n\t"
> +		 "beq%%T%ul-\n\t"
> +		 "l%s 2,%%%u(1)",
> +		 fun, ptrload, fun + 2);
> +    }
> +  else if (DEFAULT_ABI == ABI_V4)
> +    {
> +      if (speculate)
> +	sprintf (str,
> +		 "b%%T%u%s",
> +		 fun, "l" + sibcall);

It's not at all clear to me what {"l" + sibcall} is doing here.
Whatever it is, it's clever enough that it warrants a comment... :-)
Does adding "l" to false result in the null string?  Is that
standard?

> +      else
> +	sprintf (str,
> +		 "crset 2\n\t"
> +		 "beq%%T%u%s-%s",
> +		 fun, "l" + sibcall, sibcall ? "\n\tb $" : "");

And similar...

> +    }
> +  else
> +    gcc_unreachable ();
> +  return str;
> +}
> +
>  #if defined (HAVE_GAS_HIDDEN) && !TARGET_MACHO
>  /* Emit an assembler directive to set symbol visibility for DECL to
>     VISIBILITY_TYPE.  */
> diff --git a/gcc/config/rs6000/rs6000.md b/gcc/config/rs6000/rs6000.md
> index 52088fdfbdb..9d9e29d12eb 100644
> --- a/gcc/config/rs6000/rs6000.md
> +++ b/gcc/config/rs6000/rs6000.md
> @@ -10540,11 +10540,7 @@ (define_insn "*call_indirect_nonlocal_sysv<mode>"
>    else if (INTVAL (operands[2]) & CALL_V4_CLEAR_FP_ARGS)
>      output_asm_insn ("creqv 6,6,6", operands);
>
> -  if (rs6000_speculate_indirect_jumps
> -      || which_alternative == 1 || which_alternative == 3)
> -    return "b%T0l";
> -  else
> -    return "crset 2\;beq%T0l-";
> +  return rs6000_output_indirect_call (operands, 0, false);

Looks like this breaks Darwin?  This pattern matches for DEFAULT_ABI == ABI_DARWIN
but rs6000_output_indirect_call will hit gcc_unreachable() for that ABI.  

>  }
>    [(set_attr "type" "jmpreg,jmpreg,jmpreg,jmpreg")
>     (set_attr_alternative "length"
> @@ -10630,11 +10626,7 @@ (define_insn "*call_value_indirect_nonlocal_sysv<mode>"
>    else if (INTVAL (operands[3]) & CALL_V4_CLEAR_FP_ARGS)
>      output_asm_insn ("creqv 6,6,6", operands);
>
> -  if (rs6000_speculate_indirect_jumps
> -      || which_alternative == 1 || which_alternative == 3)
> -    return "b%T1l";
> -  else
> -    return "crset 2\;beq%T1l-";
> +  return rs6000_output_indirect_call (operands, 1, false);

Here also...

Otherwise this looks correct to me, though I can't approve it.  It's a nice cleanup!

Thanks,
Bill

>  }
>    [(set_attr "type" "jmpreg,jmpreg,jmpreg,jmpreg")
>     (set_attr_alternative "length"
> @@ -10765,21 +10757,16 @@ (define_insn "*call_indirect_aix<mode>"
>     (use (match_operand:P 2 "memory_operand" "<ptrm>,<ptrm>"))
>     (set (reg:P TOC_REGNUM) (unspec:P [(match_operand:P 3 "const_int_operand" "n,n")] UNSPEC_TOCSLOT))
>     (clobber (reg:P LR_REGNO))]
> -  "DEFAULT_ABI == ABI_AIX && rs6000_speculate_indirect_jumps"
> -  "<ptrload> 2,%2\;b%T0l\;<ptrload> 2,%3(1)"
> -  [(set_attr "type" "jmpreg")
> -   (set_attr "length" "12")])
> -
> -(define_insn "*call_indirect_aix<mode>_nospec"
> -  [(call (mem:SI (match_operand:P 0 "register_operand" "c,*l"))
> -	 (match_operand 1 "" "g,g"))
> -   (use (match_operand:P 2 "memory_operand" "<ptrm>,<ptrm>"))
> -   (set (reg:P TOC_REGNUM) (unspec:P [(match_operand:P 3 "const_int_operand" "n,n")] UNSPEC_TOCSLOT))
> -   (clobber (reg:P LR_REGNO))]
> -  "DEFAULT_ABI == ABI_AIX && !rs6000_speculate_indirect_jumps"
> -  "crset 2\;<ptrload> 2,%2\;beq%T0l-\;<ptrload> 2,%3(1)"
> +  "DEFAULT_ABI == ABI_AIX"
> +{
> +  return rs6000_output_indirect_call (operands, 0, false);
> +}
>    [(set_attr "type" "jmpreg")
> -   (set_attr "length" "16")])
> +   (set (attr "length")
> +	(if_then_else (and (match_test "!rs6000_speculate_indirect_jumps")
> +			   (match_test "which_alternative != 1"))
> +		      (const_string "16")
> +		      (const_string "12")))])
>
>  (define_insn "*call_value_indirect_aix<mode>"
>    [(set (match_operand 0 "" "")
> @@ -10788,22 +10775,16 @@ (define_insn "*call_value_indirect_aix<mode>"
>     (use (match_operand:P 3 "memory_operand" "<ptrm>,<ptrm>"))
>     (set (reg:P TOC_REGNUM) (unspec:P [(match_operand:P 4 "const_int_operand" "n,n")] UNSPEC_TOCSLOT))
>     (clobber (reg:P LR_REGNO))]
> -  "DEFAULT_ABI == ABI_AIX && rs6000_speculate_indirect_jumps"
> -  "<ptrload> 2,%3\;b%T1l\;<ptrload> 2,%4(1)"
> -  [(set_attr "type" "jmpreg")
> -   (set_attr "length" "12")])
> -
> -(define_insn "*call_value_indirect_aix<mode>_nospec"
> -  [(set (match_operand 0 "" "")
> -	(call (mem:SI (match_operand:P 1 "register_operand" "c,*l"))
> -	      (match_operand 2 "" "g,g")))
> -   (use (match_operand:P 3 "memory_operand" "<ptrm>,<ptrm>"))
> -   (set (reg:P TOC_REGNUM) (unspec:P [(match_operand:P 4 "const_int_operand" "n,n")] UNSPEC_TOCSLOT))
> -   (clobber (reg:P LR_REGNO))]
> -  "DEFAULT_ABI == ABI_AIX && !rs6000_speculate_indirect_jumps"
> -  "crset 2\;<ptrload> 2,%3\;beq%T1l-\;<ptrload> 2,%4(1)"
> +  "DEFAULT_ABI == ABI_AIX"
> +{
> +  return rs6000_output_indirect_call (operands, 1, false);
> +}
>    [(set_attr "type" "jmpreg")
> -   (set_attr "length" "16")])
> +   (set (attr "length")
> +	(if_then_else (and (match_test "!rs6000_speculate_indirect_jumps")
> +			   (match_test "which_alternative != 1"))
> +		      (const_string "16")
> +		      (const_string "12")))])
>
>  ;; Call to indirect functions with the ELFv2 ABI.
>  ;; Operand0 is the addresss of the function to call
> @@ -10814,21 +10795,16 @@ (define_insn "*call_indirect_elfv2<mode>"
>  	 (match_operand 1 "" "g,g"))
>     (set (reg:P TOC_REGNUM) (unspec:P [(match_operand:P 2 "const_int_operand" "n,n")] UNSPEC_TOCSLOT))
>     (clobber (reg:P LR_REGNO))]
> -  "DEFAULT_ABI == ABI_ELFv2 && rs6000_speculate_indirect_jumps"
> -  "b%T0l\;<ptrload> 2,%2(1)"
> -  [(set_attr "type" "jmpreg")
> -   (set_attr "length" "8")])
> -
> -;; Variant with deliberate misprediction.
> -(define_insn "*call_indirect_elfv2<mode>_nospec"
> -  [(call (mem:SI (match_operand:P 0 "register_operand" "c,*l"))
> -	 (match_operand 1 "" "g,g"))
> -   (set (reg:P TOC_REGNUM) (unspec:P [(match_operand:P 2 "const_int_operand" "n,n")] UNSPEC_TOCSLOT))
> -   (clobber (reg:P LR_REGNO))]
> -  "DEFAULT_ABI == ABI_ELFv2 && !rs6000_speculate_indirect_jumps"
> -  "crset 2\;beq%T0l-\;<ptrload> 2,%2(1)"
> +  "DEFAULT_ABI == ABI_ELFv2"
> +{
> +  return rs6000_output_indirect_call (operands, 0, false);
> +}
>    [(set_attr "type" "jmpreg")
> -   (set_attr "length" "12")])
> +   (set (attr "length")
> +	(if_then_else (and (match_test "!rs6000_speculate_indirect_jumps")
> +			   (match_test "which_alternative != 1"))
> +		      (const_string "12")
> +		      (const_string "8")))])
>
>  (define_insn "*call_value_indirect_elfv2<mode>"
>    [(set (match_operand 0 "" "")
> @@ -10836,22 +10812,16 @@ (define_insn "*call_value_indirect_elfv2<mode>"
>  	      (match_operand 2 "" "g,g")))
>     (set (reg:P TOC_REGNUM) (unspec:P [(match_operand:P 3 "const_int_operand" "n,n")] UNSPEC_TOCSLOT))
>     (clobber (reg:P LR_REGNO))]
> -  "DEFAULT_ABI == ABI_ELFv2 && rs6000_speculate_indirect_jumps"
> -  "b%T1l\;<ptrload> 2,%3(1)"
> -  [(set_attr "type" "jmpreg")
> -   (set_attr "length" "8")])
> -
> -; Variant with deliberate misprediction.
> -(define_insn "*call_value_indirect_elfv2<mode>_nospec"
> -  [(set (match_operand 0 "" "")
> -	(call (mem:SI (match_operand:P 1 "register_operand" "c,*l"))
> -	      (match_operand 2 "" "g,g")))
> -   (set (reg:P TOC_REGNUM) (unspec:P [(match_operand:P 3 "const_int_operand" "n,n")] UNSPEC_TOCSLOT))
> -   (clobber (reg:P LR_REGNO))]
> -  "DEFAULT_ABI == ABI_ELFv2 && !rs6000_speculate_indirect_jumps"
> -  "crset 2\;beq%T1l-\;<ptrload> 2,%3(1)"
> +  "DEFAULT_ABI == ABI_ELFv2"
> +{
> +  return rs6000_output_indirect_call (operands, 1, false);
> +}
>    [(set_attr "type" "jmpreg")
> -   (set_attr "length" "12")])
> +   (set (attr "length")
> +	(if_then_else (and (match_test "!rs6000_speculate_indirect_jumps")
> +			   (match_test "which_alternative != 1"))
> +		      (const_string "12")
> +		      (const_string "8")))])
>
>  ;; Call subroutine returning any type.
>  (define_expand "untyped_call"
> @@ -11020,13 +10990,7 @@ (define_insn "*sibcall_nonlocal_sysv<mode>"
>      output_asm_insn ("creqv 6,6,6", operands);
>
>    if (which_alternative >= 2)
> -    {
> -      if (rs6000_speculate_indirect_jumps)
> -	return "b%T0";
> -      else
> -	/* Can use CR0 since it is volatile across sibcalls.  */
> -	return "crset 2\;beq%T0-\;b $";
> -    }
> +    return rs6000_output_indirect_call (operands, 0, true);
>    else
>      return rs6000_output_call (operands, 0, true, "");
>  }
>



More information about the Gcc-patches mailing list