Skip to content

Commit

Permalink
Cherry-pick 265870.440@safari-7616-branch (965be68). https://bugs.web…
Browse files Browse the repository at this point in the history
…kit.org/show_bug.cgi?id=260678

    [JSC] DFG AI GetById adhoc folding should insert watchpoints for structures
    https://bugs.webkit.org/show_bug.cgi?id=260678
    rdar://114072069

    Reviewed by Keith Miller.

    For DFG AI GetById's variants, they are tuples of StructureSet and offset.
    So, we should not obtain constant property just with offset since we first need to
    ensure that the base object is having a structure in StructureSet.
    Let's say [S0, 0] [S1, 1] variants are produced. In that case, we should not load
    a value from offset 1 when object is S0. But previously we were doing that since
    only thing we checked is that base is S0 or S1.
    This patch just extends DFG AI GetById handling to use existing tryGetConstantProperty
    mechanism with StructureSet. This properly inserts replacement watchpoints too, so that
    we can guarantee that the loaded value is inferred constant (if it gets different, then
    watchpoint fires). And we correctly check that the current object's structure is meeting
    the requirement against *variant*'s structure set.

    * JSTests/stress/same-offset-different-property-name-multiple-get-by-variants.js: Added.
    (main.const.object1):
    (main.const.object2):
    (main.const.object3):
    (main.get opt):
    (main):
    * Source/JavaScriptCore/dfg/DFGAbstractInterpreterInlines.h:
    (JSC::DFG::AbstractInterpreter<AbstractStateType>::executeEffects):
    * Source/JavaScriptCore/dfg/DFGGraph.cpp:
    (JSC::DFG::Graph::inferredValueForProperty):
    * Source/JavaScriptCore/dfg/DFGGraph.h:

    Canonical link: https://commits.webkit.org/265870.440@safari-7616-branch
  • Loading branch information
Constellation authored and aperezdc committed Oct 19, 2023
1 parent 9ef1b0b commit bf680fe
Show file tree
Hide file tree
Showing 4 changed files with 80 additions and 6 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,65 @@
function opt(wrapper, object, call, get) {
// CheckStructure
object.prototype.p1;
object.prototype.p2;

if (call) {
// CheckIsConstant
wrapper instanceof object;

if (get) {
// GetById
return object.prototype.p1.value;
}
}
}

function main() {
const object1 = function () {};
const object2 = function () {};
const object3 = function () {};
const object4 = {value: 1};

const wrapper1 = new object1();
const wrapper2 = new object2();

// S1
object1.prototype.p1 = object4;
object1.prototype.p2 = object4;

// S1 -> S2
object2.prototype.p1 = 1;
object2.prototype.p2 = 1;

delete object2.prototype.p2;
delete object2.prototype.p1;

object2.prototype.p2 = 2;
object2.prototype.p1 = 2;

Reflect.defineProperty(object3.prototype, 'p1', {
get() {
return object4;
}
});

// Just to force the compiler to emit the GetById node. Otherwise, it'll optimize it into a GetByOffset node.
opt(wrapper1, object3, /* call */ true, /* get */ true);

for (let i = 0; i < 1000000; i++) {
opt(wrapper1, object1, /* call */ true, /* get */ false);
opt(wrapper1, object2, /* call */ false, /* get */ false);
}

// S1 -> S2
delete object1.prototype.p2;
delete object1.prototype.p1;

object1.prototype.p2 = 1;
object1.prototype.p1 = 0x1234;

String(opt(wrapper1, object1, /* call */ true, /* get */ true));
}

main();

6 changes: 2 additions & 4 deletions Source/JavaScriptCore/dfg/DFGAbstractInterpreterInlines.h
Original file line number Diff line number Diff line change
Expand Up @@ -3800,10 +3800,8 @@ bool AbstractInterpreter<AbstractStateType>::executeEffects(unsigned clobberLimi
// This thing won't give us a variant that involves prototypes. If it did, we'd
// have more work to do here.
DFG_ASSERT(m_graph, node, status[i].conditionSet().isEmpty());

result.merge(
m_graph.inferredValueForProperty(
value, status[i].offset(), m_state.structureClobberState()));
const auto& variant = status[i];
result.merge(m_graph.inferredValueForProperty(value, *m_graph.addStructureSet(variant.structureSet()), variant.offset(), m_state.structureClobberState()));
}

m_state.setShouldTryConstantFolding(true);
Expand Down
11 changes: 11 additions & 0 deletions Source/JavaScriptCore/dfg/DFGGraph.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1385,6 +1385,17 @@ AbstractValue Graph::inferredValueForProperty(
return AbstractValue::heapTop();
}

AbstractValue Graph::inferredValueForProperty(const AbstractValue& base, const RegisteredStructureSet& structureSet, PropertyOffset offset, StructureClobberState clobberState)
{
if (JSValue value = tryGetConstantProperty(base.m_value, structureSet, offset)) {
AbstractValue result;
result.set(*this, *freeze(value), clobberState);
return result;
}

return AbstractValue::heapTop();
}

JSValue Graph::tryGetConstantClosureVar(JSValue base, ScopeOffset offset)
{
// This has an awesome concurrency story. See comment for GetGlobalVar in ByteCodeParser.
Expand Down
4 changes: 2 additions & 2 deletions Source/JavaScriptCore/dfg/DFGGraph.h
Original file line number Diff line number Diff line change
Expand Up @@ -923,8 +923,8 @@ class Graph final : public virtual Scannable {

// This uses either constant property inference or property type inference to derive a good abstract
// value for some property accessed with the given abstract value base.
AbstractValue inferredValueForProperty(
const AbstractValue& base, PropertyOffset, StructureClobberState);
AbstractValue inferredValueForProperty(const AbstractValue& base, PropertyOffset, StructureClobberState);
AbstractValue inferredValueForProperty(const AbstractValue& base, const RegisteredStructureSet&, PropertyOffset, StructureClobberState);

FullBytecodeLiveness& livenessFor(CodeBlock*);
FullBytecodeLiveness& livenessFor(InlineCallFrame*);
Expand Down

0 comments on commit bf680fe

Please sign in to comment.