Changeset 215310 in webkit for trunk/Source/JavaScriptCore


Ignore:
Timestamp:
Apr 12, 2017, 7:26:53 PM (8 years ago)
Author:
[email protected]
Message:

Move common stack allocation utilities out of AirAllocateStackByGraphColoring.cpp
​https://bugs.webkit.org/show_bug.cgi?id=170799

Reviewed by Michael Saboff and Keith Miller.

When I added stack allocation to allocateRegistersByLinearScan, I reused a handful of
utility functions from AirAllocateStackByGraphColoring.cpp. I accomplished this by
putting their declarations in AirAllocateStackByGraphColoring.h.

That was pretty weird.

This patch moves a family of stack allocation helper functions out of
AirAllocateStackByGraphColoring.cpp and into the new AirStackAllocation.h|cpp. The
linear scan stack allocator no longer has to include the other stack allocator's
header, which addresses my OCD.

I moved the functions transitively reachable from the two functions that the linear
scan allocator needed. This forced me to give them better names (i.e. no "fooBarImpl")
and short descriptive comments. I think that such comments are useful in code that is
doing a convoluted version of some theoretical concept.

No behavior change.

  • CMakeLists.txt:
  • JavaScriptCore.xcodeproj/project.pbxproj:
  • b3/air/AirAllocateRegistersAndStackByLinearScan.cpp:
  • b3/air/AirAllocateStackByGraphColoring.cpp:

(JSC::B3::Air::allocateStackByGraphColoring):
(JSC::B3::Air::allocateEscapedStackSlots): Deleted.
(JSC::B3::Air::updateFrameSizeBasedOnStackSlots): Deleted.

  • b3/air/AirAllocateStackByGraphColoring.h:
  • b3/air/AirStackAllocation.cpp: Added.

(JSC::B3::Air::attemptAssignment):
(JSC::B3::Air::assign):
(JSC::B3::Air::allocateAndGetEscapedStackSlotsWithoutChangingFrameSize):
(JSC::B3::Air::allocateEscapedStackSlots):
(JSC::B3::Air::updateFrameSizeBasedOnStackSlots):

  • b3/air/AirStackAllocation.h: Added.
Location:
trunk/Source/JavaScriptCore
Files:
2 added
6 edited

Legend:

