Title: [165293] trunk/Source/_javascript_Core
Revision
165293
Author
msab...@apple.com
Date
2014-03-07 14:22:46 -0800 (Fri, 07 Mar 2014)

Log Message

Clarify how we deal with "special" registers
https://bugs.webkit.org/show_bug.cgi?id=129806

Already reviewed change being relanded.

Relanding change set r165196 as it wasn't responsible for the breakage reported in
https://bugs.webkit.org/show_bug.cgi?id=129822.  That appears to be a build or

Reviewed by Michael Saboff.
configuration issue.

* assembler/ARM64Assembler.h:
(JSC::ARM64Assembler::lastRegister):
* assembler/MacroAssembler.h:
(JSC::MacroAssembler::nextRegister):
* ftl/FTLLocation.cpp:
(JSC::FTL::Location::restoreInto):
* ftl/FTLSaveRestore.cpp:
(JSC::FTL::saveAllRegisters):
(JSC::FTL::restoreAllRegisters):
* ftl/FTLSlowPathCall.cpp:
* jit/RegisterSet.cpp:
(JSC::RegisterSet::reservedHardwareRegisters):
(JSC::RegisterSet::runtimeRegisters):
(JSC::RegisterSet::specialRegisters):
(JSC::RegisterSet::calleeSaveRegisters):
* jit/RegisterSet.h:

Modified Paths

Diff

Modified: trunk/Source/_javascript_Core/ChangeLog (165292 => 165293)


--- trunk/Source/_javascript_Core/ChangeLog	2014-03-07 22:16:00 UTC (rev 165292)
+++ trunk/Source/_javascript_Core/ChangeLog	2014-03-07 22:22:46 UTC (rev 165293)
@@ -1,3 +1,33 @@
+2014-03-07  Michael Saboff  <msab...@apple.com>
+
+        Clarify how we deal with "special" registers
+        https://bugs.webkit.org/show_bug.cgi?id=129806
+
+        Already reviewed change being relanded.
+
+        Relanding change set r165196 as it wasn't responsible for the breakage reported in
+        https://bugs.webkit.org/show_bug.cgi?id=129822.  That appears to be a build or
+
+        Reviewed by Michael Saboff.
+        configuration issue.
+
+        * assembler/ARM64Assembler.h:
+        (JSC::ARM64Assembler::lastRegister):
+        * assembler/MacroAssembler.h:
+        (JSC::MacroAssembler::nextRegister):
+        * ftl/FTLLocation.cpp:
+        (JSC::FTL::Location::restoreInto):
+        * ftl/FTLSaveRestore.cpp:
+        (JSC::FTL::saveAllRegisters):
+        (JSC::FTL::restoreAllRegisters):
+        * ftl/FTLSlowPathCall.cpp:
+        * jit/RegisterSet.cpp:
+        (JSC::RegisterSet::reservedHardwareRegisters):
+        (JSC::RegisterSet::runtimeRegisters):
+        (JSC::RegisterSet::specialRegisters):
+        (JSC::RegisterSet::calleeSaveRegisters):
+        * jit/RegisterSet.h:
+
 2014-03-07  Mark Hahnenberg  <mhahnenb...@apple.com>
 
         Move GCActivityCallback to heap

Modified: trunk/Source/_javascript_Core/assembler/ARM64Assembler.h (165292 => 165293)


--- trunk/Source/_javascript_Core/assembler/ARM64Assembler.h	2014-03-07 22:16:00 UTC (rev 165292)
+++ trunk/Source/_javascript_Core/assembler/ARM64Assembler.h	2014-03-07 22:22:46 UTC (rev 165293)
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2012 Apple Inc. All rights reserved.
+ * Copyright (C) 2012, 2014 Apple Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
@@ -478,7 +478,7 @@
     typedef ARM64Registers::FPRegisterID FPRegisterID;
     
     static RegisterID firstRegister() { return ARM64Registers::x0; }
-    static RegisterID lastRegister() { return ARM64Registers::x28; }
+    static RegisterID lastRegister() { return ARM64Registers::sp; }
     
     static FPRegisterID firstFPRegister() { return ARM64Registers::q0; }
     static FPRegisterID lastFPRegister() { return ARM64Registers::q31; }

Modified: trunk/Source/_javascript_Core/assembler/MacroAssembler.h (165292 => 165293)


