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

Job Noorman via Phabricator via Openmp-commits openmp-commits at lists.llvm.org
Thu May 18 01:28:24 PDT 2023

jobnoorman added a comment.

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

> In D150549#4349531 <https://reviews.llvm.org/D150549#4349531>, @jobnoorman wrote:
>> 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?
> The motivation is similar to https://reviews.llvm.org/D137838#3921828
>> That way, you don't have to update oodles of include lines in this patch, and it makes it a bit easier to see what's going on. (You can then update all the include lines in a trivial follow-up if this change goes through, and then remove the forwarding headers in Support, to cut the dependency you want to remove.)
> (There is a non-zero probability that the patch may miss something and get reverted. By creating a forwarding header we don't risk causing too much churn to the many files.)
> For a large-scaling refactoring

Thanks for the link! If I understand the discussion there correctly, the motivation for adding the forwarding header was to not have to update any `#include` sites directly in order to reduce the size of the patch. So what you are asking me to do is

1. Update this patch to have a forwarding header and //not// change `#include` sites;
2. Create a second patch that updates `#include`s and removes the forward. This patch would be committed at some point in the future once we're sure the move is fine.

Is this correct?

I do wonder if this is necessary in this case as the patch is much smaller than the one in the link you shared.

  rG LLVM Github Monorepo



More information about the Openmp-commits mailing list