[PATCH] D156488: [PPC] Fix layering issues between MCTargetDesc and CodeGen

Thu Jul 27 16:11:12 PDT 2023

rnk added a comment.

In D156488#4540742 <https://reviews.llvm.org/D156488#4540742>, @nemanjai wrote:

> Thank you for the patch. This is actually interesting and is a bit of a change to how we thought the code was do be structured. For us the affinity between CodeGen and MCTargetDesc was an indication that the libraries, although separate, are dependent on each other. Of course, this is not necessary so I am in favour of this refactoring.
>
> I don't want to approve this patch immediately, not because I am opposed to it, but because I think it should be seen and reviewed by all interested developers for the PowerPC back end so that we can change the mindset about these libraries.

Sounds good. I kind of hacked this patch together, so there may be better places to put the code that I had to move around.

> Finally, I wonder if we can add some tests that would prevent us from forming these dependencies again in the future? Is there a buildbot that uses Bazel? Immediate notification when new dependencies are re-introduced is essential to ensuring we stay on the straight and narrow wrt. to these libraries.

There is both a bazel bot and a premerge check for Bazel, documented here:
https://github.com/llvm/llvm-project/tree/main/utils/bazel#pre-merge-testing

You have to opt in to receiving Bazel build notifications on your changes, because the understanding is that LLVM developers are not generally expected to maintain the Bazel build.

Once I land the change to remove the CodeGen dep from the *UtilsAndDesc libraries, including codegen headers from MCTargetDesc will break the bazel build, so at the very least, a Bazel user will notice and will take action.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156488