Title: [219633] trunk/Source/_javascript_Core
Revision
219633
Author
sbar...@apple.com
Date
2017-07-18 15:07:43 -0700 (Tue, 18 Jul 2017)

Log Message

AirLowerAfterRegAlloc may incorrectly use a callee save that's live as a scratch register
https://bugs.webkit.org/show_bug.cgi?id=174515
<rdar://problem/33358092>

Reviewed by Filip Pizlo.

AirLowerAfterRegAlloc was computing the set of available scratch
registers incorrectly. It was always excluding callee save registers
from the set of live registers. It did not guarantee that live callee save
registers were not in the set of scratch registers that could
get clobbered. That's incorrect as the shuffling code is free
to overwrite whatever is in the scratch register it gets passed.

* b3/air/AirLowerAfterRegAlloc.cpp:
(JSC::B3::Air::lowerAfterRegAlloc):
* b3/testb3.cpp:
(JSC::B3::functionNineArgs):
(JSC::B3::testShuffleDoesntTrashCalleeSaves):
(JSC::B3::run):
* jit/RegisterSet.h:

Modified Paths

Diff

Modified: trunk/Source/_javascript_Core/ChangeLog (219632 => 219633)


--- trunk/Source/_javascript_Core/ChangeLog	2017-07-18 21:45:29 UTC (rev 219632)
+++ trunk/Source/_javascript_Core/ChangeLog	2017-07-18 22:07:43 UTC (rev 219633)
@@ -1,3 +1,26 @@
+2017-07-18  Saam Barati  <sbar...@apple.com>
+
+        AirLowerAfterRegAlloc may incorrectly use a callee save that's live as a scratch register
+        https://bugs.webkit.org/show_bug.cgi?id=174515
+        <rdar://problem/33358092>
+
+        Reviewed by Filip Pizlo.
+
+        AirLowerAfterRegAlloc was computing the set of available scratch
+        registers incorrectly. It was always excluding callee save registers
+        from the set of live registers. It did not guarantee that live callee save
+        registers were not in the set of scratch registers that could
+        get clobbered. That's incorrect as the shuffling code is free
+        to overwrite whatever is in the scratch register it gets passed.
+
+        * b3/air/AirLowerAfterRegAlloc.cpp:
+        (JSC::B3::Air::lowerAfterRegAlloc):
+        * b3/testb3.cpp:
+        (JSC::B3::functionNineArgs):
+        (JSC::B3::testShuffleDoesntTrashCalleeSaves):
+        (JSC::B3::run):
+        * jit/RegisterSet.h:
+
 2017-07-18  Andy Estes  <aes...@apple.com>
 
         [Xcode] Enable CLANG_WARN_NON_LITERAL_NULL_CONVERSION

Modified: trunk/Source/_javascript_Core/b3/air/AirLowerAfterRegAlloc.cpp (219632 => 219633)


--- trunk/Source/_javascript_Core/b3/air/AirLowerAfterRegAlloc.cpp	2017-07-18 21:45:29 UTC (rev 219632)
+++ trunk/Source/_javascript_Core/b3/air/AirLowerAfterRegAlloc.cpp	2017-07-18 22:07:43 UTC (rev 219633)
@@ -181,7 +181,7 @@
                 regsToSave.exclude(RegisterSet::stackRegisters());
                 regsToSave.exclude(RegisterSet::reservedHardwareRegisters());
 
-                RegisterSet preUsed = regsToSave;
+                RegisterSet preUsed = liveRegs;
                 Vector<Arg> destinations = computeCCallingConvention(code, value);
                 Tmp result = cCallResult(value->type());
                 Arg originalResult = result ? inst.args[1] : Arg();

Modified: trunk/Source/_javascript_Core/b3/testb3.cpp (219632 => 219633)


--- trunk/Source/_javascript_Core/b3/testb3.cpp	2017-07-18 21:45:29 UTC (rev 219632)
+++ trunk/Source/_javascript_Core/b3/testb3.cpp	2017-07-18 22:07:43 UTC (rev 219633)
@@ -15545,6 +15545,100 @@
     }
 }
 
