This is an archive of the discontinued LLVM Phabricator instance.

[NFC][TargetParser] Remove llvm/Support/AArch64TargetParser.h
ClosedPublic

Authored by lenary on Jan 4 2023, 8:51 AM.

Details

Summary

Removes the forwarding header llvm/Support/AArch64TargetParser.h.

I am proposing to do this for all the forwarding headers left after
rGf09cf34d00625e57dea5317a3ac0412c07292148 - for each header:

  • Update all relevant in-tree includes
  • Remove the forwarding Header

Diff Detail

Event Timeline

lenary created this revision.Jan 4 2023, 8:51 AM
Herald added a project: Restricted Project. · View Herald TranscriptJan 4 2023, 8:51 AM
lenary requested review of this revision.Jan 4 2023, 8:51 AM
Herald added projects: Restricted Project, Restricted Project, Restricted Project. · View Herald TranscriptJan 4 2023, 8:51 AM

Adding some relevant reviewers.

My plan is to land the other patches like this directly onto main, as they are NFC and a cleanup. I hope this is uncontroversial, but let me know if you think it wouldn't be.

lenary updated this revision to Diff 486324.Jan 4 2023, 9:45 AM
lenary edited the summary of this revision. (Show Details)
lenary edited the summary of this revision. (Show Details)Jan 4 2023, 9:45 AM
pratlucas accepted this revision.Jan 5 2023, 1:55 AM

LGTM.

This revision is now accepted and ready to land.Jan 5 2023, 1:55 AM
MaskRay accepted this revision.EditedJan 5 2023, 3:52 PM

I don't think it is necessary to deprecate the old header then delete it after 16.0.0 is branched.
llvm/Support/AArch64TargetParser.h has very few open-source out-of-tree uses. Perhaps only ldc driver/targetmachine.cpp uses the header. So it is not worth extra expedience.
Just deleting it in another change should be fine.

Changing the include to #include "llvm/TargetParser/AArch64TargetParser.h" is totally fine, though.

lenary added a comment.Feb 3 2023, 5:41 AM

I don't think it is necessary to deprecate the old header then delete it after 16.0.0 is branched.
llvm/Support/AArch64TargetParser.h has very few open-source out-of-tree uses. Perhaps only ldc driver/targetmachine.cpp uses the header. So it is not worth extra expedience.
Just deleting it in another change should be fine.

Changing the include to #include "llvm/TargetParser/AArch64TargetParser.h" is totally fine, though.

Sorry, busy time on downstream work means I haven't got back to this.

My plan now is:

  • Rebase this to not care about the modulemap patch it is based on, as that is getting no review at all, despite being pinged.
  • Just start deleting the forwarding headers, as [NFC] commits, updating all the references to the new headers. We're after the 16 branch point, so I think that's been a long enough cut-over period.

At some point, people will need to fix the bazel/modulemap builds, but I've tried the latter and got no traction on patches to do so.

lenary updated this revision to Diff 494596.Feb 3 2023, 5:50 AM
lenary retitled this revision from [NFC][TargetParser] Deprecate llvm/Support/AArch64TargetParser.h to [NFC][TargetParser] Remove llvm/Support/AArch64TargetParser.h.
lenary edited the summary of this revision. (Show Details)
lenary updated this revision to Diff 494600.Feb 3 2023, 5:57 AM
lenary added a comment.Feb 3 2023, 5:57 AM

Most recent diff was to clang-format the patch, which has removed some duplicate includes.

MaskRay accepted this revision.Feb 3 2023, 9:22 AM
lldb/source/Plugins/Disassembler/LLVMC/DisassemblerLLVMC.cpp