This is an archive of the discontinued LLVM Phabricator instance.

[NFC] Reduce include files dependency and AA header cleanup (part 2).
ClosedPublic

Authored by dfukalov on Dec 8 2020, 8:14 AM.

Details

Summary

Continuing work started in https://reviews.llvm.org/D92489:

Removed a bunch of includes from "AliasAnalysis.h" and "LoopPassManager.h".

Diff Detail

Event Timeline

dfukalov created this revision.Dec 8 2020, 8:14 AM
dfukalov requested review of this revision.Dec 8 2020, 8:14 AM
Herald added projects: Restricted Project, Restricted Project. · View Herald TranscriptDec 8 2020, 8:14 AM
Herald added a subscriber: cfe-commits. · View Herald Transcript
dfukalov added inline comments.Dec 8 2020, 8:19 AM
llvm/include/llvm/Analysis/AliasAnalysis.h
67

Just realized wrong order, will fix in updated patch.

RKSimon added inline comments.Dec 8 2020, 10:26 AM
llvm/include/llvm/Analysis/AliasAnalysis.h
775–776

Is this necessary? It doesn't seem to match the pattern used for all the other Instruction types.

dfukalov added inline comments.Dec 8 2020, 11:33 AM
llvm/include/llvm/Analysis/AliasAnalysis.h
775–776

Actually there are no getModRefInfo(CallInst * nor getModRefInfo(InvokeInst * but the only getModRefInfo(CallBase *.
There were two implicit casts from CallInst and InvokeInst in the calls to their base CallBase and it was masked by included Instructions.h.

My thought was we use explicit cast here so I decided to refine this calls.

This patch is very busy with 3 different aims - maybe split into 3 patches? The constifications, the include cleanup and the getModRefInfo tweak

dfukalov updated this revision to Diff 310506.Dec 9 2020, 5:57 AM

Splitting change as requested.

dfukalov updated this revision to Diff 310509.Dec 9 2020, 6:08 AM
dfukalov edited the summary of this revision. (Show Details)
RKSimon added inline comments.Dec 9 2020, 6:50 AM
llvm/lib/Analysis/AliasAnalysis.cpp
685

This should probably be pulled out too

dfukalov added inline comments.Dec 9 2020, 7:23 AM
llvm/lib/Analysis/AliasAnalysis.cpp
685

It uses dyn_cast<CallBase>(I) so clang-tidy reports incomplete type 'llvm::CallBase' named in nested name specifier if the function definition is in header. This is result of removing include Instructions.h from AliasAnalysis.h.

Actually there is one more same clang-tidy report on isa<AllocaInst> in MemorySSA.h that includes AliasAnalysis.h (and so included Instructions.h through it). I thought to fix it the same way - moving function with isa<AllocaInst> to MemorySSA.cpp

Or should I leave include Instructions.h in AliasAnalysis.h?

RKSimon accepted this revision.Dec 16 2020, 3:46 AM

LGTM

llvm/lib/Analysis/AliasAnalysis.cpp
685

No, moving into a cpp file make sense - sorry for the noise.

This revision is now accepted and ready to land.Dec 16 2020, 3:46 AM
This revision was landed with ongoing or failed builds.Dec 17 2020, 3:05 AM
This revision was automatically updated to reflect the committed changes.