[PATCH v4 00/15] sframe: Enhancements to SFrame info generation

[Richard Earnshaw (lists)]

On 27/06/2024 12:02, Jens Remus wrote:
> Am 27.06.2024 um 10:39 schrieb Jan Beulich:
>> On 27.06.2024 10:28, Jens Remus wrote:
>>> Am 24.06.2024 um 16:23 schrieb Jens Remus:
>>>> Patches 1 and 2 (updated) are minor cleanups/enhancements to the
>>>> existing SFrame support on AArch64 and x86 AMD64.
> ...
>>>> Jens Remus (15):
>>>>     x86: Remove unused SFrame CFI RA register variable
>>>>     gas: Enhance arch-specific SFrame configuration descriptions
>>>>     readelf/objdump: Dump SFrame CFA fixed FP and RA offsets
>>>>     readelf/objdump: Display SFrame fixed RA offset as 'f' in dump
>>>>     gas: Print DWARF call frame insn name in SFrame warning message
>>>>     gas: Skip SFrame FDE if CFI specifies non-FP/SP base register
>>>>     gas: Warn if SFrame FDE is skipped due to non-default return column
>>>>     gas: Refactor SFrame CFI opcode DW_CFA_register processing
>>>>     gas: User readable warnings if SFrame FDE is not generated
>>>>     gas: Skip SFrame FDE if FP without RA on stack
>>>>     gas: Skip SFrame FDE if .cfi_window_save
>>>>     gas: Don't skip SFrame FDE if .cfi_register specifies RA w/o tracking
>>>>     gas: Don't skip SFrame FDE if .cfi_register specifies SP register
>>>>     gas: Test predicate whether SFrame RA tracking is used
>>>>     gas: Validate SFrame RA tracking and fixed RA offset
>>>>
>>>>    gas/config/tc-aarch64.c                       |   6 +-
>>>>    gas/config/tc-aarch64.h                       |  12 +-
>>>>    gas/config/tc-i386.c                          |   6 +-
>>>>    gas/config/tc-i386.h                          |  10 +-
>>>>    gas/gen-sframe.c                              | 246 +++++++++++++++---
>>>>    gas/gen-sframe.h                              |   2 +
>>>>    .../gas/cfi-sframe/cfi-sframe-common-1.d      |   2 +
>>>>    .../gas/cfi-sframe/cfi-sframe-common-2.d      |   2 +
>>>>    .../gas/cfi-sframe/cfi-sframe-common-3.d      |   2 +
>>>>    .../gas/cfi-sframe/cfi-sframe-common-4.d      |   6 +-
>>>>    .../gas/cfi-sframe/cfi-sframe-common-5.d      |   6 +-
>>>>    .../gas/cfi-sframe/cfi-sframe-common-6.d      |   6 +-
>>>>    .../gas/cfi-sframe/cfi-sframe-common-7.d      |   6 +-
>>>>    .../gas/cfi-sframe/cfi-sframe-common-8.d      |   4 +-
>>>>    .../gas/cfi-sframe/cfi-sframe-x86_64-1.d      |   9 +-
>>>>    gas/testsuite/gas/cfi-sframe/common-empty-1.d |   4 +-
>>>>    gas/testsuite/gas/cfi-sframe/common-empty-2.d |   4 +-
>>>>    gas/testsuite/gas/cfi-sframe/common-empty-3.d |   3 +
>>>>    .../gas/scfi/x86_64/scfi-cfi-sections-1.d     |  11 +-
>>>>    .../gas/scfi/x86_64/scfi-dyn-stack-1.d        |  11 +-
>>>>    ld/testsuite/ld-sframe/discard.s              |   1 -
>>>>    ld/testsuite/ld-x86-64/sframe-plt-1.d         |   9 +-
>>>>    ld/testsuite/ld-x86-64/sframe-simple-1.d      |  17 +-
>>>>    libsframe/sframe-dump.c                       |  18 +-
>>>>    24 files changed, 305 insertions(+), 98 deletions(-)
>>>
>>> Thank you for the review, Indu!
>>>
>>> Can I go ahead and commit the series to mainline, with the review
>>> feedback implemented and the offending trailers removed?
>>
>> I haven't been following closely what specifically Indu asked for. In
>> general, if he's happy with the sframe-specific changes, respective
>> changes can have my (implicit) okay. Target-specific changes will want
>> a target-specific okay, though. From the titles I can only identify
>> patch 1 as having x86-specific aspects. Looks like it's only patch 2
>> which has further arch-specific changes - I'm okay with the x86 parts,
>> but I can't speak for Arm64 (well, technically I could, but I'd like
>> to avoid doing so unless strictly necessary).
> 
> Richard and Marcus, could one of you please have a short look at patch 2 of this series, as it contains changes to the AArch64-specific assembler code? Note that it only rewords and harmonizes generic comments that are SFrame specific. Therefore I had not considered to Cc you as AArch64 maintainers nor the x86-64 maintainers and assumed Indu's ok as SFrame maintainer would be sufficient.

The aarch64 changes look to me to all be comment clarifications related to the SFrame stuff.  If Indu is happy, then so am I.

R.

> 
> Thanks and regards,
> Jens