This is an archive of the discontinued LLVM Phabricator instance.

[NFC][AMDGPU] Reduce includes dependencies.
ClosedPublic

Authored by dfukalov on Aug 23 2021, 3:51 PM.

Details

Summary
  1. Splitted out some parts of R600 target to separate modules/headers.
  2. Reduced some include lists in headers.
  3. Found and fixed issue with override GCNTargetMachine::getSubtargetImpl() and R600TargetMachine::getSubtargetImpl() had different return value type than base class.
  4. Minor forward declarations cleanup.

Diff Detail

Event Timeline

dfukalov created this revision.Aug 23 2021, 3:51 PM
dfukalov requested review of this revision.Aug 23 2021, 3:51 PM
Herald added a project: Restricted Project. · View Herald TranscriptAug 23 2021, 3:51 PM
Herald added a subscriber: wdng. · View Herald Transcript
foad accepted this revision.Aug 24 2021, 2:17 AM

No objections from me, seems like a good cleanup. Do you plan to do more of this? What's the end goal?

llvm/lib/Target/AMDGPU/R600.h
51

Add newline

llvm/lib/Target/AMDGPU/R600TargetTransformInfo.cpp
142

Add newline

This revision is now accepted and ready to land.Aug 24 2021, 2:17 AM
dfukalov updated this revision to Diff 368317.Aug 24 2021, 3:59 AM

Removed no new line messages.

dfukalov marked 2 inline comments as done.Aug 24 2021, 4:00 AM

No objections from me, seems like a good cleanup. Do you plan to do more of this? What's the end goal?

Yes, I'm going to try additional NFC split out of parts like R600DAGToDAGISel, R600MCInstLower etc.

The end goal is to reduce AMDGPU and R600 source dependencies to (i) make partial rebuilds faster and (ii) prepare to move R600 target out at some point in future.

Perhaps we should think about additional split of some our huge modules in the sense of (i).

foad added a comment.Aug 24 2021, 4:10 AM

Yes, I'm going to try additional NFC split out of parts like R600DAGToDAGISel, R600MCInstLower etc.

The end goal is to reduce AMDGPU and R600 source dependencies to (i) make partial rebuilds faster and (ii) prepare to move R600 target out at some point in future.

Perhaps we should think about additional split of some our huge modules in the sense of (i).

Sounds good, thanks!

dfukalov updated this revision to Diff 368570.Aug 25 2021, 12:58 AM

Fixed clang-tidy errors.

This revision was landed with ongoing or failed builds.Aug 25 2021, 2:02 AM
This revision was automatically updated to reflect the committed changes.