Changeset 215531 in webkit


Ignore:
Timestamp:
Apr 19, 2017 2:51:52 PM (7 years ago)
Author:
mark.lam@apple.com
Message:

B3StackmapSpecial should handle when stackmap values are not recoverable from a Def'ed arg.
https://bugs.webkit.org/show_bug.cgi?id=170973
<rdar://problem/30318657>

Reviewed by Filip Pizlo.

JSTests:

  • stress/regress-170973.js: Added.

Source/JavaScriptCore:

In the event of an arithmetic overflow on a binary sub instruction (where the
result register is same as one of the operand registers), the CheckSub FTL
operation will try to recover the original value in the clobbered result register.

This recover is done by adding the other operand value to the result register.
However, this recovery method only works if the width of the original value in
the result register is less or equal to the width of the expected result. If the
width of the original operand value (e.g. a JSInt32) is wider than the result
(e.g. a machine Int32), then the sub operation would have zero extended the
result and cleared the upper 32-bits of the result register. Recovery by adding
back the other operand will not restore the JSValue tag in the upper word.

This poses a problem if the stackmap value for the operand relies on that same
clobbered register.

The fix is to detect this potential scenario (i.e. width of the Def's arg < width
of a stackmap value). If this condition is detected, we'll declare the stackmap
value to be LateColdUse to ensure that the register allocator gives it a
different register if needed so that it's not dependent on the clobbered register.

  • b3/B3CheckSpecial.cpp:

(JSC::B3::CheckSpecial::forEachArg):

  • b3/B3PatchpointSpecial.cpp:

(JSC::B3::PatchpointSpecial::forEachArg):

  • b3/B3StackmapSpecial.cpp:

(JSC::B3::StackmapSpecial::forEachArgImpl):

  • b3/B3StackmapSpecial.h:
Location:
trunk
Files:
1 added
6 edited

Legend:

