[libunwind] 8ffdabd - Lazily save initialState of registers during unwind.

Sterling Augustine via cfe-commits cfe-commits at lists.llvm.org
Wed Mar 11 10:13:59 PDT 2020


Author: Sterling Augustine
Date: 2020-03-11T10:13:33-07:00
New Revision: 8ffdabdb61e1e2251b07ee93a4806cde1890f47d

URL: https://github.com/llvm/llvm-project/commit/8ffdabdb61e1e2251b07ee93a4806cde1890f47d
DIFF: https://github.com/llvm/llvm-project/commit/8ffdabdb61e1e2251b07ee93a4806cde1890f47d.diff

LOG: Lazily save initialState of registers during unwind.

Summary:
Copying all of the saved register state on every entry to
parseInstruction is a severe performance contraint, especially
because most of this saved state is never used. On x86 linux
this is about 560 bytes, and will be more on other platforms.

When performance testing libunwind, this memcpy appears at the
top of nearly all our tests.

By only saving this state as needed, we see increasing in performance
of around 2.5% for the ctak test here.

https://github.com/clasp-developers/ctak

Certain internal extremely exception-heavy tasks run in about 2/3
the time.

Note that by stashing the new boolean inside what had been padding in
the original structure, this uses no additional memory.

Subscribers: fedor.sergeev, libcxx-commits

Tags: #libc

Differential Revision: https://reviews.llvm.org/D75692

Added: 
    

Modified: 
    libunwind/src/DwarfParser.hpp

Removed: 
    


################################################################################
diff  --git a/libunwind/src/DwarfParser.hpp b/libunwind/src/DwarfParser.hpp
index 2994bd7bb41f..4a64c219255c 100644
--- a/libunwind/src/DwarfParser.hpp
+++ b/libunwind/src/DwarfParser.hpp
@@ -77,6 +77,7 @@ class CFI_Parser {
   };
   struct RegisterLocation {
     RegisterSavedWhere location;
+    bool initialStateSaved;
     int64_t value;
   };
   /// Information about a frame layout and registers saved determined
@@ -90,6 +91,40 @@ class CFI_Parser {
     bool              registersInOtherRegisters;
     bool              sameValueUsed;
     RegisterLocation  savedRegisters[kMaxRegisterNumber + 1];
+    enum class InitializeTime { kLazy, kNormal };
+
+    // When saving registers, this data structure is lazily initialized.
+    PrologInfo(InitializeTime IT = InitializeTime::kNormal) {
+      if (IT == InitializeTime::kNormal)
+        memset(this, 0, sizeof(*this));
+    }
+    void checkSaveRegister(uint64_t reg, PrologInfo &initialState) {
+      if (!savedRegisters[reg].initialStateSaved) {
+        initialState.savedRegisters[reg] = savedRegisters[reg];
+        savedRegisters[reg].initialStateSaved = true;
+      }
+    }
+    void setRegister(uint64_t reg, RegisterSavedWhere newLocation,
+                     int64_t newValue, PrologInfo &initialState) {
+      checkSaveRegister(reg, initialState);
+      savedRegisters[reg].location = newLocation;
+      savedRegisters[reg].value = newValue;
+    }
+    void setRegisterLocation(uint64_t reg, RegisterSavedWhere newLocation,
+                             PrologInfo &initialState) {
+      checkSaveRegister(reg, initialState);
+      savedRegisters[reg].location = newLocation;
+    }
+    void setRegisterValue(uint64_t reg, int64_t newValue,
+                          PrologInfo &initialState) {
+      checkSaveRegister(reg, initialState);
+      savedRegisters[reg].value = newValue;
+    }
+    void restoreRegisterToInitialState(uint64_t reg, PrologInfo &initialState) {
+      if (savedRegisters[reg].initialStateSaved)
+        savedRegisters[reg] = initialState.savedRegisters[reg];
+      // else the register still holds its initial state
+    }
   };
 
   struct PrologInfoStackEntry {
@@ -355,8 +390,6 @@ bool CFI_Parser<A>::parseFDEInstructions(A &addressSpace,
                                          const FDE_Info &fdeInfo,
                                          const CIE_Info &cieInfo, pint_t upToPC,
                                          int arch, PrologInfo *results) {
-  // clear results
-  memset(results, '\0', sizeof(PrologInfo));
   PrologInfoStackEntry *rememberStack = NULL;
 
   // parse CIE then FDE instructions
@@ -390,7 +423,9 @@ bool CFI_Parser<A>::parseInstructions(A &addressSpace, pint_t instructions,
                                       int arch, PrologInfo *results) {
   pint_t p = instructions;
   pint_t codeOffset = 0;
-  PrologInfo initialState = *results;
+  // initialState initialized as registers in results are modified. Use
+  // PrologInfo accessor functions to avoid reading uninitialized data.
+  PrologInfo initialState(PrologInfo::InitializeTime::kLazy);
 
   _LIBUNWIND_TRACE_DWARF("parseInstructions(instructions=0x%0" PRIx64 ")\n",
                          static_cast<uint64_t>(instructionsEnd));
@@ -443,8 +478,7 @@ bool CFI_Parser<A>::parseInstructions(A &addressSpace, pint_t instructions,
                 "malformed DW_CFA_offset_extended DWARF unwind, reg too big");
         return false;
       }
-      results->savedRegisters[reg].location = kRegisterInCFA;
-      results->savedRegisters[reg].value = offset;
+      results->setRegister(reg, kRegisterInCFA, offset, initialState);
       _LIBUNWIND_TRACE_DWARF("DW_CFA_offset_extended(reg=%" PRIu64 ", "
                              "offset=%" PRId64 ")\n",
                              reg, offset);
@@ -456,7 +490,7 @@ bool CFI_Parser<A>::parseInstructions(A &addressSpace, pint_t instructions,
             "malformed DW_CFA_restore_extended DWARF unwind, reg too big");
         return false;
       }
-      results->savedRegisters[reg] = initialState.savedRegisters[reg];
+      results->restoreRegisterToInitialState(reg, initialState);
       _LIBUNWIND_TRACE_DWARF("DW_CFA_restore_extended(reg=%" PRIu64 ")\n", reg);
       break;
     case DW_CFA_undefined:
@@ -466,7 +500,7 @@ bool CFI_Parser<A>::parseInstructions(A &addressSpace, pint_t instructions,
                 "malformed DW_CFA_undefined DWARF unwind, reg too big");
         return false;
       }
