Sorry, I misnumbered this patch as #1/2 when first posting v3.
On May 28, 2024, Segher Boessenkool <seg...@kernel.crashing.org> wrote:
> Please don't (incorrectly!) line-wrap changelogs. Lines are 80
> characters wide, not 60 or 72 or whatever. 80. Indents are tabs that
> take 8 columns.
ACK. When was it bumped up to 80, BTW? It wasn't always like that, was
it? I've noticed that something seems to change my line width settings
in Emacs, but I must have missed that memo.
>> +/* Return the offset to be added to the label output after CALL_INSN
>> + to compute the address to be placed in DW_AT_call_return_pc. */
>> +
>> +static int
>> +rs6000_call_offset_return_label (rtx_insn *call_insn)
>> +{
>> + /* All rs6000 CALL_INSN output patterns start with a b or bl, always
> This isn't true. There is a crlogical insn before the bcl for sysv for
> example.
Hmm, if that's so, this function will have to look at the insn and
recognize those cases and use a different way to compute the offset.
However, I don't see any relevant uses of bcl in output patterns for
insns containing a call rtx. There are other uses in profiling and pic
loading and whatnot, but those don't get mentioned in the call graph in
debug info, and so they don't get this target hook called on them.
>> + we compute the offset
>> + back to the address of the call opcode proper, then add the
>> + constant 4 bytes, to get the address after that opcode. */
>> + return 4 - get_attr_length (call_insn);
> Please explain this magic, too -- in code preferably (so with a ? :
> maybe, but don't try to "optimise" that expression, let the compiler
> do that, it is much better at it anyway :-) )
There's neither optimization nor magic, it's literally what's written in
the comment quoted above: starting from the label at the end of the call
insn (that's what the caller offsets from, as in the documentation in
the actual #1/2), subtract the length (to get to the address of the call
opcode), and add 4 (to get past the call opcode). Ok, I've reordered
the two addends for an IMHO more readable expression, but that was all.
> Is that correct for all ABIs we support?
Back when I wrote this patchset, I went through all call opcodes I could
find in the md and .c files within rs6000/, and I was satisfied that it
covered what we had then, but I won't pretend to know all about all of
the ppc ABIs. I may have missed disguised call insns, too. I was
hoping some ppc maintainer, more familiar with the port than I am, would
catch any trouble on review and let me know about pitfalls and surprises
to watch out for.
> Even if so, it needs a lot more documentation than this.
I can write more documentation, but I'm at a loss as to what you're
hoping for. If you set clearer expectations, I'll be glad to oblige.
Thanks,
--
Alexandre Oliva, happy hacker https://FSFLA.org/blogs/lxo/
Free Software Activist GNU Toolchain Engineer
More tolerance and less prejudice are key for inclusion and diversity
Excluding neuro-others for not behaving ""normal"" is *not* inclusive
