Some initial comments.
On 11/01/2021 11:10, [email protected] wrote:
> From: Daniel Engel <[email protected]>
>
> These definitions facilitate subsequent patches in this series.
>
> gcc/libgcc/ChangeLog:
> 2021-01-07 Daniel Engel <[email protected]>
>
> * config/arm/t-elf: Organize functions into logical groups.
> * config/arm/lib1funcs.S: Add FUNC_START macro variations for
> weak functions and manual control of the target section;
> rename THUMB_FUNC_START as THUMB_FUNC_ENTRY for consistency;
> removed unused macros THUMB_SYNTAX, ARM_SYM_START, SYM_END;
> removed redundant syntax directives.
This needs to be re-formatted using the correct ChangeLog style, which
is in most cases
* <filename> (<function-or-macro-name>): <Summary of change>.
You can repeat for multiple functions in the same file, but leave off
the "* <filename>" part as long as they are contiguous in the log.
> ---
> libgcc/config/arm/lib1funcs.S | 114 +++++++++++++++++++---------------
> libgcc/config/arm/t-elf | 55 +++++++++++++---
> 2 files changed, 110 insertions(+), 59 deletions(-)
>
> diff --git a/libgcc/config/arm/lib1funcs.S b/libgcc/config/arm/lib1funcs.S
> index c2fcfc503ec..b4541bae791 100644
> --- a/libgcc/config/arm/lib1funcs.S
> +++ b/libgcc/config/arm/lib1funcs.S
> @@ -69,11 +69,13 @@ see the files COPYING3 and COPYING.RUNTIME respectively.
> If not, see
> #define TYPE(x) .type SYM(x),function
> #define SIZE(x) .size SYM(x), . - SYM(x)
> #define LSYM(x) .x
> +#define LLSYM(x) .L##x
> #else
> #define __PLT__
> #define TYPE(x)
> #define SIZE(x)
> #define LSYM(x) x
> +#define LLSYM(x) x
> #endif
I can live with this.
>
> /* Function end macros. Variants for interworking. */
> @@ -247,6 +249,14 @@ LSYM(Lend_fde):
>
> #define COND(op1, op2, cond) op1 ## op2 ## cond
>
> +#ifdef __ARM_FEATURE_IT
> + #define IT(ins,c) ins##c
> +#else
> + // Assume default Thumb-1 flags-affecting suffix 's'.
> + // Almost all instructions require this in unified syntax.
> + #define IT(ins,c) ins##s
This simply doesn't make much sense, at least, not enough to make it
generally available. It seems it would be invariably wrong to replace a
conditional instruction in arm/thumb2 code with a non-conditional flag
setting instruction in thumb1. So please don't do this as it's likely
to be a source of bugs going forwards if folk don't understand exactly
when it is safe.
> +#endif
> +
> #ifdef __ARM_EABI__
> .macro ARM_LDIV0 name signed
> cmp r0, #0
> @@ -280,7 +290,6 @@ LSYM(Lend_fde):
> pop {r1, pc}
>
> #elif defined(__thumb2__)
> - .syntax unified
OK - I think this is now set unconditionally at the top level.
> .ifc \signed, unsigned
> cbz r0, 1f
> mov r0, #0xffffffff
> @@ -324,10 +333,6 @@ LSYM(Lend_fde):
> .endm
> #endif
>
> -.macro FUNC_END name
> - SIZE (__\name)
> -.endm
> -
Moved later, OK.
> .macro DIV_FUNC_END name signed
> cfi_start __\name, LSYM(Lend_div0)
> LSYM(Ldiv0):
> @@ -340,48 +345,64 @@ LSYM(Ldiv0):
> FUNC_END \name
> .endm
>
> -.macro THUMB_FUNC_START name
> - .globl SYM (\name)
> - TYPE (\name)
> - .thumb_func
> -SYM (\name):
> -.endm
I'm not really sure what you reasoning is for removing the *FUNC_START
macros and then adding almost identical macros of the form *FUNC_ENTRY.
It seems to me like unnecessary churn.
> -
> /* Function start macros. Variants for ARM and Thumb. */
> -
> #ifdef __thumb__
> #define THUMB_FUNC .thumb_func
> #define THUMB_CODE .force_thumb
> -# if defined(__thumb2__)
> -#define THUMB_SYNTAX
> -# else
> -#define THUMB_SYNTAX
> -# endif
> #else
> #define THUMB_FUNC
> #define THUMB_CODE
> -#define THUMB_SYNTAX
OK. Dead code.
> #endif
>
> -.macro FUNC_START name
> - .text
> +.macro THUMB_FUNC_ENTRY name
> + .globl SYM (\name)
> + TYPE (\name)
> + .force_thumb
> + .thumb_func
> +SYM (\name):
> +.endm
> +
> +/* Strong global export, no section change. */
> +.macro FUNC_ENTRY name
> .globl SYM (__\name)
> TYPE (__\name)
> - .align 0
It's often wrong to assume the section is correctly aligned on entry -
so removing this is suspect, unless you're adding explicit alignment
before every function instance. I know the issue of litterals in the
code segments is currently still under discussion; but if they do exist,
they have to be 32-bit aligned because the thumb1 adr instruction will
not work correctly otherwise. Arm code must always be (at least) 32-bit
aligned, and thumb code 16-bit - but there are often significant
performance wins from being more aligned than that on entry.
> THUMB_CODE
> THUMB_FUNC
> - THUMB_SYNTAX
> SYM (__\name):
> .endm
>
> -.macro ARM_SYM_START name
> - TYPE (\name)
> - .align 0
> -SYM (\name):
> +/* Weak global export, no section change. */
> +.macro WEAK_ENTRY name
> + .weak SYM(__\name)
> + FUNC_ENTRY \name
> +.endm
> +
> +/* Strong global export, explicit section. */
> +.macro FUNC_START_SECTION name section
> + .section \section,"x"
> + .align 0
> + FUNC_ENTRY \name
> .endm
>
> -.macro SYM_END name
> - SIZE (\name)
> +/* Weak global export, explicit section. */
> +.macro WEAK_START_SECTION name section
> + .weak SYM(__\name)
> + FUNC_START_SECTION \name \section
> +.endm
> +
> +/* Strong global export, default section. */
> +.macro FUNC_START name
> + FUNC_START_SECTION \name .text
> +.endm
> +
> +/* Weak global export, default section. */
> +.macro WEAK_START name
> + .weak SYM(__\name)
> + FUNC_START_SECTION \name .text
> +.endm
> +
> +.macro FUNC_END name
> + SIZE (__\name)
> .endm
>
> /* Special function that will always be coded in ARM assembly, even if
> @@ -392,7 +413,6 @@ SYM (\name):
> /* For Thumb-2 we build everything in thumb mode. */
> .macro ARM_FUNC_START name
> FUNC_START \name
> - .syntax unified
> .endm
> #define EQUIV .thumb_set
> .macro ARM_CALL name
> @@ -447,6 +467,11 @@ SYM (__\name):
> #endif
> .endm
>
> +.macro WEAK_ALIAS new old
> + .weak SYM(__\new)
> + FUNC_ALIAS \new \old
> +.endm
> +
> #ifndef NOT_ISA_TARGET_32BIT
> .macro ARM_FUNC_ALIAS new old
> .globl SYM (__\new)
> @@ -1905,10 +1930,9 @@ ARM_FUNC_START ctzsi2
>
> .text
> .align 0
> - .force_thumb
>
> .macro call_via register
> - THUMB_FUNC_START _call_via_\register
> + THUMB_FUNC_ENTRY _call_via_\register>
> bx \register
> nop
> @@ -1991,7 +2015,7 @@ _arm_return_r11:
> .macro interwork_with_frame frame, register, name, return
> .code 16
>
> - THUMB_FUNC_START \name
> + THUMB_FUNC_ENTRY \name
>
This, and all the subsequent instances of this change would go away if
you kept the original name.
> bx pc
> nop
> @@ -2008,7 +2032,7 @@ _arm_return_r11:
> .macro interwork register
> .code 16
>
> - THUMB_FUNC_START _interwork_call_via_\register
> + THUMB_FUNC_ENTRY _interwork_call_via_\register
>
> bx pc
> nop
> @@ -2045,7 +2069,7 @@ LSYM(Lchange_\register):
> /* The LR case has to be handled a little differently... */
> .code 16
>
> - THUMB_FUNC_START _interwork_call_via_lr
> + THUMB_FUNC_ENTRY _interwork_call_via_lr
>
> bx pc
> nop
> @@ -2073,9 +2097,7 @@ LSYM(Lchange_\register):
>
> .text
> .align 0
> - .force_thumb
I think .force_thumb is required to enable this code to build when the
base architecture is ARMv4 or earlier. It's a bit of a wart, but needs
careful testing if it is to be removed.
> - .syntax unified
> - THUMB_FUNC_START __gnu_thumb1_case_sqi
> + THUMB_FUNC_ENTRY __gnu_thumb1_case_sqi
> push {r1}
> mov r1, lr
> lsrs r1, r1, #1
> @@ -2092,9 +2114,7 @@ LSYM(Lchange_\register):
>
> .text
> .align 0
> - .force_thumb
> - .syntax unified
> - THUMB_FUNC_START __gnu_thumb1_case_uqi
> + THUMB_FUNC_ENTRY __gnu_thumb1_case_uqi
> push {r1}
> mov r1, lr
> lsrs r1, r1, #1
> @@ -2111,9 +2131,7 @@ LSYM(Lchange_\register):
>
> .text
> .align 0
> - .force_thumb
> - .syntax unified
> - THUMB_FUNC_START __gnu_thumb1_case_shi
> + THUMB_FUNC_ENTRY __gnu_thumb1_case_shi
> push {r0, r1}
> mov r1, lr
> lsrs r1, r1, #1
> @@ -2131,9 +2149,7 @@ LSYM(Lchange_\register):
>
> .text
> .align 0
> - .force_thumb
> - .syntax unified
> - THUMB_FUNC_START __gnu_thumb1_case_uhi
> + THUMB_FUNC_ENTRY __gnu_thumb1_case_uhi
> push {r0, r1}
> mov r1, lr
> lsrs r1, r1, #1
> @@ -2151,9 +2167,7 @@ LSYM(Lchange_\register):
>
> .text
> .align 0
> - .force_thumb
> - .syntax unified
> - THUMB_FUNC_START __gnu_thumb1_case_si
> + THUMB_FUNC_ENTRY __gnu_thumb1_case_si
> push {r0, r1}
> mov r1, lr
> adds.n r1, r1, #2 /* Align to word. */
> diff --git a/libgcc/config/arm/t-elf b/libgcc/config/arm/t-elf
> index 9da6cd37054..6a31a328073 100644
> --- a/libgcc/config/arm/t-elf
> +++ b/libgcc/config/arm/t-elf
> @@ -18,15 +18,52 @@ endif # !__symbian__
> # However this is not true for ARMv6M. Here we want to use the soft-fp C
> # implementation. The soft-fp code is only build for ARMv6M. This pulls
> # in the asm implementation for other CPUs.
> -LIB1ASMFUNCS += _udivsi3 _divsi3 _umodsi3 _modsi3 _dvmd_tls _bb_init_func \
> - _call_via_rX _interwork_call_via_rX \
> - _lshrdi3 _ashrdi3 _ashldi3 \
> - _arm_negdf2 _arm_addsubdf3 _arm_muldivdf3 _arm_cmpdf2 _arm_unorddf2 \
> - _arm_fixdfsi _arm_fixunsdfsi \
> - _arm_truncdfsf2 _arm_negsf2 _arm_addsubsf3 _arm_muldivsf3 \
> - _arm_cmpsf2 _arm_unordsf2 _arm_fixsfsi _arm_fixunssfsi \
> - _arm_floatdidf _arm_floatdisf _arm_floatundidf _arm_floatundisf \
> - _clzsi2 _clzdi2 _ctzsi2
> +
> +# Group 1: Integer functions
> +LIB1ASMFUNCS += \
> + _ashldi3 \
> + _ashrdi3 \
> + _lshrdi3 \
> + _clzdi2 \
> + _clzsi2 \
> + _ctzsi2 \
> + _dvmd_tls \
> + _divsi3 \
> + _modsi3 \
> + _udivsi3 \
> + _umodsi3 \
> +
> +# Group 2: Single precision floating point
> +LIB1ASMFUNCS += \
> + _arm_addsubsf3 \
> + _arm_cmpsf2 \
> + _arm_fixsfsi \
> + _arm_fixunssfsi \
> + _arm_floatdisf \
> + _arm_floatundisf \
> + _arm_muldivsf3 \
> + _arm_negsf2 \
> + _arm_unordsf2 \
> +
> +# Group 3: Double precision floating point
> +LIB1ASMFUNCS += \
> + _arm_addsubdf3 \
> + _arm_cmpdf2 \
> + _arm_fixdfsi \
> + _arm_fixunsdfsi \
> + _arm_floatdidf \
> + _arm_floatundidf \
> + _arm_muldivdf3 \
> + _arm_negdf2 \
> + _arm_truncdfsf2 \
> + _arm_unorddf2 \
> +
> +# Group 4: Miscellaneous
> +LIB1ASMFUNCS += \
> + _bb_init_func \
> + _call_via_rX \
> + _interwork_call_via_rX \
> +
This bit is OK.
>
> # Currently there is a bug somewhere in GCC's alias analysis
> # or scheduling code that is breaking _fpmul_parts in fp-bit.c.
>
R.
