[llvm] r328812 - [MSF] Default to FPM2, and always mark FPM pages allocated.

Zachary Turner via llvm-commits llvm-commits at lists.llvm.org
Thu Mar 29 11:34:15 PDT 2018


Author: zturner
Date: Thu Mar 29 11:34:15 2018
New Revision: 328812

URL: http://llvm.org/viewvc/llvm-project?rev=328812&view=rev
Log:
[MSF] Default to FPM2, and always mark FPM pages allocated.

There are two FPMs in an MSF file, the idea being that for
incremental updates you can write to the alternate one and then
atomically swap them on commit.  LLVM defaulted to using FPM1
on the first commit, but this differs from Microsoft's behavior
which is to default to using FPM2 on the first commit.  To
eliminate some byte-level file differences, this patch changes
LLVM's default to also be FPM2.

Additionally, LLVM was trying to be "smart" about marking FPM
pages allocated.  In addition to marking every page belonging
to the alternate FPM as unallocated, LLVM also marked pages at
the end of the main FPM which were not needed as unallocated.

In order to match the behavior of Microsoft-generated PDBs, we
now always mark every FPM block as allocated, regardless of
whether it is in the main FPM or the alt FPM, and regardless of
whether or not it describes blocks which are actually in the file.

This has the side benefit of simplifying our code.

Modified:
    llvm/trunk/lib/DebugInfo/MSF/MSFBuilder.cpp
    llvm/trunk/test/DebugInfo/PDB/write-fpm.test
    llvm/trunk/unittests/DebugInfo/MSF/MSFBuilderTest.cpp

Modified: llvm/trunk/lib/DebugInfo/MSF/MSFBuilder.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/DebugInfo/MSF/MSFBuilder.cpp?rev=328812&r1=328811&r2=328812&view=diff
==============================================================================
--- llvm/trunk/lib/DebugInfo/MSF/MSFBuilder.cpp (original)
+++ llvm/trunk/lib/DebugInfo/MSF/MSFBuilder.cpp Thu Mar 29 11:34:15 2018
@@ -29,7 +29,7 @@ static const uint32_t kFreePageMap0Block
 static const uint32_t kFreePageMap1Block = 2;
 static const uint32_t kNumReservedPages = 3;
 
