[PATCH] D139092: [LLD][ELF] Cortex-M Security Extensions (CMSE) Support

[Amilendra Kodithuwakku via Phabricator via llvm-commits]

amilendra marked 4 inline comments as done.
amilendra added inline comments.


================
Comment at: lld/ELF/Arch/ARM.cpp:1335
+
+  for (ArmCmseSGVeneer *s : sgSections) {
+    if (!s->getAddr().has_value())
----------------
peter.smith wrote:
> IIUC at this point we have two partions:
> P1
> ordered by address existing implib veneers
> P2
> reverse ordered new veneers
> 
> I think you could simplify the code by using `std::reverse(it, sgSections.end());` on the second partition rather than traversing it via llvm::reverse()
> 
> Then I think you would only need one much simpler loop. 
> ```
> for (size_t i; i < sgSections.size() ++i) {
>   ArmCmseSGVeneer* s = sgSections[i];
>   s->offset = i * s->size();
>   Defined(file, StringRef(), s->sym->binding, s->sym->stOther, s->sym->type,
>             s->offset | 1, s->size(), this)
>         .overwrite(*s->sym);
> }
> ```
Actually the reverse is also no longer needed. Thanks for the suggestion.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D139092/new/

https://reviews.llvm.org/D139092