Open Bug 1435220 Opened 2 years ago Updated 4 months ago

Clean up wasm out-of-line truncation check MASM interfaces

Categories

(Core :: Javascript: WebAssembly, enhancement, P3)

enhancement

Tracking

()

ASSIGNED

People

(Reporter: lth, Assigned: lth)

Details

(Keywords: leave-open)

Attachments

(1 file)

These APIs are defined on x86_shared only but there's no reason for that.  They can easily be lifted to ARM with minor changes, and we'll need ARM64 support before long.  I know MIPS ditto are coming in with the Rabaldr support.
Depends on: 1434843
Blocks: 1434843
No longer depends on: 1434843
No longer blocks: 1434843
Blocks: 1434843
Mostly straightforward cleanup + a little reworking in Rabaldr, see commit msg.
Attachment #8947771 - Flags: review?(bbouvier)
Comment on attachment 8947771 [details] [diff] [review]
bug1435220-truncation-check-api.patch

Review of attachment 8947771 [details] [diff] [review]:
-----------------------------------------------------------------

Nice, thanks!

::: js/src/jit/arm/MacroAssembler-arm.cpp
@@ +4919,4 @@
>  }
>  
>  void
> +MacroAssembler::oolWasmTruncateCheckF32ToI32(FloatRegister input, bool isUnsigned,

Nice! Looking up to the users, do you think we could hoist visitOutOfLineWasmTruncateCheck from the platform specific code up to CodeGenerator-shared code?
Attachment #8947771 - Flags: review?(bbouvier) → review+
(In reply to Benjamin Bouvier [:bbouvier] from comment #2)

> Nice! Looking up to the users, do you think we could hoist
> visitOutOfLineWasmTruncateCheck from the platform specific code up to
> CodeGenerator-shared code?

Oh, that would be sweet.  I'll look into that.
Pushed by lhansen@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/e16f100ae421
Clean up MASM APIs for wasm truncation checks. r=bbouvier
Leave open to investigate comment 3.
Keywords: leave-open
Dan, can you look into comment 3 in the context of bug 1435369, I feel there's a lot of overlap there possibly.
Flags: needinfo?(sunfish)
Yes, my timing was unfortunate. I'll resolve all the merge conflicts and post an update.
Flags: needinfo?(sunfish)
No longer blocks: Rabaldr-ARM64
Priority: -- → P3
No longer blocks: 1434843
Component: JavaScript Engine: JIT → Javascript: Web Assembly

The leave-open keyword is there and there is no activity for 6 months.
:lth, maybe it's time to close this bug?

Flags: needinfo?(lhansen)

(In reply to Release mgmt bot [:sylvestre / :calixte] from comment #10)

The leave-open keyword is there and there is no activity for 6 months.
:lth, maybe it's time to close this bug?

No.

Flags: needinfo?(lhansen)

The leave-open keyword is there and there is no activity for 6 months.
:lth, maybe it's time to close this bug?

Flags: needinfo?(lhansen)
Flags: needinfo?(lhansen)
You need to log in before you can comment on or make changes to this bug.