-      results->savedRegisters[reg].location = kRegisterUnused;
+      results->setRegisterLocation(reg, kRegisterUnused, initialState);
       _LIBUNWIND_TRACE_DWARF("DW_CFA_undefined(reg=%" PRIu64 ")\n", reg);
       break;
     case DW_CFA_same_value:
@@ -480,7 +514,7 @@ bool CFI_Parser<A>::parseInstructions(A &addressSpace, pint_t instructions,
       // "same value" means register was stored in frame, but its current
       // value has not changed, so no need to restore from frame.
       // We model this as if the register was never saved.
-      results->savedRegisters[reg].location = kRegisterUnused;
+      results->setRegisterLocation(reg, kRegisterUnused, initialState);
       // set flag to disable conversion to compact unwind
       results->sameValueUsed = true;
       _LIBUNWIND_TRACE_DWARF("DW_CFA_same_value(reg=%" PRIu64 ")\n", reg);
@@ -498,8 +532,8 @@ bool CFI_Parser<A>::parseInstructions(A &addressSpace, pint_t instructions,
                 "malformed DW_CFA_register DWARF unwind, reg2 too big");
         return false;
       }
-      results->savedRegisters[reg].location = kRegisterInRegister;
-      results->savedRegisters[reg].value = (int64_t)reg2;
+      results->setRegister(reg, kRegisterInRegister, (int64_t)reg2,
+                           initialState);
       // set flag to disable conversion to compact unwind
       results->registersInOtherRegisters = true;
       _LIBUNWIND_TRACE_DWARF(
@@ -576,8 +610,8 @@ bool CFI_Parser<A>::parseInstructions(A &addressSpace, pint_t instructions,
                 "malformed DW_CFA_expression DWARF unwind, reg too big");
         return false;
       }
-      results->savedRegisters[reg].location = kRegisterAtExpression;
-      results->savedRegisters[reg].value = (int64_t)p;
+      results->setRegister(reg, kRegisterAtExpression, (int64_t)p,
+                           initialState);
       length = addressSpace.getULEB128(p, instructionsEnd);
       assert(length < static_cast<pint_t>(~0) && "pointer overflow");
       p += static_cast<pint_t>(length);
@@ -595,8 +629,7 @@ bool CFI_Parser<A>::parseInstructions(A &addressSpace, pint_t instructions,
       }
       offset =
           addressSpace.getSLEB128(p, instructionsEnd) * cieInfo.dataAlignFactor;
-      results->savedRegisters[reg].location = kRegisterInCFA;
-      results->savedRegisters[reg].value = offset;
+      results->setRegister(reg, kRegisterInCFA, offset, initialState);
       _LIBUNWIND_TRACE_DWARF("DW_CFA_offset_extended_sf(reg=%" PRIu64 ", "
                              "offset=%" PRId64 ")\n",
                              reg, offset);
@@ -634,8 +667,7 @@ bool CFI_Parser<A>::parseInstructions(A &addressSpace, pint_t instructions,
       }
       offset = (int64_t)addressSpace.getULEB128(p, instructionsEnd)
                                                     * cieInfo.dataAlignFactor;
-      results->savedRegisters[reg].location = kRegisterOffsetFromCFA;
-      results->savedRegisters[reg].value = offset;
+      results->setRegister(reg, kRegisterOffsetFromCFA, offset, initialState);
       _LIBUNWIND_TRACE_DWARF("DW_CFA_val_offset(reg=%" PRIu64 ", "
                              "offset=%" PRId64 "\n",
                              reg, offset);
@@ -649,8 +681,7 @@ bool CFI_Parser<A>::parseInstructions(A &addressSpace, pint_t instructions,
       }
       offset =
           addressSpace.getSLEB128(p, instructionsEnd) * cieInfo.dataAlignFactor;
