Georg-Johann Lay <[email protected]> writes:

> CC'ing Denis
>

I'm reading [email protected]

> Am 26.05.26 um 15:15 schrieb Georg-Johann Lay:
>> This patch adds support for [[len=<words>]] in (the comments of) inline
>> asm constructs.  It serves several purposes:
>> - Cases where the expanded asm is longer than determined from the
>> number
>>    of physical and logical line breaks.  Such cases can lead to errors
>>    when a jump that uses a too optimistic jump offset is crossing an asm.
>> - Better code generation for jumps that are crossing an asm.
>>    The default length of an asm is (1 + NL) * 2 words, where NL
>>    denotes the sum of physical and logical line breaks.  However,
>>    almost all AVR instructions occupy only one 16-bit word.
>> The feature is implemented in ADJUST_INSN_LENGTH.  The length of
>> an asm is the sum over all [[len=<words>]] notes, except when an
>> unrecognized construct is found or an error occurred.  In the latter
>> case, the default insn length is used.  These <words> are supported:
>> <words> = [0-9]+
>>     Specifies a non-negative decimal integer.
>> <words> = %[0-9]+
>> <words> = %[<name>]   # Already resolved to %[0-9]+ by the middle-end.
>>     Refers to the respective asm operand, which must be CONST_INT.
>> <words> = lds
>> <words> = sts
>>     Specifies the length of a LDS or STS instruction, i.e.
>>     1 word if AVR_TINY, and 2 words otherwise.
>> <words> = %~
>> <words> = %~call
>> <words> = %~jmp
>>     Specifies the length of a %~call resp. %~jmp instruction, i.e.
>>     2 words if AVR_HAVE_JMP_CALL, and 1 word otherwise.
>> In order to observe the assigned lengths, see -fdump-rtl-shorten or
>> the
>> ";; ADDR = ..." insn addresses in the asm output with -mlog=insn_addresses.
>> The benefits of using magic comments are:
>> - The feature is backwards compatible, and the target code can use
>>    the same asm syntax since only asm comments have to be adjusted.
>>    No #ifdef feature test macros are needed.  The only case where the
>>    feature is not fully backwards compatible is when asm templates
>>    already contain invalid "[[len=" notes for some reason.  In that
>>    case, -mno-asm-len-notes restores the old behavior.
>> - Since the asm size is the sum over all notes, the final size can
>>    be stitched together from multiple annotations / parts of an asm
>>    template, and there is no need to support operations like plus.
>> There are still many asm constructs in AVR-LibC for example, like
>> in avr/pgmspace.h which is still used in C++ because avr-g++
>> doesn not (yet) support named address-spaces like __flash.
>> The patch passes without new regressions.
>> Before going all in and adding test cases etc. my question is if
>> this feature is okay in principle, and if magic comments other
>> than [[len=...]] would be preferred for some reason.
>> The only thing that doesn't work as expected (though expected
>> by the LTO designers), is using warning_at in lto1.  This means
>> that in order to get the warnings in LTO mode, one has to compile
>> with -ffat-lto-objects so cc1[plus] can do the diagnosing.
>> The patch is for trunk.
>> Johann
>> -- AVR: Support [[len=<words]] notes in inline asm to specify its
>> size.
>> This patch adds support for [[len=<words>]] in (the comments of)
>> inline
>> asm constructs.  It serves several purposes:
>> - Cases where the expanded asm is longer than determined from the
>> number
>>    of physical and logical line breaks.  Such cases can lead to errors
>>    when a jump that uses a too optimistic jump offset is crossing an asm.
>> - Better code generation for jumps that are crossing an asm.  The
>> default
>>    length of an asm is (1 + NL) * 2 words, where NL denotes the sum of
>>    physical and logical line breaks.  However, almost all AVR instructions
>>    occupy only one 16-bit word.
>> The feature is implemented in ADJUST_INSN_LENGTH.  The length of
>> an asm is the sum over all [[len=<words>]] notes, except when an
>> unrecognized construct is found or an error occurred.  In the latter
>> case, the default insn length is used.  These <words> are supported:
>> <words> = [0-9]+
>>     Specifies a non-negative decimal integer.
>> <words> = %[0-9]+
>> <words> = %[<name>]   # Already resolved to %[0-9]+ by the middle-end.
>>     Refers to the respective asm operand, which must be CONST_INT.
>> <words> = lds
>> <words> = sts
>>     Specifies the length of a LDS or STS instruction, i.e.
>>     1 word if AVR_TINY, and 2 words otherwise.
>> <words> = %~
>> <words> = %~call
>> <words> = %~jmp
>>     Specifies the length of a %~call resp. %~jmp instruction, i.e.
>>     2 words if AVR_HAVE_JMP_CALL, and 1 word otherwise.
>> In order to observe the assigned lengths, see -fdump-rtl-shorten or
>> the
>> ";; ADDR = ..." insn addresses in the asm output with -mlog=insn_addresses.
>> The benefits of using magic comments are:
>> - The feature is backwards compatible, and the target code can use
>>    the same asm syntax since only asm comments have to be adjusted.
>>    No #ifdef feature test macros are needed.  The only case where the
>>    feature is not fully backwards compatible is when asm templates
>>    already contain invalid "[[len=" notes for some reason.  In that
>>    case, -mno-asm-len-notes restores the old behavior.
>> - Since the asm size is the sum over all notes, the final size can
>>    be stitched together from multiple annotations / parts of an asm
>>    template, and there is no need to support operations like plus.
>> gcc/
>>      * config/avr/avr.cc (avr_read_number, avr_length_of_asm)
>>      (avr_maybe_length_of_asm): New static functions.
>>      (avr_adjust_insn_length): Call avr_maybe_length_of_asm on
>>      unrecognized insns.
>>      * config/avr/avr.opt (-masm-len-notes, -Wasm-len-notes): New
>>      options.
>>      * doc/invoke.texi (AVR Options): Add -masm-len-notes,
>>      -Wasm-len-notes.
>>      * doc/extend.texi (Size of an asm): Add @subsubheading
>>      "Specifying the size of an asm on AVR".
>> libgcc/config/avr/libf7/
>>      * libf7.h: Add "[len=...]]" notes to all non-empty inline asm's.
>>      * libf7.c: Dito.
>> 

Sorry for the delay, I'm not at home.
I like your solution.

Please apply the patch.

Denis.

Reply via email to