Unmodified
Added
Removed
  • trunk/JSTests/ChangeLog

    r215525 r215531  
     12017-04-19  Mark Lam  <mark.lam@apple.com>
     2
     3        B3StackmapSpecial should handle when stackmap values are not recoverable from a Def'ed arg.
     4        https://bugs.webkit.org/show_bug.cgi?id=170973
     5        <rdar://problem/30318657>
     6
     7        Reviewed by Filip Pizlo.
     8
     9        * stress/regress-170973.js: Added.
     10
    1112017-04-19  JF Bastien  <jfbastien@apple.com>
    212
  • trunk/Source/JavaScriptCore/ChangeLog

    r215526 r215531  
     12017-04-19  Mark Lam  <mark.lam@apple.com>
     2
     3        B3StackmapSpecial should handle when stackmap values are not recoverable from a Def'ed arg.
     4        https://bugs.webkit.org/show_bug.cgi?id=170973
     5        <rdar://problem/30318657>
     6
     7        Reviewed by Filip Pizlo.
     8
     9        In the event of an arithmetic overflow on a binary sub instruction (where the
     10        result register is same as one of the operand registers), the CheckSub FTL
     11        operation will try to recover the original value in the clobbered result register.
     12
     13        This recover is done by adding the other operand value to the result register.
     14        However, this recovery method only works if the width of the original value in
     15        the result register is less or equal to the width of the expected result.  If the
     16        width of the original operand value (e.g. a JSInt32) is wider than the result
     17        (e.g. a machine Int32), then the sub operation would have zero extended the
     18        result and cleared the upper 32-bits of the result register.  Recovery by adding
     19        back the other operand will not restore the JSValue tag in the upper word.
     20
     21        This poses a problem if the stackmap value for the operand relies on that same
     22        clobbered register.
     23
     24        The fix is to detect this potential scenario (i.e. width of the Def's arg < width
     25        of a stackmap value).  If this condition is detected, we'll declare the stackmap
     26        value to be LateColdUse to ensure that the register allocator gives it a
     27        different register if needed so that it's not dependent on the clobbered register.
     28
     29        * b3/B3CheckSpecial.cpp:
     30        (JSC::B3::CheckSpecial::forEachArg):
     31        * b3/B3PatchpointSpecial.cpp:
     32        (JSC::B3::PatchpointSpecial::forEachArg):
     33        * b3/B3StackmapSpecial.cpp:
     34        (JSC::B3::StackmapSpecial::forEachArgImpl):
     35        * b3/B3StackmapSpecial.h:
     36
    1372017-04-19  JF Bastien  <jfbastien@apple.com>
    238
  • trunk/Source/JavaScriptCore/b3/B3CheckSpecial.cpp

    r212970 r215531  
    109109void CheckSpecial::forEachArg(Inst& inst, const ScopedLambda<Inst::EachArgCallback>& callback)
    110110{
     111    std::optional<Width> optionalDefArgWidth;
    111112    Inst hidden = hiddenBranch(inst);
    112113    hidden.forEachArg(
    113114        [&] (Arg& arg, Arg::Role role, Bank bank, Width width) {
     115            if (Arg::isAnyDef(role)) {
     116                ASSERT(!optionalDefArgWidth); // There can only be one Def'ed arg.
     117                optionalDefArgWidth = width;
     118            }
    114119            unsigned index = &arg - &hidden.args[0];
    115120            callback(inst.args[1 + index], role, bank, width);
     
    119124    if (m_checkKind.opcode == BranchAdd32 || m_checkKind.opcode == BranchAdd64)
    120125        firstRecoverableIndex = 1;
    121     forEachArgImpl(numB3Args(inst), m_numCheckArgs + 1, inst, m_stackmapRole, firstRecoverableIndex, callback);
     126    forEachArgImpl(numB3Args(inst), m_numCheckArgs + 1, inst, m_stackmapRole, firstRecoverableIndex, callback, optionalDefArgWidth);
    122127}
    123128
  • trunk/Source/JavaScriptCore/b3/B3PatchpointSpecial.cpp

    r212970 r215531  
    6060    }
    6161
    62     forEachArgImpl(0, argIndex, inst, SameAsRep, std::nullopt, callback);
     62    forEachArgImpl(0, argIndex, inst, SameAsRep, std::nullopt, callback, std::nullopt);
    6363    argIndex += inst.origin->numChildren();
    6464
  • trunk/Source/JavaScriptCore/b3/B3StackmapSpecial.cpp

    r215407 r215531  
    7575    unsigned numIgnoredB3Args, unsigned numIgnoredAirArgs,
    7676    Inst& inst, RoleMode roleMode, std::optional<unsigned> firstRecoverableIndex,
    77     const ScopedLambda<Inst::EachArgCallback>& callback)
     77    const ScopedLambda<Inst::EachArgCallback>& callback, std::optional<Width> optionalDefArgWidth)
    7878{
    7979    StackmapValue* value = inst.origin->as<StackmapValue>();
     
    8484    ASSERT(value->children().size() >= numIgnoredB3Args);
    8585    ASSERT(inst.args.size() - numIgnoredAirArgs >= value->children().size() - numIgnoredB3Args);
    86    
     86    ASSERT(inst.args[0].kind() == Arg::Kind::Special);
     87
    8788    for (unsigned i = 0; i < value->children().size() - numIgnoredB3Args; ++i) {
    8889        Arg& arg = inst.args[i + numIgnoredAirArgs];
     
    120121                RELEASE_ASSERT_NOT_REACHED();
    121122                break;
     123            }
     124
     125            // If the Def'ed arg has a smaller width than the a stackmap value, then we may not
     126            // be able to recover the stackmap value. So, force LateColdUse to preserve the
     127            // original stackmap value across the Special operation.
     128            if (!Arg::isLateUse(role) && optionalDefArgWidth && *optionalDefArgWidth < child.value()->resultWidth()) {
     129                if (Arg::isWarmUse(role))
     130                    role = Arg::LateUse;
     131                else
     132                    role = Arg::LateColdUse;
    122133            }
    123134            break;
  • trunk/Source/JavaScriptCore/b3/B3StackmapSpecial.h

    r208985 r215531  
    11/*
    2  * Copyright (C) 2015-2016 Apple Inc. All rights reserved.
     2 * Copyright (C) 2015-2017 Apple Inc. All rights reserved.
    33 *
    44 * Redistribution and use in source and binary forms, with or without
     
    6161        unsigned numIgnoredB3Args, unsigned numIgnoredAirArgs,
    6262        Air::Inst&, RoleMode, std::optional<unsigned> firstRecoverableIndex,
    63         const ScopedLambda<Air::Inst::EachArgCallback>&);
    64    
     63        const ScopedLambda<Air::Inst::EachArgCallback>&, std::optional<Width> optionalDefArgWidth);
     64
    6565    bool isValidImpl(
    6666        unsigned numIgnoredB3Args, unsigned numIgnoredAirArgs,
Note: See TracChangeset for help on using the changeset viewer.