[PATCH] D67620: [Alignment][NFC] Remove LogAlignment functions

[Clement Courbet via Phabricator via llvm-commits]

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