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 I guess running the testsuite with -verbose or -v would help? Can you have a look? Thanks, Christophe > > > > > > > >> > >> Thanks, > >> Richard > >> > >> > >> diff --git a/gcc/testsuite/lib/scanasm.exp b/gcc/testsuite/lib/scanasm.exp > >> index 5df80325dff..2434550f0c3 100644 > >> --- a/gcc/testsuite/lib/scanasm.exp > >> +++ b/gcc/testsuite/lib/scanasm.exp > >> @@ -785,23 +785,34 @@ proc configure_check-function-bodies { config } { > >> > >> # Regexp for the start of a function definition (name in \1). > >> if { [istarget nvptx*-*-*] } { > >> - set up_config(start) {^// BEGIN(?: GLOBAL|) FUNCTION DEF: > >> ([a-zA-Z_]\S+)$} > >> + set up_config(start) { > >> + {^// BEGIN(?: GLOBAL|) FUNCTION DEF: ([a-zA-Z_]\S+)$} > >> + } > >> + } elseif { [istarget *-*-darwin*] } { > >> + set up_config(start) { > >> + {^_([a-zA-Z_]\S+):$} > >> + {^LFB[0-9]+:} > >> + } > >> } else { > >> - set up_config(start) {^([a-zA-Z_]\S+):$} > >> + set up_config(start) {{^([a-zA-Z_]\S+):$}} > >> } > >> > >> # Regexp for the end of a function definition. > >> if { [istarget nvptx*-*-*] } { > >> set up_config(end) {^\}$} > >> + } elseif { [istarget *-*-darwin*] } { > >> + set up_config(end) {^LFE[0-9]+} > >> } else { > >> set up_config(end) {^\s*\.size} > >> } > >> - > >> + > >> # Regexp for lines that aren't interesting. > >> if { [istarget nvptx*-*-*] } { > >> # Skip lines beginning with '//' comments ('-fverbose-asm', for > >> # example). > >> set up_config(fluff) {^\s*(?://)} > >> + } elseif { [istarget *-*-darwin*] } { > >> + set up_config(fluff) {^\s*(?:\.|//|@)|^L[0-9ACESV]} > >> } else { > >> # Skip lines beginning with labels ('.L[...]:') or other directives > >> # ('.align', '.cfi_startproc', '.quad [...]', '.text', etc.), '//' or > >> @@ -833,9 +844,19 @@ proc parse_function_bodies { config filename result } > >> { > >> set fd [open $filename r] > >> set in_function 0 > >> while { [gets $fd line] >= 0 } { > >> - if { [regexp $up_config(start) $line dummy function_name] } { > >> - set in_function 1 > >> - set function_body "" > >> + if { $in_function == 0 } { > >> + if { [regexp [lindex $up_config(start) 0] \ > >> + $line dummy function_name] } { > >> + set in_function 1 > >> + set function_body "" > >> + } > >> + } elseif { $in_function < [llength $up_config(start)] } { > >> + if { [regexp [lindex $up_config(start) $in_function] $line] } { > >> + incr in_function > >> + } else { > >> + verbose "parse_function_bodies: skipped $function_name" > >> + set in_function 0 > >> + } > >> } elseif { $in_function } { > >> if { [regexp $up_config(end) $line] } { > >> verbose "parse_function_bodies: > >> $function_name:\n$function_body" > >> -- > >> 2.25.1