Hi Christophe.
> On 23 Nov 2023, at 09:02, Christophe Lyon <christophe.l...@linaro.org> wrote:
>
> Hi Iain,
>
> On Mon, 6 Nov 2023 at 11:58, Richard Sandiford
> <richard.sandif...@arm.com> wrote:
>>
>> Iain Sandoe <i...@sandoe.co.uk> writes:
>>> Hi Richard,
>>>
>>>> On 5 Nov 2023, at 12:11, Richard Sandiford <richard.sandif...@arm.com>
>>>> wrote:
>>>>
>>>> Iain Sandoe <i...@sandoe.co.uk> writes:
>>>
>>>>>> On 26 Oct 2023, at 21:00, Iain Sandoe <i...@sandoe.co.uk> wrote:
>>>>>
>>>>>>> On 26 Oct 2023, at 20:49, Richard Sandiford <richard.sandif...@arm.com>
>>>>> wrote:
>>>>>>>
>>>>>>> Iain Sandoe <iains....@gmail.com> writes:
>>>>>>>> This was written before Thomas' modification to the ELF-handling to
>>>>>>>> allow
>>>>>>>> a config-based change for target details. I did consider updating this
>>>>>>>> to try and use that scheme, but I think that it would sit a little
>>>>>>>> awkwardly, since there are some differences in the start-up scanning
>>>>>>>> for
>>>>>>>> Mach-O. I would say that in all probability we could improve things
>>>>>>>> but
>>>>>>>> I'd like to put this forward as a well-tested initial implementation.
>>>>>>>
>>>>>>> Sorry, I would prefer to extend the existing function instead.
>>>>>>> E.g. there's already some divergence between the Mach-O version
>>>>>>> and the default version, in that the Mach-O version doesn't print
>>>>>>> verbose messages. I also don't think that the current default code
>>>>>>> is so watertight that it'll never need to be updated in future.
>>>>>>
>>>>>> Fair enough, will explore what can be done (as I recall last I looked the
>>>>>> primary difference was in the initial start-up scan).
>>>>>
>>>>> I’ve done this as attached.
>>>>>
>>>>> For the record, when doing it, it gave rise to the same misgivings that
>>>>> led
>>>>> to the separate implementation before.
>>>>>
>>>>> * as we add formats and uncover asm oddities, they all need to be handled
>>>>> in one set of code, IMO it could be come quite convoluted.
>>>>>
>>>>> * now making a change to the MACH-O code, means I have to check I did not
>>>>> inadvertently break ELF (and likewise, in theory, an ELF change should
>>>>> check
>>>>> MACH-O, but many folks do/can not do that).
>>>>>
>>>>> Maybe there’s some half-way-house where code can usefully be shared
>>>>> without
>>>>> those down-sides.
>>>>>
>>>>> Anyway, to make progress, is the revised version OK for trunk? (tested on
>>>>> aarch64-linux and aarch64-darwin).
>>>>
>>>> Sorry for the slow reply. I was hoping we'd be able to share a bit more
>>>> code than that, and avoid an isMACHO toggle. Does something like the
>>>> attached adaption of your patch work? Only spot-checked on
>>>> aarch64-linux-gnu so far.
>>>>
>>>> (The patch tries to avoid capturing the user label prefix, hopefully
>>>> avoiding the needsULP thing.)
>>>
>>> Yes, this works for me too for Arm64 Darwin (and probably is fine for other
>>> Darwin archs in case we implement body tests there). If we decide to emit
>>> some comment-based markers to delineat functions without unwind data,
>>> we can just amend the start and end.
>>>
>>> thanks,
>>> Iain
>>> (doing some wider testing, but for now the only mach-o cases are in the
>>> arm64 code, so the fact that those passed so far is pretty good indication).
>>
>> OK, great. It passed testing for me too, so please go ahead and commit
>> if it does for you.
>>
>>> -----
>>>
>>> As an aside what’s the intention for cases like this?
>>>
>>> .data
>>> foo:
>>> .xxxx ….
>>> .size foo, .-foo
>>
>> ATM there's no way for the test to say that specific pseudo-ops are
>> interesting to it. Same for labels. It might be useful to add
>> support for that though.
>>
>> Thanks,
>> Richard
>>
>
> As you have probably already noticed with the notification from our
> CI, this patch causes
> FAIL: gcc.target/arm/pr95646.c check-function-bodies __acle_se_bar
> At quick glance it's not obvious to me why check_function_body
> does not print "body" and "against" debug traces, so there's not hint in
> gcc.log
Yeah, I’ve reproduced this (it did not show on either Richard’s nor my aarch64
testing)
... and have a potential fix.
the problem is this:
.global bar
…
. global __acle_se_bar
foo:
__acle_se_bar:
…
=====
The change in code prevernt the second label overriding the first (but the scan
checks for the second).
Actually, that’s not legal Mach-O (two global labels cannot have the same
address).
I have a fix that re-allows the override (thinking if I should assume Mach-O
will never do this or skip the change for mach-o)
——
Iain
