[lld] r343732 - Minor refacotring of Relocations.cpp. NFC.

via llvm-commits llvm-commits at lists.llvm.org
Wed Oct 3 17:02:06 PDT 2018


Hi Rui,

This change seems to have broken the PS4 Windows bot. Can you take a look?

http://lab.llvm.org:8011/builders/llvm-clang-lld-x86_64-scei-ps4-windows10pro-fast/builds/20399/steps/build/logs/stdio

FAILED: tools/lld/ELF/CMakeFiles/lldELF.dir/Relocations.cpp.obj 
C:\PROGRA~2\MICROS~1.0\VC\bin\cl.exe  /nologo /TP -DGTEST_HAS_RTTI=0 -DUNICODE -D_CRT_NONSTDC_NO_DEPRECATE -D_CRT_NONSTDC_NO_WARNINGS -D_CRT_SECURE_NO_DEPRECATE -D_CRT_SECURE_NO_WARNINGS -D_FILE_OFFSET_BITS=64 -D_HAS_EXCEPTIONS=0 -D_LARGEFILE_SOURCE -D_SCL_SECURE_NO_DEPRECATE -D_SCL_SECURE_NO_WARNINGS -D_UNICODE -D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS -D__STDC_LIMIT_MACROS -Itools\lld\ELF -IC:\ps4-buildslave2\llvm-clang-lld-x86_64-scei-ps4-windows10pro-fast\llvm.src\tools\lld\ELF -IC:\ps4-buildslave2\llvm-clang-lld-x86_64-scei-ps4-windows10pro-fast\llvm.src\tools\lld\include -Itools\lld\include -Iinclude -IC:\ps4-buildslave2\llvm-clang-lld-x86_64-scei-ps4-windows10pro-fast\llvm.src\include /DWIN32 /D_WINDOWS   /Zc:inline /Zc:strictStrings /Oi /Zc:rvalueCast /W4 -wd4141 -wd4146 -wd4180 -wd4244 -wd4258 -wd4267 -wd4291 -wd4345 -wd4351 -wd4355 -wd4456 -wd4457 -wd4458 -wd4459 -wd4503 -wd4624 -wd4722 -wd4800 -wd4100 -wd4127 -wd4512 -wd4505 -wd4610 -wd4510 -wd4702 -wd4245 -wd4706 -wd4310 -wd4701 -wd4703 -wd4389 -wd4611 -wd4805 -wd4204 -wd4577 -wd4091 -wd4592 -wd4319 -wd4324 -w14062 -we4238 /MD /O2 /Ob2   -UNDEBUG -wd4530 -wd4062  /EHs-c- /GR- /showIncludes /Fotools\lld\ELF\CMakeFiles\lldELF.dir\Relocations.cpp.obj /Fdtools\lld\ELF\CMakeFiles\lldELF.dir\lldELF.pdb /FS -c C:\ps4-buildslave2\llvm-clang-lld-x86_64-scei-ps4-windows10pro-fast\llvm.src\tools\lld\ELF\Relocations.cpp
C:\ps4-buildslave2\llvm-clang-lld-x86_64-scei-ps4-windows10pro-fast\llvm.src\tools\lld\ELF\Relocations.cpp(1350): error C2143: syntax error: missing ';' before '}'

Douglas Yung