+void functionNineArgs(int32_t, void*, void*, void*, void*, void*, void*, void*, void*) { }
+
+void testShuffleDoesntTrashCalleeSaves()
+{
+    Procedure proc;
+
+    BasicBlock* root = proc.addBlock();
+    BasicBlock* likely = proc.addBlock();
+    BasicBlock* unlikely = proc.addBlock();
+
+    RegisterSet regs = RegisterSet::allGPRs();
+    regs.exclude(RegisterSet::stackRegisters());
+    regs.exclude(RegisterSet::reservedHardwareRegisters());
+    regs.exclude(RegisterSet::calleeSaveRegisters());
+    regs.exclude(RegisterSet::argumentGPRS());
+
+    unsigned i = 0;
+    Vector<Value*> patches;
+    for (Reg reg : regs) {
+        ++i;
+        PatchpointValue* patchpoint = root->appendNew<PatchpointValue>(proc, Int32, Origin());
+        patchpoint->clobber(RegisterSet::macroScratchRegisters());
+        RELEASE_ASSERT(reg.isGPR());
+        patchpoint->resultConstraint = ValueRep::reg(reg.gpr());
+        patchpoint->setGenerator(
+            [=] (CCallHelpers& jit, const StackmapGenerationParams& params) {
+                AllowMacroScratchRegisterUsage allowScratch(jit);
+                jit.move(CCallHelpers::TrustedImm32(i), params[0].gpr());
+            });
+        patches.append(patchpoint);
+    }
+
+    Value* arg1 = root->appendNew<ArgumentRegValue>(proc, Origin(), GPRInfo::toArgumentRegister(0 % GPRInfo::numberOfArgumentRegisters));
+    Value* arg2 = root->appendNew<ArgumentRegValue>(proc, Origin(), GPRInfo::toArgumentRegister(1 % GPRInfo::numberOfArgumentRegisters));
+    Value* arg3 = root->appendNew<ArgumentRegValue>(proc, Origin(), GPRInfo::toArgumentRegister(2 % GPRInfo::numberOfArgumentRegisters));
+    Value* arg4 = root->appendNew<ArgumentRegValue>(proc, Origin(), GPRInfo::toArgumentRegister(3 % GPRInfo::numberOfArgumentRegisters));
+    Value* arg5 = root->appendNew<ArgumentRegValue>(proc, Origin(), GPRInfo::toArgumentRegister(4 % GPRInfo::numberOfArgumentRegisters));
+    Value* arg6 = root->appendNew<ArgumentRegValue>(proc, Origin(), GPRInfo::toArgumentRegister(5 % GPRInfo::numberOfArgumentRegisters));
+    Value* arg7 = root->appendNew<ArgumentRegValue>(proc, Origin(), GPRInfo::toArgumentRegister(6 % GPRInfo::numberOfArgumentRegisters));
+    Value* arg8 = root->appendNew<ArgumentRegValue>(proc, Origin(), GPRInfo::toArgumentRegister(7 % GPRInfo::numberOfArgumentRegisters));
+
+    PatchpointValue* ptr = root->appendNew<PatchpointValue>(proc, Int64, Origin());
+    ptr->clobber(RegisterSet::macroScratchRegisters());
+    ptr->resultConstraint = ValueRep::reg(GPRInfo::regCS0);
+    ptr->appendSomeRegister(arg1);
+    ptr->setGenerator(
+        [=] (CCallHelpers& jit, const StackmapGenerationParams& params) {
+            AllowMacroScratchRegisterUsage allowScratch(jit);
+            jit.move(params[1].gpr(), params[0].gpr());
+        });
+
+    Value* condition = root->appendNew<Value>(
+        proc, Equal, Origin(), 
+        ptr,
+        root->appendNew<Const64Value>(proc, Origin(), 0));
+
+    root->appendNewControlValue(
+        proc, Branch, Origin(),
+        condition,
+        FrequentedBlock(likely, FrequencyClass::Normal), FrequentedBlock(unlikely, FrequencyClass::Rare));
+
+    // Never executes.
+    Value* const42 = likely->appendNew<Const32Value>(proc, Origin(), 42);
+    likely->appendNewControlValue(proc, Return, Origin(), const42);
+
+    // Always executes.
+    Value* constNumber = unlikely->appendNew<Const32Value>(proc, Origin(), 0x1);
+
+    unlikely->appendNew<CCallValue>(
+        proc, Void, Origin(),
+        unlikely->appendNew<ConstPtrValue>(proc, Origin(), bitwise_cast<void*>(functionNineArgs)),
+        constNumber, arg1, arg2, arg3, arg4, arg5, arg6, arg7, arg8);
+
+    PatchpointValue* voidPatch = unlikely->appendNew<PatchpointValue>(proc, Void, Origin());
+    voidPatch->clobber(RegisterSet::macroScratchRegisters());
+    for (Value* v : patches)
+        voidPatch->appendSomeRegister(v);
+    voidPatch->appendSomeRegister(arg1);
+    voidPatch->appendSomeRegister(arg2);
+    voidPatch->appendSomeRegister(arg3);
+    voidPatch->appendSomeRegister(arg4);
+    voidPatch->appendSomeRegister(arg5);
+    voidPatch->appendSomeRegister(arg6);
+    voidPatch->setGenerator([=] (CCallHelpers&, const StackmapGenerationParams&) { });
+
+    unlikely->appendNewControlValue(proc, Return, Origin(),
+        unlikely->appendNew<MemoryValue>(proc, Load, Int32, Origin(), ptr));
+
+    int32_t* inputPtr = static_cast<int32_t*>(fastMalloc(sizeof(int32_t)));
+    *inputPtr = 48;
+    CHECK(compileAndRun<int32_t>(proc, inputPtr) == 48);
+    fastFree(inputPtr);
+}
+
 // Make sure the compiler does not try to optimize anything out.
 NEVER_INLINE double zero()
 {
@@ -17088,6 +17182,8 @@
     RUN(testFloatEqualOrUnorderedFoldingNaN());
     RUN(testFloatEqualOrUnorderedDontFold());
 
+    RUN(testShuffleDoesntTrashCalleeSaves());
+
     if (isX86()) {
         RUN(testBranchBitAndImmFusion(Identity, Int64, 1, Air::BranchTest32, Air::Arg::Tmp));
         RUN(testBranchBitAndImmFusion(Identity, Int64, 0xff, Air::BranchTest32, Air::Arg::Tmp));

Modified: trunk/Source/_javascript_Core/jit/RegisterSet.h (219632 => 219633)


--- trunk/Source/_javascript_Core/jit/RegisterSet.h	2017-07-18 21:45:29 UTC (rev 219632)
+++ trunk/Source/_javascript_Core/jit/RegisterSet.h	2017-07-18 22:07:43 UTC (rev 219633)
@@ -63,7 +63,7 @@
     JS_EXPORT_PRIVATE static RegisterSet allGPRs();
     JS_EXPORT_PRIVATE static RegisterSet allFPRs();
     static RegisterSet allRegisters();
-    static RegisterSet argumentGPRS();
+    JS_EXPORT_PRIVATE static RegisterSet argumentGPRS();
 
     static RegisterSet registersToNotSaveForJSCall();
     static RegisterSet registersToNotSaveForCCall();
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to