Bug 170217 – WebAssembly: We should have Origins

• Component: New Bugs
• Status: RESOLVED FIXED

[Keith Miller]

WebAssembly: We should have Origins

[Keith Miller]

Created attachment 305681 [details]
Patch

[Keith Miller]

Here's a sample:

BB#0: ; frequency = 1.000000
    Int64 @4 = Patchpoint(generator = 0x10f487fa8, earlyClobbered = [], lateClobbered = [], usedRegisters = [], resultConstraint = SomeRegister)
    Int64 @5 = FramePointer()
    Void @8 = Store(@4, @5, offset = 24, ControlDependent|Writes:Top)
    Int64 @10 = Const64(0)
    Void @12 = Store($0(@10), @5, offset = 16, ControlDependent|Writes:Top)
    Int64 @13 = Patchpoint(generator = 0x10f4be7f0, earlyClobbered = [], lateClobbered = [], usedRegisters = [], resultConstraint = SomeRegister, ExitsSideways|ControlDependent|WritesPinned|ReadsPinned|Fence|Writes:Top|Reads:Top)
    Int64 @16 = ArgumentReg(%rdi)
    Int64 @18 = ArgumentReg(%rsi)
    Int32 @22 = Trunc(@18, Wasm: {opcode: I64Rotl, location: 5})
    Int64 @23 = RotL(@16, @22, Wasm: {opcode: I64Rotl, location: 5})
    Void @27 = Return(@23, Terminal, Wasm: {opcode: End, location: 6})

[Mark Lam]

Comment on attachment 305681 [details]
Patch

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

r=me if it builds.

> Source/JavaScriptCore/ChangeLog:9
> +        This patch adds wasm origins for B3::Values, called OpcodeOrigin. Currently,
> +        OpcodeOrigin just tracks the original opcode and the location of that opcode.

I think it would be nice to have the sample output after this paragraph.

[Keith Miller]

Created attachment 305685 [details]
Patch for landing

[Mark Lam]

Comment on attachment 305685 [details]
Patch for landing

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

> Source/JavaScriptCore/JavaScriptCore.xcodeproj/project.pbxproj:6299
> +				53C6FEF01E8AFE0C00B18425 /* WasmOpcodeOrigin.cpp */,

You need to add this to CMakeLists.txt as well.  That's what's ailing the GTK build.

[Keith Miller]

Created attachment 305705 [details]
Patch for landing

[WebKit Commit Bot]

Comment on attachment 305705 [details]
Patch for landing

Clearing flags on attachment: 305705

Committed r214529: <http://trac.webkit.org/changeset/214529>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Saam Barati]

Comment on attachment 305705 [details]
Patch for landing

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

> Source/JavaScriptCore/wasm/WasmOpcodeOrigin.h:45
> +    const OpType opcode;

Can you pack this into a 64-bit word so we don't need to heap allocate these while compiling?

[Keith Miller]

(In reply to Saam Barati from comment #9)
> Comment on attachment 305705 [details]
> Patch for landing
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=305705&action=review
> 
> > Source/JavaScriptCore/wasm/WasmOpcodeOrigin.h:45
> > +    const OpType opcode;
> 
> Can you pack this into a 64-bit word so we don't need to heap allocate these
> while compiling?

I thought of this at one point but then I decided against it; I don't know why. I put a fix in: https://bugs.webkit.org/show_bug.cgi?id=170244

[Saam Barati]

(In reply to Keith Miller from comment #10)
> (In reply to Saam Barati from comment #9)
> > Comment on attachment 305705 [details]
> > Patch for landing
> > 
> > View in context:
> > https://bugs.webkit.org/attachment.cgi?id=305705&action=review
> > 
> > > Source/JavaScriptCore/wasm/WasmOpcodeOrigin.h:45
> > > +    const OpType opcode;
> > 
> > Can you pack this into a 64-bit word so we don't need to heap allocate these
> > while compiling?
> 
> I thought of this at one point but then I decided against it; I don't know
> why. I put a fix in: https://bugs.webkit.org/show_bug.cgi?id=170244
👍🏽