[Openmp-commits] [PATCH] D150549: Move SubtargetFeature.h from MC to TargetParser

Wed May 17 05:30:50 PDT 2023

jobnoorman added a comment.

In D150549#4347056 <https://reviews.llvm.org/D150549#4347056>, @MaskRay wrote:

> This seems fine, but please consider making `MC/SubtargetFeature.h` a forwarding header temporarily.

To be clear: do you want me to check-in this forwarding header or should I just do it locally for the tests below? If the former, what exactly is the purpose of adding this header?

> Our official build system CMake doesn't do layering checking for headers (https://llvm.org/docs/CodingStandards.html#library-layering).
>
> Our unofficial build system Bazel supports layering checking <http://maskray.me/blog/2022-09-25-layering-check-with-clang>. You may edit `utils/bazel/llvm-project-overlay/llvm/BUILD.bazel` and see whether bazel reports a circular dependency. I think this patch is fine, but just in case.

I did the following:

- Add the forwarding header
- Update `BUILD.bazel` (I only had to add the `TargetParser` dependency to `llvm-tblgen` to make this work; should I add this change to this patch?)
- Run `bazel build '@llvm-project//llvm:all'` (using `clang` from `main`).

This worked fine. Is this what you meant with the circular dependency check? (I'm not familiar with bazel so I might be missing something.)

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150549