--- trunk/Source/_javascript_Core/assembler/MacroAssembler.h	2014-03-07 22:16:00 UTC (rev 165292)
+++ trunk/Source/_javascript_Core/assembler/MacroAssembler.h	2014-03-07 22:22:46 UTC (rev 165293)
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2008, 2012, 2013 Apple Inc. All rights reserved.
+ * Copyright (C) 2008, 2012, 2013, 2014 Apple Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
@@ -69,32 +69,11 @@
 class MacroAssembler : public MacroAssemblerBase {
 public:
 
-    static bool isStackRelated(RegisterID reg)
-    {
-        return reg == stackPointerRegister || reg == framePointerRegister;
-    }
-    
-    static RegisterID firstRealRegister()
-    {
-        RegisterID firstRegister = MacroAssembler::firstRegister();
-        while (MacroAssembler::isStackRelated(firstRegister))
-            firstRegister = static_cast<RegisterID>(firstRegister + 1);
-        return firstRegister;
-    }
-    
     static RegisterID nextRegister(RegisterID reg)
     {
-        RegisterID result = static_cast<RegisterID>(reg + 1);
-        while (MacroAssembler::isStackRelated(result))
-            result = static_cast<RegisterID>(result + 1);
-        return result;
+        return static_cast<RegisterID>(reg + 1);
     }
     
-    static RegisterID secondRealRegister()
-    {
-        return nextRegister(firstRealRegister());
-    }
-    
     static FPRegisterID nextFPRegister(FPRegisterID reg)
     {
         return static_cast<FPRegisterID>(reg + 1);

Modified: trunk/Source/_javascript_Core/ftl/FTLLocation.cpp (165292 => 165293)


--- trunk/Source/_javascript_Core/ftl/FTLLocation.cpp	2014-03-07 22:16:00 UTC (rev 165292)
+++ trunk/Source/_javascript_Core/ftl/FTLLocation.cpp	2014-03-07 22:22:46 UTC (rev 165293)
@@ -29,6 +29,7 @@
 #if ENABLE(FTL_JIT)
 
 #include "FTLSaveRestore.h"
+#include "RegisterSet.h"
 #include <wtf/CommaPrinter.h>
 #include <wtf/DataLog.h>
 #include <wtf/ListDump.h>
@@ -157,7 +158,7 @@
 
 void Location::restoreInto(MacroAssembler& jit, char* savedRegisters, GPRReg result, unsigned numFramesToPop) const
 {
-    if (involvesGPR() && MacroAssembler::isStackRelated(gpr())) {
+    if (involvesGPR() && RegisterSet::stackRegisters().get(gpr())) {
         // Make the result GPR contain the appropriate stack register.
         if (numFramesToPop) {
             jit.move(MacroAssembler::framePointerRegister, result);
@@ -174,7 +175,7 @@
     }
     
     if (isGPR()) {
-        if (MacroAssembler::isStackRelated(gpr())) {
+        if (RegisterSet::stackRegisters().get(gpr())) {
             // Already restored into result.
         } else
             jit.load64(savedRegisters + offsetOfGPR(gpr()), result);
@@ -197,7 +198,7 @@
         return;
         
     case Indirect:
-        if (MacroAssembler::isStackRelated(gpr())) {
+        if (RegisterSet::stackRegisters().get(gpr())) {
             // The stack register is already recovered into result.
             jit.load64(MacroAssembler::Address(result, offset()), result);
             return;

Modified: trunk/Source/_javascript_Core/ftl/FTLSaveRestore.cpp (165292 => 165293)


--- trunk/Source/_javascript_Core/ftl/FTLSaveRestore.cpp	2014-03-07 22:16:00 UTC (rev 165292)
+++ trunk/Source/_javascript_Core/ftl/FTLSaveRestore.cpp	2014-03-07 22:22:46 UTC (rev 165293)
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2013 Apple Inc. All rights reserved.
+ * Copyright (C) 2013, 2014 Apple Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
@@ -31,6 +31,7 @@
 #include "FPRInfo.h"
 #include "GPRInfo.h"
 #include "MacroAssembler.h"
+#include "RegisterSet.h"
 
 namespace JSC { namespace FTL {
 
@@ -69,38 +70,77 @@
     return offsetOfFPR(reg.fpr());
 }
 
+namespace {
+
+struct Regs {
+    Regs()
+    {
+        special = RegisterSet::stackRegisters();
+        special.merge(RegisterSet::reservedHardwareRegisters());
+        
+        first = MacroAssembler::firstRegister();
+        while (special.get(first))
+            first = MacroAssembler::nextRegister(first);
+        second = MacroAssembler::nextRegister(first);
+        while (special.get(second))
+            second = MacroAssembler::nextRegister(second);
+    }
+    
+    RegisterSet special;
+    GPRReg first;
+    GPRReg second;
+};
+
+} // anonymous namespace
+
 void saveAllRegisters(MacroAssembler& jit, char* scratchMemory)
 {
+    Regs regs;
+    
     // Get the first register out of the way, so that we can use it as a pointer.
-    jit.poke64(MacroAssembler::firstRealRegister(), 0);
-    jit.move(MacroAssembler::TrustedImmPtr(scratchMemory), MacroAssembler::firstRealRegister());
+    jit.poke64(regs.first, 0);
+    jit.move(MacroAssembler::TrustedImmPtr(scratchMemory), regs.first);
     
     // Get all of the other GPRs out of the way.
-    for (MacroAssembler::RegisterID reg = MacroAssembler::secondRealRegister(); reg <= MacroAssembler::lastRegister(); reg = MacroAssembler::nextRegister(reg))
-        jit.store64(reg, MacroAssembler::Address(MacroAssembler::firstRealRegister(), offsetOfGPR(reg)));
+    for (MacroAssembler::RegisterID reg = regs.second; reg <= MacroAssembler::lastRegister(); reg = MacroAssembler::nextRegister(reg)) {
+        if (regs.special.get(reg))
+            continue;
+        jit.store64(reg, MacroAssembler::Address(regs.first, offsetOfGPR(reg)));
+    }
     
     // Restore the first register into the second one and save it.
-    jit.peek64(MacroAssembler::secondRealRegister(), 0);
-    jit.store64(MacroAssembler::secondRealRegister(), MacroAssembler::Address(MacroAssembler::firstRealRegister(), offsetOfGPR(MacroAssembler::firstRealRegister())));
+    jit.peek64(regs.second, 0);
+    jit.store64(regs.second, MacroAssembler::Address(regs.first, offsetOfGPR(regs.first)));
     
     // Finally save all FPR's.
-    for (MacroAssembler::FPRegisterID reg = MacroAssembler::firstFPRegister(); reg <= MacroAssembler::lastFPRegister(); reg = MacroAssembler::nextFPRegister(reg))
-        jit.storeDouble(reg, MacroAssembler::Address(MacroAssembler::firstRealRegister(), offsetOfFPR(reg)));
+    for (MacroAssembler::FPRegisterID reg = MacroAssembler::firstFPRegister(); reg <= MacroAssembler::lastFPRegister(); reg = MacroAssembler::nextFPRegister(reg)) {
+        if (regs.special.get(reg))
+            continue;
+        jit.storeDouble(reg, MacroAssembler::Address(regs.first, offsetOfFPR(reg)));
+    }
 }
 
 void restoreAllRegisters(MacroAssembler& jit, char* scratchMemory)
 {
+    Regs regs;
+    
     // Give ourselves a pointer to the scratch memory.
-    jit.move(MacroAssembler::TrustedImmPtr(scratchMemory), MacroAssembler::firstRealRegister());
+    jit.move(MacroAssembler::TrustedImmPtr(scratchMemory), regs.first);
     
     // Restore all FPR's.
-    for (MacroAssembler::FPRegisterID reg = MacroAssembler::firstFPRegister(); reg <= MacroAssembler::lastFPRegister(); reg = MacroAssembler::nextFPRegister(reg))
-        jit.loadDouble(MacroAssembler::Address(MacroAssembler::firstRealRegister(), offsetOfFPR(reg)), reg);
+    for (MacroAssembler::FPRegisterID reg = MacroAssembler::firstFPRegister(); reg <= MacroAssembler::lastFPRegister(); reg = MacroAssembler::nextFPRegister(reg)) {
+        if (regs.special.get(reg))
+            continue;
+        jit.loadDouble(MacroAssembler::Address(regs.first, offsetOfFPR(reg)), reg);
+    }
     
-    for (MacroAssembler::RegisterID reg = MacroAssembler::secondRealRegister(); reg <= MacroAssembler::lastRegister(); reg = MacroAssembler::nextRegister(reg))
-        jit.load64(MacroAssembler::Address(MacroAssembler::firstRealRegister(), offsetOfGPR(reg)), reg);
+    for (MacroAssembler::RegisterID reg = regs.second; reg <= MacroAssembler::lastRegister(); reg = MacroAssembler::nextRegister(reg)) {
+        if (regs.special.get(reg))
+            continue;
+        jit.load64(MacroAssembler::Address(regs.first, offsetOfGPR(reg)), reg);
+    }
     
-    jit.load64(MacroAssembler::Address(MacroAssembler::firstRealRegister(), offsetOfGPR(MacroAssembler::firstRealRegister())), MacroAssembler::firstRealRegister());
+    jit.load64(MacroAssembler::Address(regs.first, offsetOfGPR(regs.first)), regs.first);
 }
 
 } } // namespace JSC::FTL

Modified: trunk/Source/_javascript_Core/ftl/FTLSlowPathCall.cpp (165292 => 165293)


--- trunk/Source/_javascript_Core/ftl/FTLSlowPathCall.cpp	2014-03-07 22:16:00 UTC (rev 165292)
+++ trunk/Source/_javascript_Core/ftl/FTLSlowPathCall.cpp	2014-03-07 22:22:46 UTC (rev 165293)
@@ -52,8 +52,9 @@
         , m_numArgs(numArgs)
         , m_returnRegister(returnRegister)
     {
-        // We don't care that you're using callee-save or stack registers.
+        // We don't care that you're using callee-save, stack, or hardware registers.
         m_usedRegisters.exclude(RegisterSet::stackRegisters());
+        m_usedRegisters.exclude(RegisterSet::reservedHardwareRegisters());
         m_usedRegisters.exclude(RegisterSet::calleeSaveRegisters());
         
         // The return register doesn't need to be saved.

Modified: trunk/Source/_javascript_Core/jit/RegisterSet.cpp (165292 => 165293)


--- trunk/Source/_javascript_Core/jit/RegisterSet.cpp	2014-03-07 22:16:00 UTC (rev 165292)
+++ trunk/Source/_javascript_Core/jit/RegisterSet.cpp	2014-03-07 22:22:46 UTC (rev 165293)
@@ -42,21 +42,34 @@
     return result;
 }
 
-RegisterSet RegisterSet::specialRegisters()
+RegisterSet RegisterSet::reservedHardwareRegisters()
 {
     RegisterSet result;
-    result.merge(stackRegisters());
-    result.set(GPRInfo::callFrameRegister);
+#if CPU(ARM64)
+    result.set(ARM64Registers::lr);
+#endif
+    return result;
+}
+
+RegisterSet RegisterSet::runtimeRegisters()
+{
+    RegisterSet result;
 #if USE(JSVALUE64)
     result.set(GPRInfo::tagTypeNumberRegister);
     result.set(GPRInfo::tagMaskRegister);
 #endif
-#if CPU(ARM64)
-    result.set(ARM64Registers::lr);
-#endif
     return result;
 }
 
+RegisterSet RegisterSet::specialRegisters()
+{
+    RegisterSet result;
+    result.merge(stackRegisters());
+    result.merge(reservedHardwareRegisters());
+    result.merge(runtimeRegisters());
+    return result;
+}
+
 RegisterSet RegisterSet::calleeSaveRegisters()
 {
     RegisterSet result;
@@ -69,7 +82,9 @@
     result.set(X86Registers::r15);
 #elif CPU(ARM64)
     // We don't include LR in the set of callee-save registers even though it technically belongs
-    // there. But, the way we use this list, it makes no sense to have it there.
+    // there. This is because we use this set to describe the set of registers that need to be saved
+    // beyond what you would save by the platform-agnostic "preserve return address" and "restore
+    // return address" operations in CCallHelpers.
     for (
         ARM64Registers::RegisterID reg = ARM64Registers::x19;
         reg <= ARM64Registers::x28;

Modified: trunk/Source/_javascript_Core/jit/RegisterSet.h (165292 => 165293)


--- trunk/Source/_javascript_Core/jit/RegisterSet.h	2014-03-07 22:16:00 UTC (rev 165292)
+++ trunk/Source/_javascript_Core/jit/RegisterSet.h	2014-03-07 22:22:46 UTC (rev 165293)
@@ -42,7 +42,9 @@
     RegisterSet() { }
     
     static RegisterSet stackRegisters();
-    static RegisterSet specialRegisters();
+    static RegisterSet reservedHardwareRegisters();
+    static RegisterSet runtimeRegisters();
+    static RegisterSet specialRegisters(); // The union of stack, reserved hardware, and runtime registers.
     static RegisterSet calleeSaveRegisters();
     static RegisterSet allGPRs();
     static RegisterSet allFPRs();
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to