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

[Jez Ng via Phabricator via llvm-commits]

int3 added a comment.

seems like you're addressing the comments incrementally, lmk when this is ready for review



================
Comment at: lld/MachO/Writer.cpp:545
     }
+  } else if (sym != config->entry) {
+    llvm_unreachable("invalid branch target symbol type");
----------------
gkm wrote:
> int3 wrote:
> > gkm wrote:
> > > 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);
> > >   . . .
> > > ```
> > ah. I think it'd make more sense not to call `prepareBranchTarget` with an undefined symbol in the first place...
> Dropping the call to `prepareBranchTarget(config->entry)` causes an assert in `lld/test/MachO/entry-symbol.s`. We can debug that later.
Well I looked into it :) Didn't see the assert but noticed another issue: D102137


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100818