On Fri, Aug 18, 2023 at 03:53:51PM +0200, Jose E. Marchesi via Gcc-patches
wrote:
> --- a/gcc/final.cc
> +++ b/gcc/final.cc
> @@ -815,6 +815,8 @@ make_pass_compute_alignments (gcc::context *ctxt)
> reorg.cc, since the branch splitting exposes new instructions with delay
> slots. */
>
> +static rtx call_from_call_insn (rtx_call_insn *insn);
> +
I'd say the forward declaration should go before the function comment, so
that it is clear the function comment talks about shorten_branches.
> void
> shorten_branches (rtx_insn *first)
> {
> @@ -850,6 +852,20 @@ shorten_branches (rtx_insn *first)
> for (insn = get_insns (), i = 1; insn; insn = NEXT_INSN (insn))
> {
> INSN_SHUID (insn) = i++;
> +
> + /* If this is a `call' instruction implementing a libcall,
> + and this machine requires an external definition for library
> + functions, write one out. */
> + if (CALL_P (insn))
> + {
> + rtx x = call_from_call_insn (dyn_cast <rtx_call_insn *> (insn));
> + x = XEXP (x, 0);
> + if (x && MEM_P (x)
When all conditions don't fit on one line, each && condition should be on
its own line.
> + && SYMBOL_REF_P (XEXP (x, 0))
> + && SYMBOL_REF_LIBCALL (XEXP (x, 0)))
> + assemble_external_libcall (XEXP (x, 0));
> + }
This won't work if target can't use a direct call instruction.
Consider
__int128 a, b; void foo () { a = a / b; }
on x86_64-linux. With just -O2, the above works fine, with
-O2 -mcmodel=large it will not, the call is indirect, but at least one has
REG_CALL_DECL note that could be used as fallback to the above.
And with -O0 -mcmodel=large because flag_ipa_ra is false REG_CALL_DECL isn't
emitted at all.
So, perhaps you could emit the REG_CALL_DECL note even if !flag_ipa_ra
when SYMBOL_REF_LIBCALL is set?
> +
> if (INSN_P (insn))
> continue;
>
> diff --git a/gcc/rtl.h b/gcc/rtl.h
> index e1c51156f90..945e3267a34 100644
> --- a/gcc/rtl.h
> +++ b/gcc/rtl.h
> @@ -402,6 +402,8 @@ struct GTY((desc("0"), tag("0"),
> 1 in a VALUE or DEBUG_EXPR is NO_LOC_P in var-tracking.cc.
> Dumped as "/i" in RTL dumps. */
> unsigned return_val : 1;
> + /* 1 in a SYMBOL_REF if it is the target of a libcall. */
> + unsigned is_libcall : 1;
This is wrong. struct rtx_def is carefully designed such that
it has 16 + 8 + 8 bits before the union is 32-bit and then
flexible array member which sometimes needs 64-bit alignment.
So, the above change would grow all RTX by 64 bits.
I think jump, call and in_struct are unused on SYMBOL_REF, so just
document the new meaning of the chosen bit for SYMBOL_REF.
> @@ -2734,6 +2736,10 @@ do {
> \
> #define SYMBOL_REF_USED(RTX) \
> (RTL_FLAG_CHECK1 ("SYMBOL_REF_USED", (RTX), SYMBOL_REF)->used)
>
> +/* 1 if RTX is a symbol_ref that represents a libcall target. */
> +#define SYMBOL_REF_LIBCALL(RTX) \
> + (RTL_FLAG_CHECK1 ("SYMBOL_REF_LIBCALL", (RTX), SYMBOL_REF)->is_libcall)
And change is_libcall to the selected bit-field here.
Jakub
