Changeset 240959 in webkit


Ignore:
Timestamp:
Feb 4, 2019 5:36:28 PM (5 years ago)
Author:
rmorisset@apple.com
Message:

when lowering AssertNotEmpty, create the value before creating the patchpoint
https://bugs.webkit.org/show_bug.cgi?id=194231

Reviewed by Saam Barati.

JSTests:

This test is painfully fragile: it tries to test that AssertNotEmpty on a constant produces valid B3 IR.
The problem is that AssertNotEmpty is only created by DFGConstantFolding when it can simplify a CheckStructure, and constant folding is a bit capricious (https://bugs.webkit.org/show_bug.cgi?id=133947)
So even tiny changes to this test can change the path code taken.

  • stress/assert-not-empty.js: Added.

(foo):

Source/JavaScriptCore:

This is a very simple change: we should never generate B3 IR where an instruction depends on a value that comes later in the instruction stream.
AssertNotEmpty was generating some such IR, it probably slipped through until now because it is a rather rare and tricky instruction to generate.

  • ftl/FTLLowerDFGToB3.cpp:

(JSC::FTL::DFG::LowerDFGToB3::compileAssertNotEmpty):

Location:
trunk
Files:
1 added
3 edited

Legend:

Unmodified
Added
Removed
  • trunk/JSTests/ChangeLog

    r240878 r240959  
     12019-02-04  Robin Morisset  <rmorisset@apple.com>
     2
     3        when lowering AssertNotEmpty, create the value before creating the patchpoint
     4        https://bugs.webkit.org/show_bug.cgi?id=194231
     5
     6        Reviewed by Saam Barati.
     7
     8        This test is painfully fragile: it tries to test that AssertNotEmpty on a constant produces valid B3 IR.
     9        The problem is that AssertNotEmpty is only created by DFGConstantFolding when it can simplify a CheckStructure, and constant folding is a bit capricious (https://bugs.webkit.org/show_bug.cgi?id=133947)
     10        So even tiny changes to this test can change the path code taken.
     11
     12        * stress/assert-not-empty.js: Added.
     13        (foo):
     14
    1152019-02-01  Mark Lam  <mark.lam@apple.com>
    216
  • trunk/Source/JavaScriptCore/ChangeLog

    r240951 r240959  
     12019-02-04  Robin Morisset  <rmorisset@apple.com>
     2
     3        when lowering AssertNotEmpty, create the value before creating the patchpoint
     4        https://bugs.webkit.org/show_bug.cgi?id=194231
     5
     6        Reviewed by Saam Barati.
     7
     8        This is a very simple change: we should never generate B3 IR where an instruction depends on a value that comes later in the instruction stream.
     9        AssertNotEmpty was generating some such IR, it probably slipped through until now because it is a rather rare and tricky instruction to generate.
     10
     11        * ftl/FTLLowerDFGToB3.cpp:
     12        (JSC::FTL::DFG::LowerDFGToB3::compileAssertNotEmpty):
     13
    1142019-02-04  Yusuke Suzuki  <ysuzuki@apple.com>
    215
  • trunk/Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp

    r240679 r240959  
    31123112            return;
    31133113
     3114        LValue val = lowJSValue(m_node->child1());
    31143115        PatchpointValue* patchpoint = m_out.patchpoint(Void);
    3115         patchpoint->appendSomeRegister(lowJSValue(m_node->child1()));
     3116        patchpoint->appendSomeRegister(val);
    31163117        patchpoint->setGenerator(
    31173118            [=] (CCallHelpers& jit, const StackmapGenerationParams& params) {
Note: See TracChangeset for help on using the changeset viewer.