[PATCH] D100818: [lld-macho] Implement branch-range-extension thunks

[Greg McGary via Phabricator via llvm-commits]

gkm marked 10 inline comments as done.
gkm added inline comments.


================
Comment at: lld/MachO/MergedOutputSection.h:20
 
+class Defined;
+
----------------
int3 wrote:
> leftover?
Not anymore.


================
Comment at: lld/MachO/Writer.cpp:545
     }
+  } else if (sym != config->entry) {
+    llvm_unreachable("invalid branch target symbol type");
----------------
int3 wrote:
> why is this `config->entry` check necessary?
Because of this ...
```
template <class LP> void Writer::run() {
  prepareBranchTarget(config->entry);
  . . .
```
... and this ...
```
bool macho::link(ArrayRef<const char *> argsArr, bool canExitEarly,
                 raw_ostream &stdoutOS, raw_ostream &stderrOS) {
  . . .
  config->entry = symtab->addUndefined(args.getLastArgValue(OPT_e, "_main"),
                                       /*file=*/nullptr,
                                       /*isWeakRef=*/false);
  . . .
```


================
Comment at: lld/MachO/Writer.cpp:564
     prepareBranchTarget(sym);
+    r.isCallSite = true;
+    isec->callSiteCount++;
----------------
int3 wrote:
> can't we just check for `relocAttrs.hasAttr(RelocAttrBits::BRANCH)` instead of adding a new property on Reloc?
After seeing your abandoned diff about the performance drag of checking `relocAttrs`, I chose to save the work of doing so again. Perhaps it is a micro-efficiency that I should avoid?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100818