[PATCH] D73498: [NFC] Remove extra headers included in Loop Unroll and LoopUnrollAndJam files

Reid Kleckner via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Jan 29 17:09:05 PST 2020


rnk added a comment.

I don't have time to review all these headers, but I looked at 3, and 2/3 are used, and would cause build errors if they did not happen to be pulled in by transitive includes. Generally, code should include what it uses, so 2/3 of the changes I looked at seem undesirable.

If you want to meaningfully make the build faster, I would do it like this:

- extract the command line to recompile the source file of interest (`rm lib/.../LoopUnrollAndJam.cpp.o` `ninja -v -n lib/.../LoopUnrollAndJam.cpp.o`) Paste it in a shell script to easily rerun.
- compile the file, count the number of lines in the .d file to approximate how many includes it actually needed
- remove one include from the source file
- recompile, recount number of lines in .d file. If it did not get smaller, put the header back.
- repeat.

In this way, you only ever make changes that actually make the build faster. It is also best to start by focusing on header files instead of cpp files, since they introduce transitive includes. Because of that, removing an include from a header has more impact on the overall build time.



================
Comment at: llvm/lib/Transforms/Scalar/LoopUnrollAndJamPass.cpp:14
 #include "llvm/Transforms/Scalar/LoopUnrollAndJamPass.h"
-#include "llvm/ADT/None.h"
-#include "llvm/ADT/STLExtras.h"
----------------
dmgreen wrote:
> These 4 are used in one way of another.
None is used in this file.


================
Comment at: llvm/lib/Transforms/Scalar/LoopUnrollAndJamPass.cpp:16
-#include "llvm/ADT/STLExtras.h"
-#include "llvm/ADT/SmallPtrSet.h"
-#include "llvm/ADT/StringRef.h"
----------------
SmallPtrSet is used in this file.


================
Comment at: llvm/lib/Transforms/Utils/LoopUnrollAndJam.cpp:26
 #include "llvm/IR/BasicBlock.h"
-#include "llvm/IR/DataLayout.h"
 #include "llvm/IR/DebugInfoMetadata.h"
----------------
This seems good, it doesn't seem used, so far as I can tell.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D73498/new/

https://reviews.llvm.org/D73498





More information about the llvm-commits mailing list