Attachment 362240 Details for Bug 194038 – patch

[patch] patch

b-backup.diff (text/plain), 5.09 KB, created by Saam Barati on 2019-02-17 11:58:23 PST

(hide)

Description:

Filename:

MIME Type:

Creator: Saam Barati

Created: 2019-02-17 11:58:23 PST

Size: 5.09 KB

patch

obsolete

>Index: Source/JavaScriptCore/ChangeLog
>===================================================================
>--- Source/JavaScriptCore/ChangeLog	(revision 241651)
>+++ Source/JavaScriptCore/ChangeLog	(working copy)
>@@ -1,3 +1,23 @@
>+2019-02-17  Saam Barati  <sbarati@apple.com>
>+
>+        WasmB3IRGenerator models some effects incorrectly
>+        https://bugs.webkit.org/show_bug.cgi?id=194038
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        * wasm/WasmB3IRGenerator.cpp:
>+        (JSC::Wasm::B3IRGenerator::restoreWasmContextInstance):
>+        (JSC::Wasm::B3IRGenerator::restoreWebAssemblyGlobalState):
>+        These two functions were using global state instead of the
>+        arguments passed into the function.
>+
>+        (JSC::Wasm::B3IRGenerator::addOp<F64ConvertUI64>):
>+        (JSC::Wasm::B3IRGenerator::addOp<OpType::F32ConvertUI64>):
>+        (JSC::Wasm::B3IRGenerator::addOp<OpType::I64TruncUF64>):
>+        (JSC::Wasm::B3IRGenerator::addOp<OpType::I64TruncUF32>):
>+        Any patchpoint that allows scratch register usage must
>+        also say that it clobbers the scratch registers.
>+
> 2019-02-17  Commit Queue  <commit-queue@webkit.org>
> 
>         Unreviewed, rolling out r241612.
>Index: Source/JavaScriptCore/wasm/WasmB3IRGenerator.cpp
>===================================================================
>--- Source/JavaScriptCore/wasm/WasmB3IRGenerator.cpp	(revision 241650)
>+++ Source/JavaScriptCore/wasm/WasmB3IRGenerator.cpp	(working copy)
>@@ -317,7 +317,7 @@ void B3IRGenerator::restoreWasmContextIn
>     effects.reads = B3::HeapRange::top();
>     patchpoint->effects = effects;
>     patchpoint->clobberLate(RegisterSet(m_wasmContextInstanceGPR));
>-    patchpoint->append(instanceValue(), ValueRep::SomeRegister);
>+    patchpoint->append(arg, ValueRep::SomeRegister);
>     GPRReg wasmContextInstanceGPR = m_wasmContextInstanceGPR;
>     patchpoint->setGenerator([=] (CCallHelpers& jit, const StackmapGenerationParams& param) {
>         jit.move(param[0].gpr(), wasmContextInstanceGPR);
>@@ -461,9 +461,9 @@ void B3IRGenerator::restoreWebAssemblyGl
> 
>     if (restoreCachedStackLimit == RestoreCachedStackLimit::Yes) {
>         // The Instance caches the stack limit, but also knows where its canonical location is.
>-        Value* pointerToActualStackLimit = m_currentBlock->appendNew<MemoryValue>(m_proc, Load, pointerType(), origin(), instanceValue(), safeCast<int32_t>(Instance::offsetOfPointerToActualStackLimit()));
>-        Value* actualStackLimit = m_currentBlock->appendNew<MemoryValue>(m_proc, Load, pointerType(), origin(), pointerToActualStackLimit);
>-        m_currentBlock->appendNew<MemoryValue>(m_proc, Store, origin(), actualStackLimit, instanceValue(), safeCast<int32_t>(Instance::offsetOfCachedStackLimit()));
>+        Value* pointerToActualStackLimit = block->appendNew<MemoryValue>(m_proc, Load, pointerType(), origin(), instanceValue(), safeCast<int32_t>(Instance::offsetOfPointerToActualStackLimit()));
>+        Value* actualStackLimit = block->appendNew<MemoryValue>(m_proc, Load, pointerType(), origin(), pointerToActualStackLimit);
>+        block->appendNew<MemoryValue>(m_proc, Store, origin(), actualStackLimit, instanceValue(), safeCast<int32_t>(Instance::offsetOfCachedStackLimit()));
>     }
> 
>     if (!!memory) {
>@@ -1610,6 +1610,7 @@ auto B3IRGenerator::addOp<F64ConvertUI64
>     PatchpointValue* patchpoint = m_currentBlock->appendNew<PatchpointValue>(m_proc, Double, origin());
>     if (isX86())
>         patchpoint->numGPScratchRegisters = 1;
>+    patchpoint->clobber(RegisterSet::macroScratchRegisters());
>     patchpoint->append(ConstrainedValue(arg, ValueRep::SomeRegister));
>     patchpoint->setGenerator([=] (CCallHelpers& jit, const StackmapGenerationParams& params) {
>         AllowMacroScratchRegisterUsage allowScratch(jit);
>@@ -1630,6 +1631,7 @@ auto B3IRGenerator::addOp<OpType::F32Con
>     PatchpointValue* patchpoint = m_currentBlock->appendNew<PatchpointValue>(m_proc, Float, origin());
>     if (isX86())
>         patchpoint->numGPScratchRegisters = 1;
>+    patchpoint->clobber(RegisterSet::macroScratchRegisters());
>     patchpoint->append(ConstrainedValue(arg, ValueRep::SomeRegister));
>     patchpoint->setGenerator([=] (CCallHelpers& jit, const StackmapGenerationParams& params) {
>         AllowMacroScratchRegisterUsage allowScratch(jit);
>@@ -1839,6 +1841,7 @@ auto B3IRGenerator::addOp<OpType::I64Tru
>         patchpoint->append(signBitConstant, ValueRep::SomeRegister);
>         patchpoint->numFPScratchRegisters = 1;
>     }
>+    patchpoint->clobber(RegisterSet::macroScratchRegisters());
>     patchpoint->setGenerator([=] (CCallHelpers& jit, const StackmapGenerationParams& params) {
>         AllowMacroScratchRegisterUsage allowScratch(jit);
>         FPRReg scratch = InvalidFPRReg;
>@@ -1904,6 +1907,7 @@ auto B3IRGenerator::addOp<OpType::I64Tru
>         patchpoint->append(signBitConstant, ValueRep::SomeRegister);
>         patchpoint->numFPScratchRegisters = 1;
>     }
>+    patchpoint->clobber(RegisterSet::macroScratchRegisters());
>     patchpoint->setGenerator([=] (CCallHelpers& jit, const StackmapGenerationParams& params) {
>         AllowMacroScratchRegisterUsage allowScratch(jit);
>         FPRReg scratch = InvalidFPRReg;

Actions: View | Formatted Diff | Diff

Attachments on bug 194038: 360607 | 362240