-      results->savedRegisters[reg].location = kRegisterOffsetFromCFA;
-      results->savedRegisters[reg].value = offset;
+      results->setRegister(reg, kRegisterOffsetFromCFA, offset, initialState);
       _LIBUNWIND_TRACE_DWARF("DW_CFA_val_offset_sf(reg=%" PRIu64 ", "
                              "offset=%" PRId64 "\n",
                              reg, offset);
@@ -662,8 +693,8 @@ bool CFI_Parser<A>::parseInstructions(A &addressSpace, pint_t instructions,
                 "malformed DW_CFA_val_expression DWARF unwind, reg too big");
         return false;
       }
-      results->savedRegisters[reg].location = kRegisterIsExpression;
-      results->savedRegisters[reg].value = (int64_t)p;
+      results->setRegister(reg, kRegisterIsExpression, (int64_t)p,
+                           initialState);
       length = addressSpace.getULEB128(p, instructionsEnd);
       assert(length < static_cast<pint_t>(~0) && "pointer overflow");
       p += static_cast<pint_t>(length);
@@ -685,8 +716,7 @@ bool CFI_Parser<A>::parseInstructions(A &addressSpace, pint_t instructions,
       }
       offset = (int64_t)addressSpace.getULEB128(p, instructionsEnd)
                                                     * cieInfo.dataAlignFactor;
-      results->savedRegisters[reg].location = kRegisterInCFA;
-      results->savedRegisters[reg].value = -offset;
+      results->setRegister(reg, kRegisterInCFA, -offset, initialState);
       _LIBUNWIND_TRACE_DWARF(
           "DW_CFA_GNU_negative_offset_extended(%" PRId64 ")\n", offset);
       break;
@@ -699,25 +729,27 @@ bool CFI_Parser<A>::parseInstructions(A &addressSpace, pint_t instructions,
     case DW_CFA_AARCH64_negate_ra_state:
       switch (arch) {
 #if defined(_LIBUNWIND_TARGET_AARCH64)
-      case REGISTERS_ARM64:
-        results->savedRegisters[UNW_ARM64_RA_SIGN_STATE].value ^= 0x1;
-        _LIBUNWIND_TRACE_DWARF("DW_CFA_AARCH64_negate_ra_state\n");
-        break;
+        case REGISTERS_ARM64: {
+          int64_t value =
+              results->savedRegisters[UNW_ARM64_RA_SIGN_STATE].value ^ 0x1;
+          results->setRegisterValue(UNW_ARM64_RA_SIGN_STATE, value, initialState);
+          _LIBUNWIND_TRACE_DWARF("DW_CFA_AARCH64_negate_ra_state\n");
+        } break;
 #endif
+
 #if defined(_LIBUNWIND_TARGET_SPARC)
       // case DW_CFA_GNU_window_save:
       case REGISTERS_SPARC:
         _LIBUNWIND_TRACE_DWARF("DW_CFA_GNU_window_save()\n");
         for (reg = UNW_SPARC_O0; reg <= UNW_SPARC_O7; reg++) {
-          results->savedRegisters[reg].location = kRegisterInRegister;
-          results->savedRegisters[reg].value =
-              ((int64_t)reg - UNW_SPARC_O0) + UNW_SPARC_I0;
+          results->setRegister(reg, kRegisterInRegister,
+                               ((int64_t)reg - UNW_SPARC_O0) + UNW_SPARC_I0,
+                               initialState);
         }
 
         for (reg = UNW_SPARC_L0; reg <= UNW_SPARC_I7; reg++) {
-          results->savedRegisters[reg].location = kRegisterInCFA;
-          results->savedRegisters[reg].value =
-              ((int64_t)reg - UNW_SPARC_L0) * 4;
+          results->setRegister(reg, kRegisterInCFA,
+                               ((int64_t)reg - UNW_SPARC_L0) * 4, initialState);
         }
         break;
 #endif
@@ -740,8 +772,7 @@ bool CFI_Parser<A>::parseInstructions(A &addressSpace, pint_t instructions,
         }
         offset = (int64_t)addressSpace.getULEB128(p, instructionsEnd)
                                                     * cieInfo.dataAlignFactor;
-        results->savedRegisters[reg].location = kRegisterInCFA;
-        results->savedRegisters[reg].value = offset;
+        results->setRegister(reg, kRegisterInCFA, offset, initialState);
         _LIBUNWIND_TRACE_DWARF("DW_CFA_offset(reg=%d, offset=%" PRId64 ")\n",
                                operand, offset);
         break;
@@ -758,7 +789,7 @@ bool CFI_Parser<A>::parseInstructions(A &addressSpace, pint_t instructions,
                   reg);
           return false;
         }
-        results->savedRegisters[reg] = initialState.savedRegisters[reg];
+        results->restoreRegisterToInitialState(reg, initialState);
         _LIBUNWIND_TRACE_DWARF("DW_CFA_restore(reg=%" PRIu64 ")\n",
                                static_cast<uint64_t>(operand));
         break;


        


More information about the cfe-commits mailing list