> -----Original Message-----
> From: llvm-commits [mailto:llvm-commits-bounces at lists.llvm.org] On
> Behalf Of Rui Ueyama via llvm-commits
> Sent: Wednesday, October 03, 2018 15:20
> To: llvm-commits at lists.llvm.org
> Subject: [lld] r343732 - Minor refacotring of Relocations.cpp. NFC.
> 
> Author: ruiu
> Date: Wed Oct  3 15:20:26 2018
> New Revision: 343732
> 
> URL: http://llvm.org/viewvc/llvm-project?rev=343732&view=rev
> Log:
> Minor refacotring of Relocations.cpp. NFC.
> 
> This patch splits ThunkCreator::mergeThunks into two smaller functions.
> Also adds blank lines to various places so that the code doesn't look
> too dense.
> 
> Modified:
>     lld/trunk/ELF/Relocations.cpp
> 
> Modified: lld/trunk/ELF/Relocations.cpp
> URL: http://llvm.org/viewvc/llvm-
> project/lld/trunk/ELF/Relocations.cpp?rev=343732&r1=343731&r2=343732&vi
> ew=diff
> =======================================================================
> =======
> --- lld/trunk/ELF/Relocations.cpp (original)
> +++ lld/trunk/ELF/Relocations.cpp Wed Oct  3 15:20:26 2018
> @@ -1065,6 +1065,29 @@ template <class ELFT> void elf::scanRelo
>      scanRelocs<ELFT>(S, S.rels<ELFT>());
>  }
> 
> +static bool mergeCmp(const InputSection *A, const InputSection *B) {
> +  // std::merge requires a strict weak ordering.
> +  if (A->OutSecOff < B->OutSecOff)
> +    return true;
> +
> +  if (A->OutSecOff == B->OutSecOff) {
> +    auto *TA = dyn_cast<ThunkSection>(A);
> +    auto *TB = dyn_cast<ThunkSection>(B);
> +
> +    // Check if Thunk is immediately before any specific Target
> +    // InputSection for example Mips LA25 Thunks.
> +    if (TA && TA->getTargetInputSection() == B)
> +      return true;
> +
> +    // Place Thunk Sections without specific targets before
> +    // non-Thunk Sections.
> +    if (TA && !TB && !TA->getTargetInputSection())
> +      return true;
> +  }
> +
> +  return false;
> +}
> +
>  // Thunk Implementation
>  //
>  // Thunks (sometimes called stubs, veneers or branch islands) are
> small pieces
> @@ -1167,6 +1190,7 @@ void ThunkCreator::mergeThunks(ArrayRef<
>                         [](const std::pair<ThunkSection *, uint32_t>
> &TS) {
>                           return TS.first->getSize() == 0;
>                         });
> +
>          // ISD->ThunkSections contains all created ThunkSections,
> including
>          // those inserted in previous passes. Extract the Thunks
> created this
>          // pass and order them in ascending OutSecOff.
> @@ -1182,27 +1206,11 @@ void ThunkCreator::mergeThunks(ArrayRef<
>          // Merge sorted vectors of Thunks and InputSections by
> OutSecOff
>          std::vector<InputSection *> Tmp;
>          Tmp.reserve(ISD->Sections.size() + NewThunks.size());
> -        auto MergeCmp = [](const InputSection *A, const InputSection
> *B) {
> -          // std::merge requires a strict weak ordering.
> -          if (A->OutSecOff < B->OutSecOff)
> -            return true;
> -          if (A->OutSecOff == B->OutSecOff) {
> -            auto *TA = dyn_cast<ThunkSection>(A);
> -            auto *TB = dyn_cast<ThunkSection>(B);
> -            // Check if Thunk is immediately before any specific
> Target
> -            // InputSection for example Mips LA25 Thunks.
> -            if (TA && TA->getTargetInputSection() == B)
> -              return true;
> -            if (TA && !TB && !TA->getTargetInputSection())
> -              // Place Thunk Sections without specific targets before
> -              // non-Thunk Sections.
> -              return true;
> -          }
> -          return false;
> -        };
> +
>          std::merge(ISD->Sections.begin(), ISD->Sections.end(),
>                     NewThunks.begin(), NewThunks.end(),
> std::back_inserter(Tmp),
> -                   MergeCmp);
> +                   mergeCmp);
> +
>          ISD->Sections = std::move(Tmp);
>        });
>  }
> @@ -1246,20 +1254,23 @@ ThunkSection *ThunkCreator::getISThunkSe
>    // Find InputSectionRange within Target Output Section (TOS) that
> the
>    // InputSection (IS) that we need to precede is in.
>    OutputSection *TOS = IS->getParent();
> -  for (BaseCommand *BC : TOS->SectionCommands)
> -    if (auto *ISD = dyn_cast<InputSectionDescription>(BC)) {
> -      if (ISD->Sections.empty())
> -        continue;
> -      InputSection *first = ISD->Sections.front();
> -      InputSection *last = ISD->Sections.back();
> -      if (IS->OutSecOff >= first->OutSecOff &&
> -          IS->OutSecOff <= last->OutSecOff) {
> -        TS = addThunkSection(TOS, ISD, IS->OutSecOff);
> -        ThunkedSections[IS] = TS;
> -        break;
> -      }
> -    }
> -  return TS;
> +  for (BaseCommand *BC : TOS->SectionCommands) {
> +    auto *ISD = dyn_cast<InputSectionDescription>(BC);
> +    if (!ISD || ISD->Sections.empty())
> +      continue;
> +
> +    InputSection *First = ISD->Sections.front();
> +    InputSection *Last = ISD->Sections.back();
> +
> +    if (IS->OutSecOff < First->OutSecOff || Last->OutSecOff < IS-
> >OutSecOff)
> +      continue;
> +
> +    TS = addThunkSection(TOS, ISD, IS->OutSecOff);
> +    ThunkedSections[IS] = TS;
> +    return TS;
> +  }
> +
> +  return nullptr;
>  }
> 
>  // Create one or more ThunkSections per OS that can be used to place
> Thunks.
> @@ -1281,10 +1292,12 @@ ThunkSection *ThunkCreator::getISThunkSe
>  void ThunkCreator::createInitialThunkSections(
>      ArrayRef<OutputSection *> OutputSections) {
>    uint32_t ThunkSectionSpacing = Target->getThunkSectionSpacing();
> +
>    forEachInputSectionDescription(
>        OutputSections, [&](OutputSection *OS, InputSectionDescription
> *ISD) {
>          if (ISD->Sections.empty())
>            return;
> +
>          uint32_t ISDBegin = ISD->Sections.front()->OutSecOff;
>          uint32_t ISDEnd =
>              ISD->Sections.back()->OutSecOff + ISD->Sections.back()-
> >getSize();
> @@ -1314,13 +1327,14 @@ ThunkSection *ThunkCreator::addThunkSect
>                                              InputSectionDescription
> *ISD,
>                                              uint64_t Off) {
>    auto *TS = make<ThunkSection>(OS, Off);
> -  ISD->ThunkSections.push_back(std::make_pair(TS, Pass));
> +  ISD->ThunkSections.push_back({TS, Pass});
>    return TS;
>  }
> 
>  std::pair<Thunk *, bool> ThunkCreator::getThunk(Symbol &Sym, RelType
> Type,
>                                                  uint64_t Src) {
>    std::vector<Thunk *> *ThunkVec = nullptr;
> +
>    // We use (section, offset) pair to find the thunk position if
> possible so
>    // that we create only one thunk for aliased symbols or ICFed
> sections.
>    if (auto *D = dyn_cast<Defined>(&Sym))
> @@ -1328,15 +1342,17 @@ std::pair<Thunk *, bool> ThunkCreator::g
>        ThunkVec = &ThunkedSymbolsBySection[{D->Section->Repl, D-
> >Value}];
>    if (!ThunkVec)
>      ThunkVec = &ThunkedSymbols[&Sym];
> +
>    // Check existing Thunks for Sym to see if they can be reused
> -  for (Thunk *ET : *ThunkVec)
> -    if (ET->isCompatibleWith(Type) &&
> -        Target->inBranchRange(Type, Src, ET->getThunkTargetSym()-
> >getVA()))
> -      return std::make_pair(ET, false);
> +  for (Thunk *T : *ThunkVec)
> +    if (T->isCompatibleWith(Type) &&
> +        Target->inBranchRange(Type, Src, T->getThunkTargetSym()-
> >getVA()))
> +      return {T, false};
> +
>    // No existing compatible Thunk in range, create a new one
>    Thunk *T = addThunk(Type, Sym);
>    ThunkVec->push_back(T);
> -  return std::make_pair(T, true);
> +  return {T, true};
>  }
> 
>  // Call Fn on every executable InputSection accessed via the linker
> script
> @@ -1358,10 +1374,10 @@ void ThunkCreator::forEachInputSectionDe
>  // was originally to a Thunk, but is no longer in range we revert the
>  // relocation back to its original non-Thunk target.
>  bool ThunkCreator::normalizeExistingThunk(Relocation &Rel, uint64_t
> Src) {
> -  if (Thunk *ET = Thunks.lookup(Rel.Sym)) {
> +  if (Thunk *T = Thunks.lookup(Rel.Sym)) {
>      if (Target->inBranchRange(Rel.Type, Src, Rel.Sym->getVA()))
>        return true;
> -    Rel.Sym = &ET->Destination;
> +    Rel.Sym = &T->Destination;
>      if (Rel.Sym->isInPlt())
>        Rel.Expr = toPlt(Rel.Expr);
>    }
> @@ -1395,11 +1411,13 @@ bool ThunkCreator::normalizeExistingThun
>  // relocation out of range error.
>  bool ThunkCreator::createThunks(ArrayRef<OutputSection *>
> OutputSections) {
>    bool AddressesChanged = false;
> +
>    if (Pass == 0 && Target->getThunkSectionSpacing())
>      createInitialThunkSections(OutputSections);
> -  else if (Pass == 10)
> -    // With Thunk Size much smaller than branch range we expect to
> -    // converge quickly; if we get to 10 something has gone wrong.
> +
> +  // With Thunk Size much smaller than branch range we expect to
> +  // converge quickly; if we get to 10 something has gone wrong.
> +  if (Pass == 10)
>      fatal("thunk creation not converged");
> 
>    // Create all the Thunks and insert them into synthetic
> ThunkSections. The
> @@ -1422,9 +1440,11 @@ bool ThunkCreator::createThunks(ArrayRef
>              if (!Target->needsThunk(Rel.Expr, Rel.Type, IS->File, Src,
>                                      *Rel.Sym))
>                continue;
> +
>              Thunk *T;
>              bool IsNew;
>              std::tie(T, IsNew) = getThunk(*Rel.Sym, Rel.Type, Src);
> +
>              if (IsNew) {
>                // Find or create a ThunkSection for the new Thunk
>                ThunkSection *TS;
> @@ -1435,13 +1455,16 @@ bool ThunkCreator::createThunks(ArrayRef
>                TS->addThunk(T);
>                Thunks[T->getThunkTargetSym()] = T;
>              }
> +
>              // Redirect relocation to Thunk, we never go via the PLT
> to a Thunk
>              Rel.Sym = T->getThunkTargetSym();
>              Rel.Expr = fromPlt(Rel.Expr);
>            }
> +
>          for (auto &P : ISD->ThunkSections)
>            AddressesChanged |= P.first->assignOffsets();
>        });
> +
>    for (auto &P : ThunkedSections)
>      AddressesChanged |= P.second->assignOffsets();
> 
> 
> 
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits


More information about the llvm-commits mailing list