On Thu, 23 Nov 2023 at 10:09, Iain Sandoe <i...@sandoe.co.uk> wrote: > > 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. >
It makes sense, aarch64 and arm are different targets. > 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) > Good news, thanks! Christophe > —— > > > Iain >