Unmodified
Added
Removed

  • trunk/Source/JavaScriptCore/CMakeLists.txt

    r215292 r215310  
    110110    b3/air/AirSimplifyCFG.cpp
    111111    b3/air/AirSpecial.cpp
     112    b3/air/AirStackAllocation.cpp
    112113    b3/air/AirStackSlot.cpp
    113114    b3/air/AirStackSlotKind.cpp

  • trunk/Source/JavaScriptCore/ChangeLog

    r215292 r215310  
     12017-04-12  Filip Pizlo  <[email protected]>
     2
     3        Move common stack allocation utilities out of AirAllocateStackByGraphColoring.cpp
     4        https://bugs.webkit.org/show_bug.cgi?id=170799
     5
     6        Reviewed by Michael Saboff and Keith Miller.
     7       
     8        When I added stack allocation to allocateRegistersByLinearScan, I reused a handful of
     9        utility functions from AirAllocateStackByGraphColoring.cpp. I accomplished this by
     10        putting their declarations in AirAllocateStackByGraphColoring.h.
     11       
     12        That was pretty weird.
     13       
     14        This patch moves a family of stack allocation helper functions out of
     15        AirAllocateStackByGraphColoring.cpp and into the new AirStackAllocation.h|cpp. The
     16        linear scan stack allocator no longer has to include the other stack allocator's
     17        header, which addresses my OCD.
     18       
     19        I moved the functions transitively reachable from the two functions that the linear
     20        scan allocator needed. This forced me to give them better names (i.e. no "fooBarImpl")
     21        and short descriptive comments. I think that such comments are useful in code that is
     22        doing a convoluted version of some theoretical concept.
     23       
     24        No behavior change.
     25
     26        * CMakeLists.txt:
     27        * JavaScriptCore.xcodeproj/project.pbxproj:
     28        * b3/air/AirAllocateRegistersAndStackByLinearScan.cpp:
     29        * b3/air/AirAllocateStackByGraphColoring.cpp:
     30        (JSC::B3::Air::allocateStackByGraphColoring):
     31        (JSC::B3::Air::allocateEscapedStackSlots): Deleted.
     32        (JSC::B3::Air::updateFrameSizeBasedOnStackSlots): Deleted.
     33        * b3/air/AirAllocateStackByGraphColoring.h:
     34        * b3/air/AirStackAllocation.cpp: Added.
     35        (JSC::B3::Air::attemptAssignment):
     36        (JSC::B3::Air::assign):
     37        (JSC::B3::Air::allocateAndGetEscapedStackSlotsWithoutChangingFrameSize):
     38        (JSC::B3::Air::allocateEscapedStackSlots):
     39        (JSC::B3::Air::updateFrameSizeBasedOnStackSlots):
     40        * b3/air/AirStackAllocation.h: Added.
     41
    1422017-04-12  Filip Pizlo  <[email protected]>
    243

  • trunk/Source/JavaScriptCore/JavaScriptCore.xcodeproj/project.pbxproj

    r215292 r215310  
    458458                0F5CF9841E9D537700C18692 /* AirLowerStackArgs.h in Headers */ = {isa = PBXBuildFile; fileRef = 0F5CF9831E9D537500C18692 /* AirLowerStackArgs.h */; };
    459459                0F5CF9851E9D537A00C18692 /* AirLowerStackArgs.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 0F5CF9821E9D537500C18692 /* AirLowerStackArgs.cpp */; };
     460                0F5CF9881E9ED65000C18692 /* AirStackAllocation.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 0F5CF9861E9ED64E00C18692 /* AirStackAllocation.cpp */; };
     461                0F5CF9891E9ED65200C18692 /* AirStackAllocation.h in Headers */ = {isa = PBXBuildFile; fileRef = 0F5CF9871E9ED64E00C18692 /* AirStackAllocation.h */; };
    460462                0F5D085D1B8CF99D001143B4 /* DFGNodeOrigin.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 0F5D085C1B8CF99D001143B4 /* DFGNodeOrigin.cpp */; };
    461463                0F5EF91E16878F7A003E5C25 /* JITThunks.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 0F5EF91B16878F78003E5C25 /* JITThunks.cpp */; };
    … …  
    29972999                0F5CF9821E9D537500C18692 /* AirLowerStackArgs.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; name = AirLowerStackArgs.cpp; path = b3/air/AirLowerStackArgs.cpp; sourceTree = "<group>"; };
    29983000                0F5CF9831E9D537500C18692 /* AirLowerStackArgs.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; name = AirLowerStackArgs.h; path = b3/air/AirLowerStackArgs.h; sourceTree = "<group>"; };
     3001                0F5CF9861E9ED64E00C18692 /* AirStackAllocation.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; name = AirStackAllocation.cpp; path = b3/air/AirStackAllocation.cpp; sourceTree = "<group>"; };
     3002                0F5CF9871E9ED64E00C18692 /* AirStackAllocation.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; name = AirStackAllocation.h; path = b3/air/AirStackAllocation.h; sourceTree = "<group>"; };
    29993003                0F5D085C1B8CF99D001143B4 /* DFGNodeOrigin.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; name = DFGNodeOrigin.cpp; path = dfg/DFGNodeOrigin.cpp; sourceTree = "<group>"; };
    30003004                0F5EF91B16878F78003E5C25 /* JITThunks.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = JITThunks.cpp; sourceTree = "<group>"; };
    … …  
    56575661                                0FEC85621BDACDC70080FF74 /* AirSpecial.cpp */,
    56585662                                0FEC85631BDACDC70080FF74 /* AirSpecial.h */,
     5663                                0F5CF9861E9ED64E00C18692 /* AirStackAllocation.cpp */,
     5664                                0F5CF9871E9ED64E00C18692 /* AirStackAllocation.h */,
    56595665                                0FEC85661BDACDC70080FF74 /* AirStackSlot.cpp */,
    56605666                                0FEC85671BDACDC70080FF74 /* AirStackSlot.h */,
    … …  
    86148620                                E3FF75331D9CEA1800C7E16D /* DOMJITGetterSetter.h in Headers */,
    86158621                                E35CA1541DBC3A5C00F83516 /* DOMJITHeapRange.h in Headers */,
     8622                                0F5CF9891E9ED65200C18692 /* AirStackAllocation.h in Headers */,
    86168623                                E3C08E3C1DA41B810039478F /* DOMJITPatchpoint.h in Headers */,
    86178624                                534638711E70CF3D00F12AC1 /* JSRunLoopTimer.h in Headers */,
    … …  
    1090710914                                AD2FCBF61DB58DAD00B3E736 /* WebAssemblyMemoryPrototype.cpp in Sources */,
    1090810915                                AD2FCC001DB58DAD00B3E736 /* WebAssemblyModuleConstructor.cpp in Sources */,
     10916                                0F5CF9881E9ED65000C18692 /* AirStackAllocation.cpp in Sources */,
    1090910917                                AD2FCC021DB58DAD00B3E736 /* WebAssemblyModulePrototype.cpp in Sources */,
    1091010918                                AD4937C71DDD0AAE0077C807 /* WebAssemblyModuleRecord.cpp in Sources */,

  • trunk/Source/JavaScriptCore/b3/air/AirAllocateRegistersAndStackByLinearScan.cpp

    r215292 r215310  
    2929#if ENABLE(B3_JIT)
    3030
    31 #include "AirAllocateStackByGraphColoring.h"
    3231#include "AirArgInlines.h"
    3332#include "AirCode.h"
    … …  
    4039#include "AirPhaseScope.h"
    4140#include "AirRegLiveness.h"
     41#include "AirStackAllocation.h"
    4242#include "AirTmpInlines.h"
    4343#include "AirTmpMap.h"

  • trunk/Source/JavaScriptCore/b3/air/AirAllocateStackByGraphColoring.cpp

    r215292 r215310  
    3535#include "AirLiveness.h"
    3636#include "AirPhaseScope.h"
     37#include "AirStackAllocation.h"
    3738#include "StackAlignment.h"
    3839#include <wtf/ListDump.h>
    … …  
    4344
    4445const bool verbose = false;
    45 
    46 bool attemptAssignment(
    47     StackSlot* slot, intptr_t offsetFromFP, const Vector<StackSlot*>& otherSlots)
    48 {
    49     if (verbose)
    50         dataLog("Attempting to assign ", pointerDump(slot), " to ", offsetFromFP, " with interference ", pointerListDump(otherSlots), "\n");
    51 
    52     // Need to align it to the slot's desired alignment.
    53     offsetFromFP = -WTF::roundUpToMultipleOf(slot->alignment(), -offsetFromFP);
    54    
    55     for (StackSlot* otherSlot : otherSlots) {
    56         if (!otherSlot->offsetFromFP())
    57             continue;
    58         bool overlap = WTF::rangesOverlap(
    59             offsetFromFP,
    60             offsetFromFP + static_cast<intptr_t>(slot->byteSize()),
    61             otherSlot->offsetFromFP(),
    62             otherSlot->offsetFromFP() + static_cast<intptr_t>(otherSlot->byteSize()));
    63         if (overlap)
    64             return false;
    65     }
    66 
    67     if (verbose)
    68         dataLog("Assigned ", pointerDump(slot), " to ", offsetFromFP, "\n");
    69     slot->setOffsetFromFP(offsetFromFP);
    70     return true;
    71 }
    72 
    73 void assign(StackSlot* slot, const Vector<StackSlot*>& otherSlots)
    74 {
    75     if (verbose)
    76         dataLog("Attempting to assign ", pointerDump(slot), " with interference ", pointerListDump(otherSlots), "\n");
    77    
    78     if (attemptAssignment(slot, -static_cast<intptr_t>(slot->byteSize()), otherSlots))
    79         return;
    80 
    81     for (StackSlot* otherSlot : otherSlots) {
    82         if (!otherSlot->offsetFromFP())
    83             continue;
    84         bool didAssign = attemptAssignment(
    85             slot, otherSlot->offsetFromFP() - static_cast<intptr_t>(slot->byteSize()), otherSlots);
    86         if (didAssign)
    87             return;
    88     }
    89 
    90     RELEASE_ASSERT_NOT_REACHED();
    91 }
    9246
    9347struct CoalescableMove {
    … …  
    13084};
    13185
    132 Vector<StackSlot*> allocateEscapedStackSlotsImpl(Code& code)
    133 {
    134     // Allocate all of the escaped slots in order. This is kind of a crazy algorithm to allow for
    135     // the possibility of stack slots being assigned frame offsets before we even get here.
    136     RELEASE_ASSERT(!code.frameSize());
    137     Vector<StackSlot*> assignedEscapedStackSlots;
    138     Vector<StackSlot*> escapedStackSlotsWorklist;
    139     for (StackSlot* slot : code.stackSlots()) {
    140         if (slot->isLocked()) {
    141             if (slot->offsetFromFP())
    142                 assignedEscapedStackSlots.append(slot);
    143             else
    144                 escapedStackSlotsWorklist.append(slot);
    145         } else {
    146             // It would be super strange to have an unlocked stack slot that has an offset already.
    147             ASSERT(!slot->offsetFromFP());
    148         }
    149     }
    150     // This is a fairly expensive loop, but it's OK because we'll usually only have a handful of
    151     // escaped stack slots.
    152     while (!escapedStackSlotsWorklist.isEmpty()) {
    153         StackSlot* slot = escapedStackSlotsWorklist.takeLast();
    154         assign(slot, assignedEscapedStackSlots);
    155         assignedEscapedStackSlots.append(slot);
    156     }
    157     return assignedEscapedStackSlots;
    158 }
    159 
    160 template<typename Collection>
    161 void updateFrameSizeBasedOnStackSlotsImpl(Code& code, const Collection& collection)
    162 {
    163     unsigned frameSize = 0;
    164     for (StackSlot* slot : collection)
    165         frameSize = std::max(frameSize, static_cast<unsigned>(-slot->offsetFromFP()));
    166     code.setFrameSize(WTF::roundUpToMultipleOf(stackAlignmentBytes(), frameSize));
    167 }
    168 
    16986} // anonymous namespace
    170 
    171 void allocateEscapedStackSlots(Code& code)
    172 {
    173     updateFrameSizeBasedOnStackSlotsImpl(code, allocateEscapedStackSlotsImpl(code));
    174 }
    175 
    176 void updateFrameSizeBasedOnStackSlots(Code& code)
    177 {
    178     updateFrameSizeBasedOnStackSlotsImpl(code, code.stackSlots());
    179 }
    18087
    18188void allocateStackByGraphColoring(Code& code)
    … …  
    18592    handleCalleeSaves(code);
    18693   
    187     Vector<StackSlot*> assignedEscapedStackSlots = allocateEscapedStackSlotsImpl(code);
     94    Vector<StackSlot*> assignedEscapedStackSlots =
     95        allocateAndGetEscapedStackSlotsWithoutChangingFrameSize(code);
    18896
    18997    // Now we handle the spill slots.

  • trunk/Source/JavaScriptCore/b3/air/AirAllocateStackByGraphColoring.h

    r215292 r215310  
    4242void allocateStackByGraphColoring(Code&);
    4343
    44 // These are utilities shared by this phase and allocateRegistersAndStackByLinearScan().
    45 void allocateEscapedStackSlots(Code&);
    46 void updateFrameSizeBasedOnStackSlots(Code&);
    47 
    4844} } } // namespace JSC::B3::Air
    4945
Note: See TracChangeset for help on using the changeset viewer.