This is the mail archive of the
binutils@sourceware.org
mailing list for the binutils project.
Re: [PATCH] gold: Add linker relaxation of tail calls on sparc.
- From: Ian Lance Taylor <iant at google dot com>
- To: David Miller <davem at davemloft dot net>
- Cc: binutils at sourceware dot org
- Date: Tue, 24 Apr 2012 14:57:54 -0700
- Subject: Re: [PATCH] gold: Add linker relaxation of tail calls on sparc.
- References: <20120423.030821.684142687786581661.davem@davemloft.net>
David Miller <davem@davemloft.net> writes:
> * sparc.cc (Target_sparc::Relocate::relax_call): New function.
> (Target_sparc::Relocate::relocate): Call it for R_SPARC_WDISP30
> and R_SPARC_WPLT30.
> +template<int size, bool big_endian>
> +inline void
> +Target_sparc<size, big_endian>::Relocate::relax_call(
> + Target_sparc<size, big_endian>* target,
> + unsigned char* view,
> + const elfcpp::Rela<size, big_endian>& rela)
> +{
> + typedef typename elfcpp::Swap<32, true>::Valtype Insntype;
> + Insntype *wv = reinterpret_cast<Insntype*>(view);
> + Insntype call_insn, delay_insn, set_insn;
> + uint32_t op3, reg, off;
> +
> + // This code tries to relax call instructions that meet
> + // certain criteria.
> + //
> + // The first criteria is that the call must be such that the return
> + // address which the call writes into %o7 is unused. Two sequences
> + // meet this criteria, and are used to implement tail calls.
> + //
> + // Leaf function tail call:
> + //
> + // or %o7, %g0, %ANY_REG
> + // call FUNC
> + // or %ANY_REG, %g0, %o7
> + //
> + // Non-leaf function tail call:
> + //
> + // call FUNC
> + // restore
> + //
> + // The second criteria is that the call destination is close. If
> + // the displacement can fit in a signed 22-bit immediate field of a
> + // pre-V9 branch, we can do it. If we are generating a 64-bit
> + // object or a 32-bit object with ELF machine type EF_SPARC32PLUS,
> + // and the displacement fits in a signed 19-bit immediate field,
> + // then we can use a V9 branch.
> +
> + call_insn = elfcpp::Swap<32, true>::readval(wv);
> + delay_insn = elfcpp::Swap<32, true>::readval(wv + 1);
It is possible that reading delay_insn is reading past the end of the
section data. Consider passing in view_size from the caller and
verifying that rela.get_r_offset() + 8 <= view_size.
> + // Now check the branch distance. We are called after the
> + // call has been relocated, so we just have to peek at the
> + // offset contained in the instruction.
> + off = call_insn & 0x3fffffff;
> + if ((off & 0x3fe00000)
> + && (off & 0x3fe00000) != 0x3fe00000)
> + return;
I think this is a little clearer if you write the explicit != 0.
if ((off & 0x3fe00000) != 0
&& (off & 0x3fe00000) != 0x3fe00000)
This is OK with those changes.
Thanks.
Ian