From: "Richard Earnshaw (lists)" <Richard.Earnshaw@arm.com>
To: Renlin Li <renlin.li@foss.arm.com>,
"gcc-patches@gcc.gnu.org" <gcc-patches@gcc.gnu.org>,
James Greenhalgh <James.Greenhalgh@arm.com>,
Richard Henderson <rth@redhat.com>,
Wilco Dijkstra <Wilco.Dijkstra@arm.com>,
Ramana Radhakrishnan <Ramana.Radhakrishnan@arm.com>
Subject: Re: [PATCH][AARCH64]Simplify call, call_value, sibcall, sibcall_value patterns.
Date: Tue, 02 May 2017 13:02:00 -0000 [thread overview]
Message-ID: <2a3dfac4-372e-9744-7680-809be729daad@arm.com> (raw)
In-Reply-To: <5840444E.4020200@foss.arm.com>
On 01/12/16 15:39, Renlin Li wrote:
> Hi all,
>
> This patch refactors the code used in call, call_value, sibcall,
> sibcall_value expanders.
>
> Before the change, the logic is following:
>
> call expander --> call_internal --> call_reg/call_symbol
> call_vlaue expander --> call_value_internal -->
> call_value_reg/call_value_symbol
>
> sibcall expander --> sibcall_internal --> sibcall_insn
> sibcall_value expander --> sibcall_value_internal --> sibcall_value_insn
>
> After the change, the logic is simplified into:
>
> call expander --> aarch64_expand_call() --> call_insn
> call_value expander --> aarch64_expand_call() --> call_value_insn
>
> sibcall expander --> aarch64_expand_call() --> sibcall_insn
> sibcall_value expander --> aarch64_expand_call() --> sibcall_value_insn
>
> The code are factored out from each expander into aarch64_expand_call ().
>
> This also fixes the two issues Richard Henderson suggests in comments 8:
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64971
>
> aarch64-none-elf regression test Okay, aarch64-linux bootstrap Okay.
> Okay for trunk?
>
> Regards,
> Renlin Li
>
>
> gcc/ChangeLog:
>
> 2016-12-01 Renlin Li <renlin.li@arm.com>
>
> * config/aarch64/aarch64-protos.h (aarch64_expand_call): Declare.
> * config/aarch64/aarch64.c (aarch64_expand_call): Define.
> * config/aarch64/constraints.md (Usf): Add long call check.
> * config/aarch64/aarch64.md (call): Use aarch64_expand_call.
> (call_value): Likewise.
> (sibcall): Likewise.
> (sibcall_value): Likewise.
> (call_insn): New.
> (call_value_insn): New.
> (sibcall_insn): Update rtx pattern.
> (sibcall_value_insn): Likewise.
> (call_internal): Remove.
> (call_value_internal): Likewise.
> (sibcall_internal): Likewise.
> (sibcall_value_internal): Likewise.
> (call_reg): Likewise.
> (call_symbol): Likewise.
> (call_value_reg): Likewise.
> (call_value_symbol): Likewise.
>
>
> new.diff
>
>
> diff --git a/gcc/config/aarch64/aarch64-protos.h b/gcc/config/aarch64/aarch64-protos.h
> index 7f67f14..3a5babb 100644
> --- a/gcc/config/aarch64/aarch64-protos.h
> +++ b/gcc/config/aarch64/aarch64-protos.h
> @@ -305,6 +305,7 @@ bool aarch64_const_vec_all_same_int_p (rtx, HOST_WIDE_INT);
> bool aarch64_constant_address_p (rtx);
> bool aarch64_emit_approx_div (rtx, rtx, rtx);
> bool aarch64_emit_approx_sqrt (rtx, rtx, bool);
> +void aarch64_expand_call (rtx, rtx, bool);
> bool aarch64_expand_movmem (rtx *);
> bool aarch64_float_const_zero_rtx_p (rtx);
> bool aarch64_function_arg_regno_p (unsigned);
> diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c
> index 68a3380..c313cf5 100644
> --- a/gcc/config/aarch64/aarch64.c
> +++ b/gcc/config/aarch64/aarch64.c
> @@ -4343,6 +4343,51 @@ aarch64_fixed_condition_code_regs (unsigned int *p1, unsigned int *p2)
> return true;
> }
>
> +/* This function is used by the call expanders of the machine description.
> + RESULT is the register in which the result is returned. It's NULL for
> + "call" and "sibcall".
> + MEM is the location of the function call.
> + SIBCALL indicates whether this function call is normal call or sibling call.
> + It will generate different pattern accordingly. */
> +
> +void
> +aarch64_expand_call (rtx result, rtx mem, bool sibcall)
> +{
> + rtx call, callee, tmp;
> + rtvec vec;
> + machine_mode mode;
> +
> + gcc_assert (MEM_P (mem));
> + callee = XEXP (mem, 0);
> + mode = GET_MODE (callee);
> + gcc_assert (mode == Pmode);
> +
> + /* Decide if we should generate indirect calls by loading the
> + 64-bit address of the callee into a register before performing
Drop '64-bit'. This code should also work for ILP32, where the
addresses are 32-bit.
> + the branch-and-link. */
> +
> + if (GET_CODE (callee) == SYMBOL_REF
Use SYMBOL_REF_P.
OK with those changes.
R.
> + ? (aarch64_is_long_call_p (callee)
> + || aarch64_is_noplt_call_p (callee))
> + : !REG_P (callee))
> + XEXP (mem, 0) = force_reg (mode, callee);
> +
> + call = gen_rtx_CALL (VOIDmode, mem, const0_rtx);
> +
> + if (result != NULL_RTX)
> + call = gen_rtx_SET (result, call);
> +
> + if (sibcall)
> + tmp = ret_rtx;
> + else
> + tmp = gen_rtx_CLOBBER (VOIDmode, gen_rtx_REG (Pmode, LR_REGNUM));
> +
> + vec = gen_rtvec (2, call, tmp);
> + call = gen_rtx_PARALLEL (VOIDmode, vec);
> +
> + aarch64_emit_call_insn (call);
> +}
> +
> /* Emit call insn with PAT and do aarch64-specific handling. */
>
> void
> diff --git a/gcc/config/aarch64/aarch64.md b/gcc/config/aarch64/aarch64.md
> index bc6d8a2..5682686 100644
> --- a/gcc/config/aarch64/aarch64.md
> +++ b/gcc/config/aarch64/aarch64.md
> @@ -718,12 +718,6 @@
> ;; Subroutine calls and sibcalls
> ;; -------------------------------------------------------------------
>
> -(define_expand "call_internal"
> - [(parallel [(call (match_operand 0 "memory_operand" "")
> - (match_operand 1 "general_operand" ""))
> - (use (match_operand 2 "" ""))
> - (clobber (reg:DI LR_REGNUM))])])
> -
> (define_expand "call"
> [(parallel [(call (match_operand 0 "memory_operand" "")
> (match_operand 1 "general_operand" ""))
> @@ -732,57 +726,22 @@
> ""
> "
> {
> - rtx callee, pat;
> -
> - /* In an untyped call, we can get NULL for operand 2. */
> - if (operands[2] == NULL)
> - operands[2] = const0_rtx;
> -
> - /* Decide if we should generate indirect calls by loading the
> - 64-bit address of the callee into a register before performing
> - the branch-and-link. */
> - callee = XEXP (operands[0], 0);
> - if (GET_CODE (callee) == SYMBOL_REF
> - ? (aarch64_is_long_call_p (callee)
> - || aarch64_is_noplt_call_p (callee))
> - : !REG_P (callee))
> - XEXP (operands[0], 0) = force_reg (Pmode, callee);
> -
> - pat = gen_call_internal (operands[0], operands[1], operands[2]);
> - aarch64_emit_call_insn (pat);
> + aarch64_expand_call (NULL_RTX, operands[0], false);
> DONE;
> }"
> )
>
> -(define_insn "*call_reg"
> - [(call (mem:DI (match_operand:DI 0 "register_operand" "r"))
> +(define_insn "*call_insn"
> + [(call (mem:DI (match_operand:DI 0 "aarch64_call_insn_operand" "r, Usf"))
> (match_operand 1 "" ""))
> - (use (match_operand 2 "" ""))
> (clobber (reg:DI LR_REGNUM))]
> ""
> - "blr\\t%0"
> - [(set_attr "type" "call")]
> -)
> -
> -(define_insn "*call_symbol"
> - [(call (mem:DI (match_operand:DI 0 "" ""))
> - (match_operand 1 "" ""))
> - (use (match_operand 2 "" ""))
> - (clobber (reg:DI LR_REGNUM))]
> - "GET_CODE (operands[0]) == SYMBOL_REF
> - && !aarch64_is_long_call_p (operands[0])
> - && !aarch64_is_noplt_call_p (operands[0])"
> - "bl\\t%a0"
> - [(set_attr "type" "call")]
> + "@
> + blr\\t%0
> + bl\\t%a0"
> + [(set_attr "type" "call, call")]
> )
>
> -(define_expand "call_value_internal"
> - [(parallel [(set (match_operand 0 "" "")
> - (call (match_operand 1 "memory_operand" "")
> - (match_operand 2 "general_operand" "")))
> - (use (match_operand 3 "" ""))
> - (clobber (reg:DI LR_REGNUM))])])
> -
> (define_expand "call_value"
> [(parallel [(set (match_operand 0 "" "")
> (call (match_operand 1 "memory_operand" "")
> @@ -792,60 +751,23 @@
> ""
> "
> {
> - rtx callee, pat;
> -
> - /* In an untyped call, we can get NULL for operand 3. */
> - if (operands[3] == NULL)
> - operands[3] = const0_rtx;
> -
> - /* Decide if we should generate indirect calls by loading the
> - 64-bit address of the callee into a register before performing
> - the branch-and-link. */
> - callee = XEXP (operands[1], 0);
> - if (GET_CODE (callee) == SYMBOL_REF
> - ? (aarch64_is_long_call_p (callee)
> - || aarch64_is_noplt_call_p (callee))
> - : !REG_P (callee))
> - XEXP (operands[1], 0) = force_reg (Pmode, callee);
> -
> - pat = gen_call_value_internal (operands[0], operands[1], operands[2],
> - operands[3]);
> - aarch64_emit_call_insn (pat);
> + aarch64_expand_call (operands[0], operands[1], false);
> DONE;
> }"
> )
>
> -(define_insn "*call_value_reg"
> +(define_insn "*call_value_insn"
> [(set (match_operand 0 "" "")
> - (call (mem:DI (match_operand:DI 1 "register_operand" "r"))
> + (call (mem:DI (match_operand:DI 1 "aarch64_call_insn_operand" "r, Usf"))
> (match_operand 2 "" "")))
> - (use (match_operand 3 "" ""))
> (clobber (reg:DI LR_REGNUM))]
> ""
> - "blr\\t%1"
> - [(set_attr "type" "call")]
> -
> -)
> -
> -(define_insn "*call_value_symbol"
> - [(set (match_operand 0 "" "")
> - (call (mem:DI (match_operand:DI 1 "" ""))
> - (match_operand 2 "" "")))
> - (use (match_operand 3 "" ""))
> - (clobber (reg:DI LR_REGNUM))]
> - "GET_CODE (operands[1]) == SYMBOL_REF
> - && !aarch64_is_long_call_p (operands[1])
> - && !aarch64_is_noplt_call_p (operands[1])"
> - "bl\\t%a1"
> - [(set_attr "type" "call")]
> + "@
> + blr\\t%1
> + bl\\t%a1"
> + [(set_attr "type" "call, call")]
> )
>
> -(define_expand "sibcall_internal"
> - [(parallel [(call (match_operand 0 "memory_operand" "")
> - (match_operand 1 "general_operand" ""))
> - (return)
> - (use (match_operand 2 "" ""))])])
> -
> (define_expand "sibcall"
> [(parallel [(call (match_operand 0 "memory_operand" "")
> (match_operand 1 "general_operand" ""))
> @@ -853,29 +775,11 @@
> (use (match_operand 2 "" ""))])]
> ""
> {
> - rtx pat;
> - rtx callee = XEXP (operands[0], 0);
> - if (!REG_P (callee)
> - && ((GET_CODE (callee) != SYMBOL_REF)
> - || aarch64_is_noplt_call_p (callee)))
> - XEXP (operands[0], 0) = force_reg (Pmode, callee);
> -
> - if (operands[2] == NULL_RTX)
> - operands[2] = const0_rtx;
> -
> - pat = gen_sibcall_internal (operands[0], operands[1], operands[2]);
> - aarch64_emit_call_insn (pat);
> + aarch64_expand_call (NULL_RTX, operands[0], true);
> DONE;
> }
> )
>
> -(define_expand "sibcall_value_internal"
> - [(parallel [(set (match_operand 0 "" "")
> - (call (match_operand 1 "memory_operand" "")
> - (match_operand 2 "general_operand" "")))
> - (return)
> - (use (match_operand 3 "" ""))])])
> -
> (define_expand "sibcall_value"
> [(parallel [(set (match_operand 0 "" "")
> (call (match_operand 1 "memory_operand" "")
> @@ -884,19 +788,7 @@
> (use (match_operand 3 "" ""))])]
> ""
> {
> - rtx pat;
> - rtx callee = XEXP (operands[1], 0);
> - if (!REG_P (callee)
> - && ((GET_CODE (callee) != SYMBOL_REF)
> - || aarch64_is_noplt_call_p (callee)))
> - XEXP (operands[1], 0) = force_reg (Pmode, callee);
> -
> - if (operands[3] == NULL_RTX)
> - operands[3] = const0_rtx;
> -
> - pat = gen_sibcall_value_internal (operands[0], operands[1], operands[2],
> - operands[3]);
> - aarch64_emit_call_insn (pat);
> + aarch64_expand_call (operands[0], operands[1], true);
> DONE;
> }
> )
> @@ -904,8 +796,7 @@
> (define_insn "*sibcall_insn"
> [(call (mem:DI (match_operand:DI 0 "aarch64_call_insn_operand" "Ucs, Usf"))
> (match_operand 1 "" ""))
> - (return)
> - (use (match_operand 2 "" ""))]
> + (return)]
> "SIBLING_CALL_P (insn)"
> "@
> br\\t%0
> @@ -918,8 +809,7 @@
> (call (mem:DI
> (match_operand:DI 1 "aarch64_call_insn_operand" "Ucs, Usf"))
> (match_operand 2 "" "")))
> - (return)
> - (use (match_operand 3 "" ""))]
> + (return)]
> "SIBLING_CALL_P (insn)"
> "@
> br\\t%1
> diff --git a/gcc/config/aarch64/constraints.md b/gcc/config/aarch64/constraints.md
> index 7a2847a..bd28386 100644
> --- a/gcc/config/aarch64/constraints.md
> +++ b/gcc/config/aarch64/constraints.md
> @@ -118,7 +118,8 @@
> (define_constraint "Usf"
> "@internal Usf is a symbol reference under the context where plt stub allowed."
> (and (match_code "symbol_ref")
> - (match_test "!aarch64_is_noplt_call_p (op)")))
> + (match_test "!(aarch64_is_noplt_call_p (op)
> + || aarch64_is_long_call_p (op))")))
>
> (define_constraint "UsM"
> "@internal
>
next prev parent reply other threads:[~2017-05-02 12:54 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-12-01 15:40 Renlin Li
2017-05-02 13:02 ` Richard Earnshaw (lists) [this message]
2017-05-15 11:33 ` Renlin Li
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=2a3dfac4-372e-9744-7680-809be729daad@arm.com \
--to=richard.earnshaw@arm.com \
--cc=James.Greenhalgh@arm.com \
--cc=Ramana.Radhakrishnan@arm.com \
--cc=Wilco.Dijkstra@arm.com \
--cc=gcc-patches@gcc.gnu.org \
--cc=renlin.li@foss.arm.com \
--cc=rth@redhat.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).