[PATCH] D61698: [COFF] Store alignment in log2 form, NFC

Reid Kleckner via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon May 20 15:54:56 PDT 2019


rnk marked 2 inline comments as done.
rnk added inline comments.


================
Comment at: lld/COFF/Chunks.cpp:865
 void MergeChunk::addSection(SectionChunk *C) {
-  auto *&MC = Instances[C->Alignment];
+  assert(isPowerOf2_32(C->getAlignment()));
+  uint8_t P2Align = llvm::Log2_32(C->getAlignment());
----------------
aganea wrote:
> This is essentially the same code as `Chunk::setAlignment`, can you make it so that a static version of that function is used here instead?
I'm not sure it's worth it, it's only combining the assert with the log. The simplification I would prefer is to add back the log2 accessor, which is what I had originally, but I did this since it was requested.


================
Comment at: lld/COFF/Chunks.h:71
+    P2Align = llvm::Log2_32(Align);
+  }
+
----------------
aganea wrote:
> `assert(P2Align<=Log2MaxSectionAlignment)` ?
Added


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D61698





More information about the llvm-commits mailing list