[PATCH] D154501: [JITLink][RISCV] Move relax to PostAllocationPasses

[Lang Hames via Phabricator via llvm-commits]

lhames accepted this revision.
lhames added a comment.
This revision is now accepted and ready to land.

LGTM.

> afaict, llvm-jitlink gathers symbol values during a custom
> post-fixup pass [1]. Therefore, it isn't affected by this bug and I'm
> not quite sure how to add a test for it. Any suggestions?

One approach would be to move the anonymous pass that calls `register<Format>GraphInfo` in `llvm-jitlink.cpp` to the post-allocation phase. If I do that I can immediately see the relaxation testcases failing. That doesn't work in general though because later phases (pre- and post-fixup) might introduce new symbols that `llvm-jitlink` needs to know about -- I see other testcases breaking because of this.

Maybe we could add a post-allocation pass in llvm-jitlink that records the initial addresses, then have `register<Format>GraphInfo` throw an error if any of the previously resolved symbols has changed address.

It'd be even better if we could catch this in an assert in LinkGraph itself. If we modified the `Section` class to have a pointer back to the `LinkGraph` and modified the `LinkGraph` to track the phase, then we could write checks like:

  void Addressable::setAddress(ExecutorAddr Adds) {
    assert(
      (!isDefined() || static_cast<Block&>(*this).getSection().getGraph().getPhase() < PreFixupPass) &&
      "Cannot change defined symbol address after symbol resolution");
  }

I think you can land this without waiting for that though. :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D154501