[PATCH] D36749: [LLD][ELF][AArch64] Complete implementation of -fix-cortex-a53-843419
Rafael Avila de Espindola via llvm-commits
llvm-commits at lists.llvm.org
Fri Dec 8 19:41:33 PST 2017
Peter Smith via Phabricator <reviews at reviews.llvm.org> writes:
> -// ADRP (0xff8 or 0xffc)
> +// ADRP (0xff8 or 0xffc).
> // 2.)
> -// - load or store single register or either integer or vector registers
> -// - STP or STNP of either vector or vector registers
> -// - Advanced SIMD ST1 store instruction
> -// Must not write Rn
> -// 3.) optional instruction, can't be a branch, must not write Rn, may read Rn
> +// - load or store single register or either integer or vector registers.
> +// - STP or STNP of either vector or vector registers.
> +// - Advanced SIMD ST1 store instruction.
> +// - Must not write Rn.
> +// 3.) optional instruction, can't be a branch, must not write Rn, may read Rn.
> // 4.) A load or store instruction from the Load/Store register unsigned
> -// immediate class using Rn as the base register
> +// immediate class using Rn as the base register.
>
> // Each section contains a sequence of instructions that should be recognized
> // as erratum 843419. The test cases cover the major variations such as:
> -// adrp starts at 0xfff8 or 0xfffc
> -// Variations in instruction class for instruction 2
> -// Optional instruction 3 present or not
> -// Load or store for instruction 4.
> +// - adrp starts at 0xfff8 or 0xfffc.
> +// - Variations in instruction class for instruction 2.
> +// - Optional instruction 3 present or not.
> +// - Load or store for instruction 4.
These comment updates are fine. Please commit them first.
> +#include <map>
> +#include <vector>
> +
> namespace lld {
> namespace elf {
>
> +class Defined;
> +class InputSection;
> +class InputSectionDescription;
> class OutputSection;
> -void reportA53Errata843419Fixes();
> +class Patch843419Section;
> +
> +class SectionPatcher {
Maybe call it AArch843419Patcher?
> + // Apply any relocation transferred from the original PatcheeSection.
> + // For a SyntheticSection Buf already has OutSecOff added, but relocateAlloc
> + // also adds OutSecOff so we need to subtract to avoid double counting.
> + this->relocateAlloc(Buf - OutSecOff, Buf - OutSecOff + getSize());
I wonder if we could read the already patched output buffer and avoid
this? Can we guarantee that the patch is always written after the patchee?
> +// Insert the PatchSections we have created back into the
> +// InputSectionDescription. As inserting patches alters the addresses of
> +// InputSections that follow them, we try and place the patches after all the
> +// executable sections, although we may need to insert them earlier if the
> +// InputSectionDescription is larger than the maximum branch range.
> +void SectionPatcher::insertPatches(InputSectionDescription &ISD,
> + std::vector<Patch843419Section *> &Patches) {
> + uint64_t ISLimit;
> + uint64_t PrevISLimit = ISD.Sections.front()->OutSecOff;
> + uint64_t PatchUpperBound = PrevISLimit + Target->ThunkSectionSpacing;
> +
> + // Set the OutSecOff of patches to the place where we want to insert them.
> + // We use a similar strategy to Thunk placement. Place patches roughly
> + // every multiple of maximum branch range.
> + auto PatchIt = Patches.begin();
> + auto PatchEnd = Patches.end();
> + for (const InputSection *IS : ISD.Sections) {
> + ISLimit = IS->OutSecOff + IS->getSize();
> + if (ISLimit > PatchUpperBound) {
> + while (PatchIt != PatchEnd) {
> + if ((*PatchIt)->getLDSTAddr() >= PrevISLimit)
> + break;
> + (*PatchIt)->OutSecOff = PrevISLimit;
> + ++PatchIt;
> + }
> + PatchUpperBound = PrevISLimit + Target->ThunkSectionSpacing;
> }
> - if (DataSym == MapSyms.end())
> - break;
> - CodeSym = std::next(DataSym);
> + PrevISLimit = ISLimit;
> + }
> + for (; PatchIt != PatchEnd; ++PatchIt) {
> + (*PatchIt)->OutSecOff = ISLimit;
> }
> -}
>
> -// Scan all the executable code in an AArch64 link to detect the Cortex-A53
> -// erratum 843419.
> -// FIXME: The current implementation only scans for the erratum sequence, it
> -// does not attempt to fix it.
> -void lld::elf::reportA53Errata843419Fixes() {
> - std::map<InputSection *, std::vector<const Defined *>> SectionMap =
> - makeAArch64SectionMap();
> + // merge all patch sections
> + std::vector<InputSection *> Tmp;
> + Tmp.reserve(ISD.Sections.size() + Patches.size());
> + auto MergeCmp = [](const InputSection *A, const InputSection *B) {
> + if (A->OutSecOff < B->OutSecOff)
> + return true;
> + if (A->OutSecOff == B->OutSecOff && isa<Patch843419Section>(A) &&
> + !isa<Patch843419Section>(B))
> + return true;
> + return false;
> + };
> + std::merge(ISD.Sections.begin(), ISD.Sections.end(), Patches.begin(),
> + Patches.end(), std::back_inserter(Tmp), MergeCmp);
After this the patch sections still have a OutSecOff of the limit of
where they can be placed, no? Is that OK?
> +void SectionPatcher::patchInputSectionDescription(
> + InputSectionDescription &ISD, std::vector<Patch843419Section *> &Patches) {
return the std::vector.
Cheers,
Rafael
More information about the llvm-commits
mailing list