-static const uint32_t kDefaultFreePageMap = kFreePageMap0Block;
+static const uint32_t kDefaultFreePageMap = kFreePageMap1Block;
 static const uint32_t kDefaultBlockMapAddr = kNumReservedPages;
 
 MSFBuilder::MSFBuilder(uint32_t BlockSize, uint32_t MinBlockCount, bool CanGrow,
@@ -112,11 +112,11 @@ Error MSFBuilder::allocateBlocks(uint32_
     FreeBlocks.resize(NewBlockCount, true);
     // If we crossed over an fpm page, we actually need to allocate 2 extra
     // blocks for each FPM group crossed and mark both blocks from the group as
-    // used.  We may not actually use them since there are many more FPM blocks
-    // present than are required to represent all blocks in a given PDB, but we
-    // need to make sure they aren't allocated to a stream or something else.
-    // At the end when committing the PDB, we'll go through and mark the
-    // extraneous ones unused.
+    // used.  FPM blocks are marked as allocated regardless of whether or not
+    // they ultimately describe the status of blocks in the file.  This means
+    // that not only are extraneous blocks at the end of the main FPM marked as
+    // allocated, but also blocks from the alternate FPM are always marked as
+    // allocated.
     while (NextFpmBlock < NewBlockCount) {
       NewBlockCount += 2;
       FreeBlocks.resize(NewBlockCount, true);
@@ -244,19 +244,6 @@ uint32_t MSFBuilder::computeDirectoryByt
   return Size;
 }
 
-static void finalizeFpmBlockStatus(uint32_t B, ArrayRef<ulittle32_t> &FpmBlocks,
-                                   BitVector &Fpm) {
-  if (FpmBlocks.empty() || FpmBlocks.front() != B) {
-    Fpm.set(B);
-    return;
-  }
-
-  // If the next block in the actual layout is this block, it should *not* be
-  // free.
-  assert(!Fpm.test(B));
-  FpmBlocks = FpmBlocks.drop_front();
-}
-
 Expected<MSFLayout> MSFBuilder::build() {
   SuperBlock *SB = Allocator.Allocate<SuperBlock>();
   MSFLayout L;
@@ -315,19 +302,6 @@ Expected<MSFLayout> MSFBuilder::build()
     }
   }
 
-  // FPM blocks occur in pairs at every `BlockLength` interval.  While blocks of
-  // this form are reserved for FPM blocks, not all blocks of this form will
-  // actually be needed for FPM data because there are more blocks of this form
-  // than are required to represent a PDB file with a given number of blocks.
-  // So we need to find out which blocks are *actually* going to be real FPM
-  // blocks, then mark the reset of the reserved blocks as unallocated.
-  MSFStreamLayout FpmLayout = msf::getFpmStreamLayout(L, true);
-  auto FpmBlocks = makeArrayRef(FpmLayout.Blocks);
-  for (uint32_t B = kFreePageMap0Block; B < SB->NumBlocks;
-       B += msf::getFpmIntervalLength(L)) {
-    finalizeFpmBlockStatus(B, FpmBlocks, FreeBlocks);
-    finalizeFpmBlockStatus(B + 1, FpmBlocks, FreeBlocks);
-  }
   L.FreePageMap = FreeBlocks;
 
   return L;

Modified: llvm/trunk/test/DebugInfo/PDB/write-fpm.test
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/DebugInfo/PDB/write-fpm.test?rev=328812&r1=328811&r2=328812&view=diff
==============================================================================
--- llvm/trunk/test/DebugInfo/PDB/write-fpm.test (original)
+++ llvm/trunk/test/DebugInfo/PDB/write-fpm.test Thu Mar 29 11:34:15 2018
@@ -4,8 +4,8 @@
 
 CHECK:                             Free Page Map
 CHECK-NEXT: ============================================================
-CHECK-NEXT: Block 1 (
-CHECK-NEXT:   1000: 04F8FFFF FFFFFFFF FFFFFFFF FFFFFFFF FFFFFFFF FFFFFFFF FFFFFFFF FFFFFFFF  |................................|
-CHECK-NEXT:   1020: FFFFFFFF FFFFFFFF FFFFFFFF FFFFFFFF FFFFFFFF FFFFFFFF FFFFFFFF FFFFFFFF  |................................|
-CHECK:        1FE0: FFFFFFFF FFFFFFFF FFFFFFFF FFFFFFFF FFFFFFFF FFFFFFFF FFFFFFFF FFFFFFFF  |................................|
+CHECK-NEXT: Block 2 (
+CHECK-NEXT:   2000: 00F8FFFF FFFFFFFF FFFFFFFF FFFFFFFF FFFFFFFF FFFFFFFF FFFFFFFF FFFFFFFF  |................................|
+CHECK-NEXT:   2020: FFFFFFFF FFFFFFFF FFFFFFFF FFFFFFFF FFFFFFFF FFFFFFFF FFFFFFFF FFFFFFFF  |................................|
+CHECK:        2FE0: FFFFFFFF FFFFFFFF FFFFFFFF FFFFFFFF FFFFFFFF FFFFFFFF FFFFFFFF FFFFFFFF  |................................|
 CHECK:      )

Modified: llvm/trunk/unittests/DebugInfo/MSF/MSFBuilderTest.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/unittests/DebugInfo/MSF/MSFBuilderTest.cpp?rev=328812&r1=328811&r2=328812&view=diff
==============================================================================
--- llvm/trunk/unittests/DebugInfo/MSF/MSFBuilderTest.cpp (original)
+++ llvm/trunk/unittests/DebugInfo/MSF/MSFBuilderTest.cpp Thu Mar 29 11:34:15 2018
@@ -384,10 +384,9 @@ TEST_F(MSFBuilderTest, StreamDoesntUseFp
   EXPECT_EQ(StreamSize, L.StreamSizes[*SN]);
 
   for (uint32_t I = 0; I <= 3; ++I) {
-    // Pages from the regular FPM are allocated, while pages from the alt fpm
-    // are free.
+    // Pages from both FPMs are always allocated.
+    EXPECT_FALSE(L.FreePageMap.test(2 + I * 4096));
     EXPECT_FALSE(L.FreePageMap.test(1 + I * 4096));
-    EXPECT_TRUE(L.FreePageMap.test(2 + I * 4096));
   }
 
   for (uint32_t I = 1; I <= 3; ++I) {




More information about the llvm-commits mailing list