| Summary: | WebAssembly: Fix f32.min, f64.min and f64.max operations on NaN | ||||||||
|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Tadeu Zagallo <tzagallo> | ||||||
| Component: | JavaScriptCore | Assignee: | Tadeu Zagallo <tzagallo> | ||||||
| Status: | RESOLVED FIXED | ||||||||
| Severity: | Normal | CC: | commit-queue, ews-watchlist, keith_miller, mark.lam, msaboff, rmorisset, saam, webkit-bug-importer, ysuzuki | ||||||
| Priority: | P2 | Keywords: | InRadar | ||||||
| Version: | WebKit Nightly Build | ||||||||
| Hardware: | Unspecified | ||||||||
| OS: | Unspecified | ||||||||
| See Also: | https://bugs.webkit.org/show_bug.cgi?id=175691 | ||||||||
| Attachments: |
|
||||||||
|
Description
Tadeu Zagallo
2019-03-25 14:47:49 PDT
Created attachment 365907 [details]
Patch
This patch modifies one of the wasm.json files. Please ensure that any changes in one have been mirrored to the other. You can find the wasm.json files at "Source/JavaScriptCore/wasm/wasm.json" and "JSTests/wasm/wasm.json". Comment on attachment 365907 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=365907&action=review r=me with a few minor nits. > Source/JavaScriptCore/ChangeLog:8 > + Generalize the fix for f32.max from r243446 to all min and max float operations. nit: Also worth one sentence on what r243446 fixed. > Source/JavaScriptCore/wasm/WasmAirIRGenerator.cpp:572 > + enum MinOrMax { Min, Max }; nit: let's use `enum class`. > Source/JavaScriptCore/wasm/WasmAirIRGenerator.cpp:2792 > + auto isEqualOp = minOrMax == Max ? andOp : orOp; nit: this variable name is kind of a weird, since we're in the "isEqual" block, but this op isn't about equality. I'd vote for just inlining this in the append. > Source/JavaScriptCore/wasm/WasmAirIRGenerator.cpp:2798 > + auto moveOp = floatType == F32 ? MoveFloat : MoveDouble; nit: I'd just inline this below into the append. Also, this can just be: append(isLessThan, moveOpForValueType(floatType), ...) Created attachment 365980 [details]
Patch for landing
Comment on attachment 365980 [details] Patch for landing Clearing flags on attachment: 365980 Committed r243514: <https://trac.webkit.org/changeset/243514> All reviewed patches have been landed. Closing bug. |