Some initial comments. On 11/01/2021 11:10, g...@danielengel.com wrote: > From: Daniel Engel <g...@danielengel.com> > > These definitions facilitate subsequent patches in this series. > > gcc/libgcc/ChangeLog: > 2021-01-07 Daniel Engel <g...@danielengel.com> > > * 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.