This is an archive of the discontinued LLVM Phabricator instance.

[NFC][AMDGPU] Reduce includes dependencies, part 2
ClosedPublic

Authored by dfukalov on Sep 7 2021, 3:26 AM.

Details

Summary
  1. Splitted out some parts of R600 target to separate modules/headers.
  2. Reduced some include lists in headers.
  3. Minor forward declarations, redundant includes and flags in GCNSubtarget cleanup.

Diff Detail

Event Timeline

dfukalov created this revision.Sep 7 2021, 3:26 AM
dfukalov requested review of this revision.Sep 7 2021, 3:26 AM
Herald added a reviewer: sstefan1. · View Herald Transcript
Herald added a reviewer: baziotis. · View Herald Transcript
Herald added a project: Restricted Project. · View Herald Transcript
Herald added a subscriber: wdng. · View Herald Transcript
foad added a comment.Sep 7 2021, 8:05 AM

Seems reasonable to me, but I haven't looked at all the details.

Drive by only.

llvm/lib/Target/AMDGPU/AMDGPUISelDAGToDAG.h
73

FWIW, I would recommend static, or anonymous namespaces rather than inline.

dfukalov updated this revision to Diff 371272.Sep 8 2021, 1:18 AM

Addressed comment.

dfukalov marked an inline comment as done.Sep 8 2021, 1:19 AM
arsenm added inline comments.Sep 15 2021, 2:37 PM
llvm/lib/Target/AMDGPU/AMDGPUISelDAGToDAG.h
15–16

What's the point of splitting this out? Nothing outside needs to access this

27

Bot is warning you to add inlines

dfukalov updated this revision to Diff 373094.Sep 16 2021, 3:41 PM

Fixed all clang-tidy warnings except "invalid case style".

dfukalov added inline comments.Sep 16 2021, 3:42 PM
llvm/lib/Target/AMDGPU/AMDGPUISelDAGToDAG.h
15–16

New "R600ISelDAGToDAG.cpp" uses class AMDGPUDAGToDAGISel declared below in the header.

foad accepted this revision.Oct 1 2021, 12:26 AM
This revision is now accepted and ready to land.Oct 1 2021, 12:26 AM
This revision was landed with ongoing or failed builds.Oct 1 2021, 7:50 AM
This revision was automatically updated to reflect the committed changes.