- Place types and functions in the llvm::codelayout namespace
- Change EdgeCountT from pair<pair<uint64_t, uint64_t>, uint64_t> to a struct and utilize structured bindings. It is not conventional to use the "T" suffix for structure types.
- Remove a redundant copy in ChainT::merge.
- Change {ExtTSPImpl,CDSortImpl}::run to use return value instead of an output parameter
- Rename applyCDSLayout to computeCacheDirectedLayout: (a) avoid rare abbreviation "CDS" (cache-directed sort) (b) "compute" is more conventional for the specific use case
- Change the parameter types from std::vector to ArrayRef so that SmallVector arguments can be used.
- Similarly, rename applyExtTspLayout to computeExtTspLayout.
Details
Diff Detail
- Repository
- rG LLVM Github Monorepo
Event Timeline
New reviews will be disabled on Oct 1 (or later, as I may be the operator and I will be out of town...). For this change it is convenient to use Phabricator to link to D152840
place functions under llvm::codelayout::
change EdgeCountT to a struct named EdgeCount
llvm/lib/Transforms/Utils/CodeLayout.cpp | ||
---|---|---|
1410–1411 | I probably wouldn't use a structured binding if it's a struct with named members already (obfuscates that fact a bit, risks naming the bindings differently than the members, or having them otherwise get out of sync, etc). (similarly elsewhere in this change) |
Thank you for the refactoring. Looks good, but let me retrigger testing with the latest changes.
ping @spupyrev
Might be at the point where this should/could be a struct with named members?