[PATCH] D67284: [LLD][ELF][ARM] Implement --fix-cortex-a8 to fix erratum 657417

[Fangrui Song via Phabricator via llvm-commits]

MaskRay added inline comments.


================
Comment at: ELF/ARMErrataFix.cpp:490
+    auto thumbSym = llvm::find_if(mapSyms, [&](const Defined *ms) {
+      return ms->getName().startswith("$t");
+    });
----------------
peter.smith wrote:
> MaskRay wrote:
> > Above you use `s->getName() == "$t" || s->getName().startswith("$t.");`
> I think using ms here is defensible. Beforehand we were processing all symbols, that may or may not be mapping symbols. Here we know that the symbol is a mapping symbol as we filtered them out earlier.
> 
> I don't mind changing if you'd prefer s instead of ms? 
`ms` as the variable name is fine.  Sorry, I should have been clearer. I meant why `.startswith("$t")` is used here. But I see the reason now:

Because the elements contain exclusively mapping symbols:

```
 if (!isArmMapSymbol(def) && !isThumbMapSymbol(def) &&
          !isDataMapSymbol(def))
        continue;
```

`ms->getName().startswith("$t");` should be sufficient.


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

https://reviews.llvm.org/D67284