[ARM] VFP11 denorm erratum workaround in linker

Julian Brown julian@codesourcery.com
Mon Jan 22 15:06:00 GMT 2007
Nick Clifton wrote:
> Hi Julian,

Hi, sorry for the rather extreme delay in getting back to this, but here 
goes...

>> We've worked around this erratum in the linker
> 
> Hmm, I was going to argue against this, but when I reviewed my thoughts 
> I realised that they came down to the fact that I would rather a large 
> patch like this, with its potential for introducing new bugs, be put 
> into gcc rather than the linker :-)
> 
> For the record though, let me state that I do not like patches like 
> this, when they are liable to introduce new bugs, and they are being 
> used work around somebody else's problem that they cannot/will not fix. 
>   I appreciate that working around hardware bugs is a necessary evil, I 
> just do not like it very much.

The justification for the fix being in the linker is that it then works 
for static libraries as well as executables, in that they can be 
distributed without worrying about separate versions with the erratum 
workaround enabled. The ability to distribute such static libraries is 
important to ARM's customers, AIUI.

>> GCC at present only outputs scalar code for VFP units, so the default 
>> (pre-ARMv7 architectures) is to fix scalar code. The workaround can be 
>> disabled or selected for vector operations using the linker options:
> 
> Why would you want to only fix scalar code, but not fix vector code ? 
> Just because gcc does not currently produce any, there is no guarantee 
> that there will not be any in the final link.  Surely the safe default 
> should be to fix both ?

I've changed the default to not fix either, see below.

>>   --vfp11-denorm-fix={none,scalar,vector}
> 
> Shouldn't that be:
> 
>    --vfp11-denorm-fix={none,scalar,vector,both}
> 
> Or does "vector" mode imply "scalar" as well ?

Yes, vector-mode fixes are a superset of scalar-mode fixes. I've noted 
this in ld.texinfo.

> I am also worried about the future.  Suppose a new revision of the VFP 
> unit comes out (in less than v7 architecture) which does not have this 
> problem.  Being able to disable this workaround on the command line is 
> all very well, but what if the user forgets, or does not know about it ? 
>  I would not be at all surprised to receive bug reports in the future 
> saying "why is your stupid linker creating all these branches to some 
> .vfp11_veneer section?".  To my mind hardware workarounds should be 
> disabled by default and only enabled by users who know that they are 
> working with broken hardware.  (This also emphasises the fact that the 
> software is working around broken hardware, so do not blame the software 
> for any reduction in performance).

Yeah, I agree, so I've made the workaround fix optional: if a user knows 
he is using buggy hardware, the workaround must now be enabled explicitly.

>> The workaround works by scanning input code sections for 
>> potentially-troublesome code sequences, and replacing them by branches 
>> to a specially-constructed veneer (similar to the way ARM<->Thumb 
>> interworking stubs are created at link-time). The veneer contains the 
>> original instruction plus a branch back to the original subsequent 
>> instruction, which is sufficient to avoid the erratum in either scalar 
>> or vector mode.
> 
> In a really large program, could these branches go out of range ?

Maybe (we'd be talking about 32Mb binaries?) -- I've added error 
checking for that case, but I'm not sure if ld would already have blown 
up long before that point... I suspect so :-)

>> Tested with cross to arm-none-eabi, and also with the GCC testsuite 
>> (C/C++/libstdc++) with -mfpu=vfp/-mfloat-abi=softfp.
> 
> Did you also run the gas/ld/binutils testsuites ?

Yep.

>> + /* Information about a VFP11 erratum veneer, or a branch to such a 
>> veneer.
>> +    The type can be:
>> +      'a': branch to ARM veneer
>> +      't': branch to Thumb veneer
>> +      'A': ARM veneer
>> +      'T': Thumb veneer.  */
> 
>> +   char type;
> 
> I do not see much point in 'type' being a char.  An enum would be better 
> (and self-documenting).  I doubt that you would save any space by using 
> the char as the structure is likely to be padded out to an int boundary 
> anyway.

Fixed. (The previous ARM/Thumb/data mapping-symbol code still uses chars 
though).

> It would be very nice if you could include a URL for the errata sheet 
> here.  Assuming that ARM make the sheet available for direct download.

I've had a look, and I don't think there's a public description. Richard?

> This information really needs to be in the documentation.  In fact you 
> need to add documentation of the new switch to ld.texinfo and a mention 
> of it to ld/NEWS as well.

I've added a section to ld.texinfo.

> One other thing - I think that it would be a good idea to include a 
> linker testsuite entry to check this new feature.

...and a couple of tests, though they're by no means exhaustive.

OK now?

Cheers,

Julian

