Bug 254710 - WasmBBQJIT performs redundant overflow check when dividend is constant
Summary: WasmBBQJIT performs redundant overflow check when dividend is constant
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebAssembly (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: David Degazio
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2023-03-29 17:22 PDT by David Degazio
Modified: 2023-03-30 09:53 PDT (History)
1 user (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description David Degazio 2023-03-29 17:22:59 PDT
rdar://106823148

In WASM, when generating an integer division, it's necessary to check for the case INT_MIN / -1, and throw an integer overflow exception. In WasmBBQJIT on ARM64, we try to avoid doing redundant work, so when a division is given a constant operand, we use it to rule out impossible errors (e.g. if we are dividing by a nonzero constant, we don't need to check for division by zero). When we detect that the left operand is a constant, we use the following code to check for the aforementioned case:

    if (isSigned && !IsMod && dividend == std::numeric_limits<IntType>::min()) {
        Jump isNegativeOne = is32
            ? m_jit.branch32(RelationalCondition::Equal, rhsLocation.asGPR(), TrustedImm32(-1))
            : m_jit.branch64(RelationalCondition::Equal, rhsLocation.asGPR(), TrustedImm64(-1));
        throwExceptionIf(ExceptionType::IntegerOverflow, isNegativeOne);
        checkedForNegativeOne = true;
    }

By setting checkedForNegativeOne = true, when we fall through to the general case, the hope is that we don't emit another more general for negative one, since we just checked for it in the specialized case. However, in the case where the constant dividend is *not* INT_MIN, we should *also* consider the check complete - if we know statically that the left operand isn't INT_MIN, it's impossible for the division to be computing INT_MIN / -1. So we should always be setting checkedForNegativeOne = true when the dividend is a constant in WasmBBQJIT.
Comment 1 David Degazio 2023-03-29 17:49:47 PDT
Pull request: https://github.com/WebKit/WebKit/pull/12150
Comment 2 EWS 2023-03-30 09:53:54 PDT
Committed 262335@main (700f6525d4cf): <https://commits.webkit.org/262335@main>

Reviewed commits have been landed. Closing PR #12150 and removing active labels.