[PATCH] D67620: [Alignment][NFC] Remove LogAlignment functions
Clement Courbet via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Tue Sep 17 02:19:36 PDT 2019
courbet requested changes to this revision.
courbet added inline comments.
This revision now requires changes to proceed.
================
Comment at: llvm/lib/Target/ARM/ARMBasicBlockInfo.cpp:129
// Include the alignment of the current block.
- unsigned LogAlign = MF.getBlockNumbered(i)->getLogAlignment();
- unsigned Offset = BBInfo[i - 1].postOffset(LogAlign);
- unsigned KnownBits = BBInfo[i - 1].postKnownBits(LogAlign);
+ const llvm::Align Align = MF.getBlockNumbered(i)->getAlignment();
+ const unsigned LogAlign = Log2(Align);
----------------
inline ?
================
Comment at: llvm/lib/Target/ARM/ARMConstantIslandPass.cpp:488
// MachineConstantPool measures alignment in bytes. We measure in log2(bytes).
+ const llvm::Align MaxAlign(MCP->getConstantPoolAlignment());
----------------
Remove `We measure in log2(bytes).`
================
Comment at: llvm/lib/Target/ARM/ARMConstantIslandPass.cpp:528
// CPEMI. This is bucket sort with iterators.
- for (unsigned a = LogAlign + 1; a <= MaxLogAlign; ++a)
+ for (unsigned a = LogAlign + 1; a <= Log2(MaxAlign); ++a)
if (InsPoint[a] == InsAt)
----------------
you might want to cache the end bound.
================
Comment at: llvm/lib/Target/ARM/ARMConstantIslandPass.cpp:643
+/// getCPEAlign - Returns the required alignment of the constant pool entry
/// represented by CPEMI. Alignment is measured in log2(bytes) units.
+llvm::Align ARMConstantIslands::getCPEAlign(const MachineInstr *CPEMI) {
----------------
ditto - remove `Alignment is measured in log2(bytes) units.`
================
Comment at: llvm/lib/Target/ARM/ARMConstantIslandPass.cpp:649
case ARM::JUMPTABLE_TBB:
- return isThumb1 ? 2 : 0;
+ return isThumb1 ? llvm::Align(4) : llvm::Align(2);
case ARM::JUMPTABLE_TBH:
----------------
This should be `llvm::Align(1);`
================
Comment at: llvm/lib/Target/ARM/ARMConstantIslandPass.cpp:1016
+ const unsigned CPELogAlign = Log2(getCPEAlign(U.CPEMI));
unsigned CPEOffset = BBInfo[Water->getNumber()].postOffset(CPELogAlign);
unsigned NextBlockOffset;
----------------
As discussed, let's make `postOffset` take an Align to avoid escaping the type system.
================
Comment at: llvm/lib/Target/ARM/ARMConstantIslandPass.cpp:1561
// This block no longer needs to be aligned.
- CPEBB->setLogAlignment(0);
- } else
+ CPEBB->setAlignment(llvm::Align(1));
+ } else {
----------------
(for later) Consider introducing `static constexpr llvm::Align llvm::Align::None` (or `Unaligned`)
================
Comment at: llvm/lib/Target/Hexagon/HexagonBranchRelaxation.cpp:108
for (auto &B : MF) {
- if (B.getLogAlignment()) {
+ if (B.getAlignment() > 1) {
// Although we don't know the exact layout of the final code, we need
----------------
would be useful here: `if (B.getAlignment() != Align::None)`
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D67620/new/
https://reviews.llvm.org/D67620
More information about the llvm-commits
mailing list