ChangeLog

     bfd/
     * bfd-in2.h: Regenerate.
     * bfd-in.h (bfd_arm_vfp11_fix): New enum. Specify how VFP11
     instruction scanning should be done.
     (bfd_elf32_arm_init_maps, bfd_elf32_arm_vfp11_erratum_scan)
     (bfd_elf32_arm_vfp11_fix_veneer_locations): Add prototypes.
     (bfd_elf32_arm_set_target_relocs): Add vfp11 fix type argument to
     prototype.
     * elf-bfd.h (elf_backend_write_section): Add struct bfd_link_info
     argument.
     * elf32-arm.c (VFP11_ERRATUM_VENEER_SECTION_NAME)
     (VFP11_ERRATUM_VENEER_ENTRY_NAME): Define macros.
     (elf32_vfp11_erratum_type): New enum.
     (elf32_vfp11_erratum_list): New struct. List of veneers or jumps to
     veneers.
     (_arm_elf_section_data): Add mapsize, erratumcount, erratumlist.
     (elf32_arm_link_hash_table): Add vfp11_erratum_glue_size,
     vfp11_fix and num_vfp11_fixes fields.
     (elf32_arm_link_hash_table_create): Initialise vfp11_fix,
     vfp11_erratum_glue_size, num_vfp11_fixes fields.
     (VFP11_ERRATUM_VENEER_SIZE): Define. Size of an (ARM) veneer.
     (bfd_elf32_arm_allocate_interworking_sections): Initialise erratum
     glue section.
     (elf32_arm_section_map_add): Add an code/data mapping symbol entry
     to a section's map.
     (record_vfp11_erratum_veneer): Create a single veneer, and its
     associated symbols.
     (bfd_elf32_arm_add_glue_sections_to_bfd): Add vfp11 erratum glue.
     (bfd_elf32_arm_init_maps): Initialise mapping symbol table for input
     BFDs.
     (bfd_elf32_arm_set_vfp11_fix): Set the type of erratum workaround
     required.
     (bfd_arm_vfp11_pipe): Define VFP11 instruction pipes.
     (bfd_arm_vfp11_regno): Recode a register number from a VFP11 insn.
     (bfd_arm_vfp11_write_mask): Update write mask according to coded
     register number.
     (bfd_arm_vfp11_antidependency): New function.
     (bfd_arm_vfp11_insn_decode): Decode a VFP11 insn.
     (elf32_arm_compare_mapping): Declare.
     (bfd_elf32_arm_vfp11_erratum_scan): Scan the sections of an input
     BFD for potential erratum-triggering insns. Record results.
     (bfd_elf32_arm_vfp11_fix_veneer_locations): Find out where veneers
     and branches to veneers have been placed in virtual memory after
     layout.
     (bfd_elf32_arm_set_target_relocs): Set vfp11_fix field in global
     hash table.
     (elf32_arm_output_symbol_hook): Remove.
     (elf32_arm_write_section): Output veneers, and branches to veneers.
     Use maps from input sections, not output sections, for code
     byte-swapping.
     * elf32-ppc.c (ppc_elf_write_section): Add dummy link_info argument.
     * elf32-score.c (_bfd_score_elf_write_section): Likewise.
     * elfxx-mips.c (_bfd_mips_elf_write_section): Likewise.
     * elfxx-mips.h (_bfd_mips_elf_write_section): Likewise.

     ld/
     * NEWS: Mention --vfp11-denorm-fix option.
     * ld.texinfo: Document above.
     * emulparams/armelf_linux.sh (OTHER_TEXT_SECTIONS): Add
     .vfp11_veneer section.
     * emulparams/armelf.sh (OTHER_TEXT_SECTIONS): Likewise.
     * emultempl/armelf.em (vfp11_denorm_fix): New static variable.
     (arm_elf_before_allocation): Call bfd_elf32_arm_set_vfp11_fix,
     bfd_elf32_arm_init_maps and bfd_elf32_arm_vfp11_erratum_scan.
     (arm_elf_after_allocation): New function. Call
     bfd_elf32_arm_vfp11_fix_veneer_locations for all input statements.
     (arm_elf_create_output_section_statements): Pass vfp11 fix command
     line option to BFD.
     (OPTION_VFP11_DENORM_FIX): New option.
     (PARSE_AND_LIST_LONGOPTS): Handle new option.
     (PARSE_AND_LIST_OPTIONS): Likewise.
     (PARSE_AND_LIST_ARGS_CASES): Likewise.
     (LDEMUL_AFTER_ALLOCATION): Define.

     ld/testsuite/
     * ld-arm/arm-elf.exp: Add VFP11 tests.
     * ld-arm/vfp11-fix-none.s: New file.
     * ld-arm/vfp11-fix-none.d: Expected disassembly of above.
     * ld-arm/vfp11-fix-scalar.s: New file.
     * ld-arm/vfp11-fix-scalar.d: Expected disassembly of above.
     * ld-arm/vfp11-fix-vector.s: New file.
     * ld-arm/vfp11-fix-vector.d: Expected disassembly of above.
-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: mainline-vfp11-erratum-fix-15
URL: <https://sourceware.org/pipermail/binutils/attachments/20070122/487781ca/attachment.ksh>


More information about the Binutils mailing list