[Webkit-unassigned] [Bug 235581] Add support for WASM branch hinting proposal

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=235581

Yusuke Suzuki <ysuzuki at apple.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #450087|review?                     |review+
              Flags|                            |

--- Comment #8 from Yusuke Suzuki <ysuzuki at apple.com> ---
Comment on attachment 450087
  --> https://bugs.webkit.org/attachment.cgi?id=450087
Updated patch after review

View in context: https://bugs.webkit.org/attachment.cgi?id=450087&action=review

r=me

> Source/JavaScriptCore/wasm/WasmAirIRGenerator.cpp:3186
> +        if (hint == BranchHint::Unlikely)
> +            takenFrequency = B3::FrequencyClass::Rare;
> +        else if (hint == BranchHint::Likely)
> +            notTakenFrequency = B3::FrequencyClass::Rare;

Let's use switch.

> Source/JavaScriptCore/wasm/WasmAirIRGenerator.cpp:3439
> +        if (hint == BranchHint::Unlikely)
> +            targetFrequency = B3::FrequencyClass::Rare;
> +        else if (hint == BranchHint::Likely)
> +            continuationFrequency = B3::FrequencyClass::Rare;

Ditto.

> Source/JavaScriptCore/wasm/WasmB3IRGenerator.cpp:2486
> +        if (hint == BranchHint::Unlikely)
> +            takenFrequency = FrequencyClass::Rare;
> +        else if (hint == BranchHint::Likely)
> +            notTakenFrequency = FrequencyClass::Rare;

Let's use switch, which can ensure that all enums are handled.

switch (hint) {
case BranchHint::Unlikely:
    takenFrequency = FrequencyClass::Rare;
    break;
case BranchHint::Likely:
    notTakenFrequency = FrequencyClass::Rare;
    break;
}

> Source/JavaScriptCore/wasm/WasmB3IRGenerator.cpp:2742
> +        if (hint == BranchHint::Unlikely)
> +            targetFrequency = FrequencyClass::Rare;
> +        else if (hint == BranchHint::Likely)
> +            continuationFrequency = FrequencyClass::Rare;

Ditto.

> Source/JavaScriptCore/wasm/WasmBranchHintsSectionParser.cpp:70
> +            BranchHint branchHint = static_cast<BranchHint>(parsedBranchHint);

Let's add

ASSERT(isValidBranchHint(branchHint));

It should be due to parseVarUInt1. But such an ASSERT is helpful if this branch hint is extended to have more values (then, assert hits and we can fix the issue :)).

-- 
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.webkit.org/pipermail/webkit-unassigned/attachments/20220128/80256b18/attachment-0001.htm>