BBQJIT should support gc and funcref opcodes #14013

kmiller68 · 2023-05-18T14:23:59Z

`6ef70e7`

BBQJIT should support gc and funcref opcodes
https://bugs.webkit.org/show_bug.cgi?id=256959

Reviewed by Yusuke Suzuki.

This patch adds support for the various gc and funcref opcodes to the new BBQ JIT.
Most of the implementations are just translations of what the B3IRGenerator does.
The main difference is that for opcodes which need to make a C call, e.g. for allocation,
they do so by creating a `Value::fromTemp` that does not conflict with any parameter `Value`.
This is needed because otherwise the BBQJIT allocator gets confused between the existing parameters
that were not passed to the C call and the result of the C call. Also, since BBQJIT doesn't have a
good way to branch over a call both `ref.cast` and `ref.test` just call an operation.

Also, this patch fixes an issue where we weren't checking for the spec's limit on array.new_fixed
static argument count.

Lastly, there is a workaround for a clang bug where it crashed when compiling a unified source.
The workaround was to @no-unify one of the files in that bundle.

* JSTests/wasm/gc/array_new_fixed.js:
* JSTests/wasm/gc/i31.js:
(testI31Get):
* Source/JavaScriptCore/JavaScriptCore.xcodeproj/project.pbxproj:
* Source/JavaScriptCore/Sources.txt:
* Source/JavaScriptCore/runtime/Options.cpp:
(JSC::Options::notifyOptionsChanged):
* Source/JavaScriptCore/wasm/WasmBBQJIT.cpp:
(JSC::Wasm::BBQJIT::isValidValueTypeKind):
(JSC::Wasm::BBQJIT::pointerType):
(JSC::Wasm::BBQJIT::toValueKind):
(JSC::Wasm::BBQJIT::Value::fromRef):
(JSC::Wasm::BBQJIT::Value::fromTemp):
(JSC::Wasm::BBQJIT::Value::fromLocal):
(JSC::Wasm::BBQJIT::Value::pinned):
(JSC::Wasm::BBQJIT::Value::type const):
(JSC::Wasm::BBQJIT::BBQJIT):
(JSC::Wasm::BBQJIT::addTableGet):
(JSC::Wasm::BBQJIT::addTableSet):
(JSC::Wasm::BBQJIT::addTableInit):
(JSC::Wasm::BBQJIT::addTableSize):
(JSC::Wasm::BBQJIT::addTableGrow):
(JSC::Wasm::BBQJIT::addTableFill):
(JSC::Wasm::BBQJIT::addTableCopy):
(JSC::Wasm::BBQJIT::addGrowMemory):
(JSC::Wasm::BBQJIT::addMemoryFill):
(JSC::Wasm::BBQJIT::addMemoryCopy):
(JSC::Wasm::BBQJIT::addMemoryInit):
(JSC::Wasm::BBQJIT::atomicWait):
(JSC::Wasm::BBQJIT::atomicNotify):
(JSC::Wasm::BBQJIT::addI31New):
(JSC::Wasm::BBQJIT::addI31GetS):
(JSC::Wasm::BBQJIT::addI31GetU):
(JSC::Wasm::BBQJIT::getTypeDefinition):
(JSC::Wasm::BBQJIT::getArrayTypeDefinition):
(JSC::Wasm::BBQJIT::getArrayElementType):
(JSC::Wasm::BBQJIT::marshallToI64):
(JSC::Wasm::BBQJIT::addArrayNew):
(JSC::Wasm::BBQJIT::addArrayNewDefault):
(JSC::Wasm::BBQJIT::pushArrayNewFromSegment):
(JSC::Wasm::BBQJIT::addArrayNewData):
(JSC::Wasm::BBQJIT::addArrayNewElem):
(JSC::Wasm::BBQJIT::emitArraySetUnchecked):
(JSC::Wasm::BBQJIT::addArrayNewFixed):
(JSC::Wasm::BBQJIT::addArrayGet):
(JSC::Wasm::BBQJIT::addArraySet):
(JSC::Wasm::BBQJIT::addArrayLen):
(JSC::Wasm::BBQJIT::emitStructSet):
(JSC::Wasm::BBQJIT::addStructNewDefault):
(JSC::Wasm::BBQJIT::addStructNew):
(JSC::Wasm::BBQJIT::addStructGet):
(JSC::Wasm::BBQJIT::addStructSet):
(JSC::Wasm::BBQJIT::addRefTest):
(JSC::Wasm::BBQJIT::addRefCast):
(JSC::Wasm::BBQJIT::addExternInternalize):
(JSC::Wasm::BBQJIT::emitThrowOnNullReference):
(JSC::Wasm::BBQJIT::addI32Popcnt):
(JSC::Wasm::BBQJIT::addI64Popcnt):
(JSC::Wasm::BBQJIT::addRefFunc):
(JSC::Wasm::BBQJIT::toB3Type):
(JSC::Wasm::BBQJIT::emitCCall):
(JSC::Wasm::BBQJIT::addCallRef):
* Source/JavaScriptCore/wasm/WasmFunctionParser.h:
(JSC::Wasm::FunctionParser<Context>::parseExpression):
* Source/JavaScriptCore/wasm/WasmLimits.h:
* Source/JavaScriptCore/wasm/WasmOperations.cpp:
(JSC::Wasm::JSC_DEFINE_JIT_OPERATION):
* Source/JavaScriptCore/wasm/WasmOperations.h:

Canonical link: https://commits.webkit.org/264638@main

450b85b

Misc	iOS, tvOS & watchOS	macOS	Linux	Windows
✅ 🧪 style	✅ 🛠 ios	✅ 🛠 mac	✅ 🛠 wpe	✅ 🛠 wincairo
	✅ 🛠 ios-sim	✅ 🛠 mac-AS-debug	✅ 🧪 wpe-wk2
✅ 🧪 webkitperl	✅ 🧪 ios-wk2	✅ 🧪 api-mac	✅ 🛠 gtk
	✅ 🧪 ios-wk2-wpt	✅ 🧪 mac-wk1	✅ 🧪 gtk-wk2
✅ 🛠 🧪 jsc	✅ 🧪 api-ios	✅ 🧪 mac-wk2	✅ 🧪 api-gtk
✅ 🛠 🧪 jsc-arm64	✅ 🛠 tv	✅ 🧪 mac-AS-debug-wk2	✅ 🛠 jsc-armv7
	✅ 🛠 tv-sim		✅ 🧪 jsc-armv7-tests
✅ 🛠 🧪 merge	✅ 🛠 watch		✅ 🛠 jsc-mips
	✅ 🛠 watch-sim		✅ 🧪 jsc-mips-tests

webkit-early-warning-system · 2023-05-18T14:24:11Z

EWS run on previous version of this PR (hash e038447)

Misc	iOS, tvOS & watchOS	macOS	Linux	Windows
✅ 🧪 style	❌ 🛠 ios	❌ 🛠 mac	❌ 🛠 wpe	✅ 🛠 wincairo
	❌ 🛠 ios-sim	❌ 🛠 mac-AS-debug	❌ 🧪 wpe-wk2
✅ 🧪 webkitperl	❌ 🧪 ios-wk2	❌ 🧪 api-mac	❌ 🛠 gtk
	❌ 🧪 ios-wk2-wpt	❌ 🧪 mac-wk1	❌ 🧪 gtk-wk2
❌ 🛠 🧪 jsc	❌ 🧪 api-ios	❌ 🧪 mac-wk2	❌ 🧪 api-gtk
❌ 🛠 🧪 jsc-arm64	❌ 🛠 tv	❌ 🧪 mac-AS-debug-wk2	✅ 🛠 jsc-armv7
	❌ 🛠 tv-sim	❌ 🧪 mac-wk2-stress	✅ 🧪 jsc-armv7-tests
	❌ 🛠 watch		✅ 🛠 jsc-mips
	❌ 🛠 watch-sim		✅ 🧪 jsc-mips-tests

webkit-early-warning-system · 2023-05-18T15:31:36Z

EWS run on previous version of this PR (hash 3d4d4d7)

Misc	iOS, tvOS & watchOS	macOS	Linux	Windows
✅ 🧪 style	✅ 🛠 ios	✅ 🛠 mac	✅ 🛠 wpe	✅ 🛠 wincairo
	✅ 🛠 ios-sim	✅ 🛠 mac-AS-debug	✅ 🧪 wpe-wk2
✅ 🧪 webkitperl	✅ 🧪 ios-wk2	✅ 🧪 api-mac	✅ 🛠 gtk
	✅ 🧪 ios-wk2-wpt	✅ 🧪 mac-wk1	✅ 🧪 gtk-wk2
❌ 🛠 🧪 jsc	✅ 🧪 api-ios	~~🧪 mac-wk2~~	✅ 🧪 api-gtk
✅ 🛠 🧪 jsc-arm64	✅ 🛠 tv	✅ 🧪 mac-AS-debug-wk2	✅ 🛠 jsc-armv7
	✅ 🛠 tv-sim		✅ 🧪 jsc-armv7-tests
	✅ 🛠 watch		✅ 🛠 jsc-mips
	✅ 🛠 watch-sim		✅ 🧪 jsc-mips-tests

webkit-early-warning-system · 2023-05-18T17:39:24Z

EWS run on previous version of this PR (hash 17ecb7a)

Misc	iOS, tvOS & watchOS	macOS	Linux	Windows
✅ 🧪 style	✅ 🛠 ios	✅ 🛠 mac	✅ 🛠 wpe	✅ 🛠 wincairo
	✅ 🛠 ios-sim	✅ 🛠 mac-AS-debug	✅ 🧪 wpe-wk2
✅ 🧪 webkitperl	✅ 🧪 ios-wk2	✅ 🧪 api-mac	✅ 🛠 gtk
	✅ 🧪 ios-wk2-wpt	✅ 🧪 mac-wk1	✅ 🧪 gtk-wk2
✅ 🛠 🧪 jsc	✅ 🧪 api-ios	✅ 🧪 mac-wk2	✅ 🧪 api-gtk
✅ 🛠 🧪 jsc-arm64	✅ 🛠 tv	✅ 🧪 mac-AS-debug-wk2	✅ 🛠 jsc-armv7
	✅ 🛠 tv-sim		✅ 🧪 jsc-armv7-tests
	✅ 🛠 watch		✅ 🛠 jsc-mips
	✅ 🛠 watch-sim		✅ 🧪 jsc-mips-tests

Constellation · 2023-05-19T00:59:07Z

Source/JavaScriptCore/wasm/WasmBBQJIT.cpp

+        case TypeKind::Eqref:
+        case TypeKind::Anyref:
+        case TypeKind::Nullref:
+            // FIXME: Should this just be I64?


I think they are I64 since it is EncodedJSValue (it can also be jsNull etc.).

Yeah, I can do that.

Constellation · 2023-05-19T01:16:19Z

Source/JavaScriptCore/wasm/WasmBBQJIT.cpp

+        return { };
+    }
+
+    void emitStructSet(Value structValue, const StructType& structType, uint32_t fieldIndex, Value value)


I wonder if we should pass GPRReg structValueGPR here since it is confusing that we consume value while we do not consume structValue.

Constellation · 2023-05-19T01:17:19Z

Source/JavaScriptCore/wasm/WasmBBQJIT.cpp

+        }
+
+        Location arrayLocation = loadIfNecessary(arrayref);
+        emitThrowOnNullReference(ExceptionType::NullArrayLen, arrayLocation);


Dont we need to consume arrayref?

Yes, fixed. I think it kinda works right now because result is the same temp as array and gets the same location. I was trying to think of a validation for this but I couldn't come up with anything that would obviously work.

Constellation · 2023-05-19T01:21:57Z

Source/JavaScriptCore/wasm/WasmBBQJIT.cpp

+            return { };
+        }
+
+        Location arrayLocation = loadIfNecessary(arrayref);


Don't we need to consume arrayref?

No, it get's consumed by the emitCCall in emitArraySetUnchecked.

webkit-early-warning-system · 2023-05-19T15:05:56Z

EWS run on previous version of this PR (hash d7ed8c0)

Misc	iOS, tvOS & watchOS	macOS	Linux	Windows
✅ 🧪 style	✅ 🛠 ios	✅ 🛠 mac	✅ 🛠 wpe	✅ 🛠 wincairo
	✅ 🛠 ios-sim	✅ 🛠 mac-AS-debug	✅ 🧪 wpe-wk2
✅ 🧪 webkitperl	✅ 🧪 ios-wk2	✅ 🧪 api-mac	✅ 🛠 gtk
	✅ 🧪 ios-wk2-wpt	✅ 🧪 mac-wk1	✅ 🧪 gtk-wk2
✅ 🛠 🧪 jsc	✅ 🧪 api-ios	✅ 🧪 mac-wk2	✅ 🧪 api-gtk
✅ 🛠 🧪 jsc-arm64	✅ 🛠 tv	✅ 🧪 mac-AS-debug-wk2	✅ 🛠 jsc-armv7
	✅ 🛠 tv-sim		✅ 🧪 jsc-armv7-tests
	✅ 🛠 watch		✅ 🛠 jsc-mips
	✅ 🛠 watch-sim		✅ 🧪 jsc-mips-tests

ddegazio · 2023-05-19T19:56:19Z

Source/JavaScriptCore/wasm/WasmBBQJIT.cpp

+
+        LOG_INSTRUCTION("I31New", value, RESULT(result));
+
+        // Should I be able to load this from an Address? Doesn't matter on ARM tho.


On this, right now we tend to just materialize constants inline throughout BBQJIT. We don't have the luxury of lifting shared constants or knowing if a function will ever use i.e. NumberTag (hence we'd have to conservatively always reserve a slot for it). So this is consistent with how other constants are handled in BBQJIT, although we could definitely consider improving this in the future.

AH, yeah I should have deleted that comment. I wrote that when as part of the first op I implemented so I didn't know what I was doing.

ddegazio · 2023-05-19T20:23:42Z

Source/JavaScriptCore/wasm/WasmBBQJIT.cpp

+            Value::fromI32(typeIndex),
+            Value::fromI32(args.size()),
+        };
+        Value allocationResult = Value::fromTemp(TypeKind::Structref, currentControlData().enclosedHeight() + currentControlData().implicitSlots() + m_parser->expressionStack().size() + args.size());


Why is this Structref and not Arrayref? Or I64 for that matter (since operationWasmArrayNewEmpty returns an encoded JSValue)?

Copy paste error, will fix.

ddegazio · 2023-05-19T20:25:59Z

Source/JavaScriptCore/wasm/WasmBBQJIT.cpp

+            emitArraySetUnchecked(typeIndex, pinnedResult, Value::fromI32(i), args[i]);
+        }
+
+        result = topValue(TypeKind::Structref);


ddegazio · 2023-05-19T20:29:57Z

Source/JavaScriptCore/wasm/WasmBBQJIT.cpp

@@ -385,6 +403,38 @@ class BBQJIT {
        return 0;
    }

+    static TypeKind toValueKind(TypeKind kind)


If we're going to start caring about distinguishing between BBQ value kind and WASM type, maybe this is the time to define a new enum ValueKind for the former instead of using TypeKind in the current ad-hoc way. Would make it a lot clearer what a given type is being used for at any given time.

Yeah, I think I'll do that in a different patch though.

ddegazio · 2023-05-19T20:36:25Z

Source/JavaScriptCore/wasm/WasmBBQJIT.cpp

+        case TypeKind::Eqref:
+        case TypeKind::Anyref:
+        case TypeKind::Nullref:
+            return TypeKind::I64;


Maybe we can define an alias for TypeKind::I64 called something like TypeKind::JSValue (or preferably, we put this in some new ValueKind enumeration). Functionally this wouldn't change anything (and I guess we don't go this route in B3), but looking at the instruction implementations below it could be helpful for following whether we're using an I64 as an arithmetic integer v.s. as a means for storing a JS value. Maybe also enables better assertions and logging.

Yeah, that'd be a good idea.

ddegazio · 2023-05-19T20:38:48Z

Source/JavaScriptCore/wasm/WasmBBQJIT.cpp

+
+        if (isFloatingPointType(resultType.kind)) {
+            consume(getResult);
+            result = Value::fromTemp(resultType.kind, currentControlData().enclosedHeight() + currentControlData().implicitSlots() + m_parser->expressionStack().size());


I think this can just be result = topValue(resultType.kind);

ddegazio · 2023-05-19T20:54:12Z

Source/JavaScriptCore/wasm/WasmBBQJIT.cpp

    {
        // Currently, we assume the Wasm calling convention is the same as the C calling convention
-        Vector<Type, 1> resultTypes = { Type { returnType, 0u } };
+        Vector<Type, 1> resultTypes = { Type { result.type(), 0u } };


Let's add an ASSERT(!result.isNone()); to make sure nobody accidentally passes in an empty value.

There's an ASSERT(result.isTemp()) below. I'll move that to the top of the function.

ddegazio · 2023-05-19T21:11:55Z

Source/JavaScriptCore/wasm/WasmBBQJIT.cpp

-                else
-                    emitCCall(&operationPopcount32, Vector<Value> { operand }, TypeKind::I32, result);
+                else {
+                    result = topValue(TypeKind::I32);


The EMIT_UNARY(...) macro will already assign result to the top value on the stack and give it a register, so it should be able to be passed in. However, this creates a somewhat nasty tacit dependency - emitCCall will bind result to the returnValueGPR, which will error if result is already bound to a different register (to avoid mucking with the register allocator state). It shouldn't currently error specifically because we only allocate caller-saved registers, which get flushed across the call regardless; however, if we add callee-saved register allocation to BBQJIT in the future, we could get very niche errors.

All this is to say, I think the most correct thing with the current abstractions would be to

consume(result); emitCCall(&operationPopcount32, Vector<Value> { operand }, result);

so result is quietly unbound from its current register before we pass it into emitCCall. This only applies to addI32Popcnt and addI64Popcnt since they are currently the only functions that use EMIT_UNARY/EMIT_BINARY and also contain a call.

I'll make the change to consume. Unfortunately I think we won't have any bots that test this code path anyway. Since I don't think we have any bots that run on old enough CPUs that they'll fail the supportsCountPopulation() test now that arm64 always passes. (For reference the first Intel processor with POPCNT came out 14 years ago https://en.wikipedia.org/wiki/Nehalem_(microarchitecture) and I think earlier for AMD)

ddegazio · 2023-05-19T21:17:08Z

Source/JavaScriptCore/wasm/WasmBBQJIT.cpp

-                else
-                    emitCCall(&operationPopcount64, Vector<Value> { operand }, TypeKind::I32, result);
+                else {
+                    result = topValue(TypeKind::I32);


This isn't strictly related to your patch but I just realized this is slightly wrong...operationPopcount64 returns a 64-bit result, not a 32-bit one, so I32 is the wrong type. If we reuse the result from EMIT_UNARY like I mentioned in my previous comment, it should have the correct type already; otherwise, this should probably use TypeKind::I64.

Fixed via switching it to consume.

webkit-early-warning-system · 2023-05-20T14:58:46Z

EWS run on previous version of this PR (hash 8fe886f)

Misc	iOS, tvOS & watchOS	macOS	Linux	Windows
✅ 🧪 style	✅ 🛠 ios	✅ 🛠 mac	✅ 🛠 wpe	✅ 🛠 wincairo
	✅ 🛠 ios-sim	✅ 🛠 mac-AS-debug	✅ 🧪 wpe-wk2
✅ 🧪 webkitperl	✅ 🧪 ios-wk2	✅ 🧪 api-mac	✅ 🛠 gtk
	✅ 🧪 ios-wk2-wpt	✅ 🧪 mac-wk1	❌ 🧪 gtk-wk2
✅ 🛠 🧪 jsc	✅ 🧪 api-ios	✅ 🧪 mac-wk2	✅ 🧪 api-gtk
✅ 🛠 🧪 jsc-arm64	✅ 🛠 tv	✅ 🧪 mac-AS-debug-wk2	✅ 🛠 jsc-armv7
	✅ 🛠 tv-sim		✅ 🧪 jsc-armv7-tests
	✅ 🛠 watch		✅ 🛠 jsc-mips
	✅ 🛠 watch-sim		✅ 🧪 jsc-mips-tests

Constellation

r=me with comments and update of addCallRef.

Constellation · 2023-05-23T22:18:23Z

Source/JavaScriptCore/wasm/WasmBBQJIT.cpp

-                else
-                    emitCCall(&operationPopcount32, Vector<Value> { operand }, TypeKind::I32, result);
+                else {
+                    consume(result);


Can we have explicit detailed comment about this hack here?

Constellation · 2023-05-23T22:18:29Z

Source/JavaScriptCore/wasm/WasmBBQJIT.cpp

-                else
-                    emitCCall(&operationPopcount64, Vector<Value> { operand }, TypeKind::I32, result);
+                else {
+                    consume(result);


Constellation · 2023-05-23T22:20:17Z

Source/JavaScriptCore/wasm/WasmBBQJIT.cpp

+        {
+            ScratchScope<3, 0> scratches(*this);


Please update this code to the latest convention. calleeCode always needs to be in GPRInfo::nonPreservedNonArgumentGPR1 to make shuffling work correctly. See updated addCallIndirect code changed in 76f47aa

webkit-early-warning-system · 2023-05-24T14:00:30Z

EWS run on previous version of this PR (hash d85acf3)

Misc	iOS, tvOS & watchOS	macOS	Linux	Windows
✅ 🧪 style	✅ 🛠 ios	✅ 🛠 mac	✅ 🛠 wpe	✅ 🛠 wincairo
	✅ 🛠 ios-sim	✅ 🛠 mac-AS-debug	✅ 🧪 wpe-wk2
✅ 🧪 webkitperl	✅ 🧪 ios-wk2	✅ 🧪 api-mac	✅ 🛠 gtk
	✅ 🧪 ios-wk2-wpt	✅ 🧪 mac-wk1	✅ 🧪 gtk-wk2
❌ 🛠 🧪 jsc	✅ 🧪 api-ios	✅ 🧪 mac-wk2	✅ 🧪 api-gtk
✅ 🛠 🧪 jsc-arm64	✅ 🛠 tv	✅ 🧪 mac-AS-debug-wk2	✅ 🛠 jsc-armv7
	✅ 🛠 tv-sim		✅ 🧪 jsc-armv7-tests
	✅ 🛠 watch		✅ 🛠 jsc-mips
	✅ 🛠 watch-sim		✅ 🧪 jsc-mips-tests

Constellation · 2023-05-25T16:40:44Z

Source/JavaScriptCore/wasm/WasmBBQJIT.cpp

+        GPRReg calleeInstance;
+        GPRReg calleeCode;
+        GPRReg jsCalleeAnchor;
+        {


You need clobber(GPRInfo::nonPreservedNonArgumentGPR1)

webkit-early-warning-system · 2023-05-26T16:24:14Z

EWS run on previous version of this PR (hash 66892b6)

Misc	iOS, tvOS & watchOS	macOS	Linux	Windows
✅ 🧪 style	✅ 🛠 ios	✅ 🛠 mac	✅ 🛠 wpe	✅ 🛠 wincairo
	✅ 🛠 ios-sim	❌ 🛠 mac-AS-debug	✅ 🧪 wpe-wk2
✅ 🧪 webkitperl	✅ 🧪 ios-wk2	✅ 🧪 api-mac	✅ 🛠 gtk
	✅ 🧪 ios-wk2-wpt	✅ 🧪 mac-wk1	✅ 🧪 gtk-wk2
✅ 🛠 🧪 jsc	✅ 🧪 api-ios	✅ 🧪 mac-wk2	✅ 🧪 api-gtk
✅ 🛠 🧪 jsc-arm64	✅ 🛠 tv	✅ 🧪 mac-AS-debug-wk2	✅ 🛠 jsc-armv7
	✅ 🛠 tv-sim		✅ 🧪 jsc-armv7-tests
	✅ 🛠 watch		✅ 🛠 jsc-mips
	✅ 🛠 watch-sim		✅ 🧪 jsc-mips-tests

webkit-early-warning-system · 2023-05-27T00:54:17Z

EWS run on previous version of this PR (hash 6eeab0a)

Misc	iOS, tvOS & watchOS	macOS	Linux	Windows
✅ 🧪 style	❌ 🛠 ios	❌ 🛠 mac	✅ 🛠 wpe	⏳ 🛠 wincairo
	❌ 🛠 ios-sim	❌ 🛠 mac-AS-debug	~~🧪 wpe-wk2~~
✅ 🧪 webkitperl	❌ 🧪 ios-wk2	❌ 🧪 api-mac	✅ 🛠 gtk
	❌ 🧪 ios-wk2-wpt	❌ 🧪 mac-wk1	⏳ 🧪 gtk-wk2
⏳ 🛠 🧪 jsc	❌ 🧪 api-ios	❌ 🧪 mac-wk2	~~🧪 api-gtk~~
⏳ 🛠 🧪 jsc-arm64	~~🛠 tv~~	❌ 🧪 mac-AS-debug-wk2	✅ 🛠 jsc-armv7
	⏳ 🛠 tv-sim	❌ 🧪 mac-wk2-stress	⏳ 🧪 jsc-armv7-tests
	⏳ 🛠 watch		✅ 🛠 jsc-mips
	❌ 🛠 watch-sim		⏳ 🧪 jsc-mips-tests

webkit-early-warning-system · 2023-05-27T01:06:25Z

EWS run on previous version of this PR (hash 2067201)

Misc	iOS, tvOS & watchOS	macOS	Linux	Windows
✅ 🧪 style	✅ 🛠 ios	✅ 🛠 mac	✅ 🛠 wpe	✅ 🛠 wincairo
	✅ 🛠 ios-sim	✅ 🛠 mac-AS-debug	❌ 🧪 wpe-wk2
✅ 🧪 webkitperl	✅ 🧪 ios-wk2	✅ 🧪 api-mac	✅ 🛠 gtk
	✅ 🧪 ios-wk2-wpt	✅ 🧪 mac-wk1	✅ 🧪 gtk-wk2
✅ 🛠 🧪 jsc	✅ 🧪 api-ios	✅ 🧪 mac-wk2	✅ 🧪 api-gtk
✅ 🛠 🧪 jsc-arm64	✅ 🛠 tv	✅ 🧪 mac-AS-debug-wk2	✅ 🛠 jsc-armv7
	✅ 🛠 tv-sim		✅ 🧪 jsc-armv7-tests
	✅ 🛠 watch		✅ 🛠 jsc-mips
	✅ 🛠 watch-sim		✅ 🧪 jsc-mips-tests

webkit-early-warning-system · 2023-05-27T17:31:52Z

EWS run on current version of this PR (hash 450b85b)

Misc	iOS, tvOS & watchOS	macOS	Linux	Windows
🧪 style	🛠 ios	🛠 mac	🛠 wpe	⏳ 🛠 wincairo
	🛠 ios-sim	🛠 mac-AS-debug	🧪 wpe-wk2
🧪 webkitperl	🧪 ios-wk2	🧪 api-mac	🛠 gtk
	🧪 ios-wk2-wpt	🧪 mac-wk1	🧪 gtk-wk2
⏳ 🛠 🧪 jsc	🧪 api-ios	🧪 mac-wk2	🧪 api-gtk
🛠 🧪 jsc-arm64	🛠 tv	🧪 mac-AS-debug-wk2	🛠 jsc-armv7
	🛠 tv-sim	🧪 mac-wk2-stress	🧪 jsc-armv7-tests
	🛠 watch		🛠 jsc-mips
	🛠 watch-sim		🧪 jsc-mips-tests

webkit-early-warning-system · 2023-05-27T17:31:54Z

EWS run on current version of this PR (hash 450b85b)

Misc	iOS, tvOS & watchOS	macOS	Linux	Windows
✅ 🧪 style	✅ 🛠 ios	✅ 🛠 mac	✅ 🛠 wpe	✅ 🛠 wincairo
	✅ 🛠 ios-sim	✅ 🛠 mac-AS-debug	✅ 🧪 wpe-wk2
✅ 🧪 webkitperl	✅ 🧪 ios-wk2	✅ 🧪 api-mac	✅ 🛠 gtk
	✅ 🧪 ios-wk2-wpt	✅ 🧪 mac-wk1	✅ 🧪 gtk-wk2
✅ 🛠 🧪 jsc	✅ 🧪 api-ios	✅ 🧪 mac-wk2	✅ 🧪 api-gtk
✅ 🛠 🧪 jsc-arm64	✅ 🛠 tv	✅ 🧪 mac-AS-debug-wk2	✅ 🛠 jsc-armv7
	✅ 🛠 tv-sim		✅ 🧪 jsc-armv7-tests
✅ 🛠 🧪 merge	✅ 🛠 watch		✅ 🛠 jsc-mips
	✅ 🛠 watch-sim		✅ 🧪 jsc-mips-tests

https://bugs.webkit.org/show_bug.cgi?id=256959 Reviewed by Yusuke Suzuki. This patch adds support for the various gc and funcref opcodes to the new BBQ JIT. Most of the implementations are just translations of what the B3IRGenerator does. The main difference is that for opcodes which need to make a C call, e.g. for allocation, they do so by creating a `Value::fromTemp` that does not conflict with any parameter `Value`. This is needed because otherwise the BBQJIT allocator gets confused between the existing parameters that were not passed to the C call and the result of the C call. Also, since BBQJIT doesn't have a good way to branch over a call both `ref.cast` and `ref.test` just call an operation. Also, this patch fixes an issue where we weren't checking for the spec's limit on array.new_fixed static argument count. Lastly, there is a workaround for a clang bug where it crashed when compiling a unified source. The workaround was to @no-unify one of the files in that bundle. * JSTests/wasm/gc/array_new_fixed.js: * JSTests/wasm/gc/i31.js: (testI31Get): * Source/JavaScriptCore/JavaScriptCore.xcodeproj/project.pbxproj: * Source/JavaScriptCore/Sources.txt: * Source/JavaScriptCore/runtime/Options.cpp: (JSC::Options::notifyOptionsChanged): * Source/JavaScriptCore/wasm/WasmBBQJIT.cpp: (JSC::Wasm::BBQJIT::isValidValueTypeKind): (JSC::Wasm::BBQJIT::pointerType): (JSC::Wasm::BBQJIT::toValueKind): (JSC::Wasm::BBQJIT::Value::fromRef): (JSC::Wasm::BBQJIT::Value::fromTemp): (JSC::Wasm::BBQJIT::Value::fromLocal): (JSC::Wasm::BBQJIT::Value::pinned): (JSC::Wasm::BBQJIT::Value::type const): (JSC::Wasm::BBQJIT::BBQJIT): (JSC::Wasm::BBQJIT::addTableGet): (JSC::Wasm::BBQJIT::addTableSet): (JSC::Wasm::BBQJIT::addTableInit): (JSC::Wasm::BBQJIT::addTableSize): (JSC::Wasm::BBQJIT::addTableGrow): (JSC::Wasm::BBQJIT::addTableFill): (JSC::Wasm::BBQJIT::addTableCopy): (JSC::Wasm::BBQJIT::addGrowMemory): (JSC::Wasm::BBQJIT::addMemoryFill): (JSC::Wasm::BBQJIT::addMemoryCopy): (JSC::Wasm::BBQJIT::addMemoryInit): (JSC::Wasm::BBQJIT::atomicWait): (JSC::Wasm::BBQJIT::atomicNotify): (JSC::Wasm::BBQJIT::addI31New): (JSC::Wasm::BBQJIT::addI31GetS): (JSC::Wasm::BBQJIT::addI31GetU): (JSC::Wasm::BBQJIT::getTypeDefinition): (JSC::Wasm::BBQJIT::getArrayTypeDefinition): (JSC::Wasm::BBQJIT::getArrayElementType): (JSC::Wasm::BBQJIT::marshallToI64): (JSC::Wasm::BBQJIT::addArrayNew): (JSC::Wasm::BBQJIT::addArrayNewDefault): (JSC::Wasm::BBQJIT::pushArrayNewFromSegment): (JSC::Wasm::BBQJIT::addArrayNewData): (JSC::Wasm::BBQJIT::addArrayNewElem): (JSC::Wasm::BBQJIT::emitArraySetUnchecked): (JSC::Wasm::BBQJIT::addArrayNewFixed): (JSC::Wasm::BBQJIT::addArrayGet): (JSC::Wasm::BBQJIT::addArraySet): (JSC::Wasm::BBQJIT::addArrayLen): (JSC::Wasm::BBQJIT::emitStructSet): (JSC::Wasm::BBQJIT::addStructNewDefault): (JSC::Wasm::BBQJIT::addStructNew): (JSC::Wasm::BBQJIT::addStructGet): (JSC::Wasm::BBQJIT::addStructSet): (JSC::Wasm::BBQJIT::addRefTest): (JSC::Wasm::BBQJIT::addRefCast): (JSC::Wasm::BBQJIT::addExternInternalize): (JSC::Wasm::BBQJIT::emitThrowOnNullReference): (JSC::Wasm::BBQJIT::addI32Popcnt): (JSC::Wasm::BBQJIT::addI64Popcnt): (JSC::Wasm::BBQJIT::addRefFunc): (JSC::Wasm::BBQJIT::toB3Type): (JSC::Wasm::BBQJIT::emitCCall): (JSC::Wasm::BBQJIT::addCallRef): * Source/JavaScriptCore/wasm/WasmFunctionParser.h: (JSC::Wasm::FunctionParser<Context>::parseExpression): * Source/JavaScriptCore/wasm/WasmLimits.h: * Source/JavaScriptCore/wasm/WasmOperations.cpp: (JSC::Wasm::JSC_DEFINE_JIT_OPERATION): * Source/JavaScriptCore/wasm/WasmOperations.h: Canonical link: https://commits.webkit.org/264638@main

webkit-commit-queue · 2023-05-28T15:29:14Z

Committed 264638@main (6ef70e7): https://commits.webkit.org/264638@main

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

kmiller68 requested a review from a team as a code owner May 18, 2023 14:23

kmiller68 marked this pull request as draft May 18, 2023 14:24

kmiller68 changed the title ~~changes~~ BBQJIT should support gc and funcref opcodes May 18, 2023

webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label May 18, 2023

kmiller68 removed the merging-blocked Applied to prevent a change from being merged label May 18, 2023

kmiller68 force-pushed the eng/BBQJIT-should-support-gc-and-funcref-opcodes branch from e038447 to 3d4d4d7 Compare May 18, 2023 15:30

kmiller68 self-assigned this May 18, 2023

kmiller68 added the WebAssembly For bugs in JavaScript WebAssembly label May 18, 2023

kmiller68 marked this pull request as ready for review May 18, 2023 17:01

kmiller68 force-pushed the eng/BBQJIT-should-support-gc-and-funcref-opcodes branch from 3d4d4d7 to 17ecb7a Compare May 18, 2023 17:38

Constellation reviewed May 19, 2023

View reviewed changes

kmiller68 force-pushed the eng/BBQJIT-should-support-gc-and-funcref-opcodes branch from 17ecb7a to d7ed8c0 Compare May 19, 2023 15:05

ddegazio reviewed May 19, 2023

View reviewed changes

kmiller68 force-pushed the eng/BBQJIT-should-support-gc-and-funcref-opcodes branch from d7ed8c0 to 8fe886f Compare May 20, 2023 14:55

webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label May 20, 2023

Constellation approved these changes May 23, 2023

View reviewed changes

kmiller68 removed the merging-blocked Applied to prevent a change from being merged label May 24, 2023

kmiller68 force-pushed the eng/BBQJIT-should-support-gc-and-funcref-opcodes branch from 8fe886f to d85acf3 Compare May 24, 2023 14:00

Constellation reviewed May 25, 2023

View reviewed changes

kmiller68 force-pushed the eng/BBQJIT-should-support-gc-and-funcref-opcodes branch from d85acf3 to 66892b6 Compare May 26, 2023 16:24

webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label May 26, 2023

kmiller68 removed the merging-blocked Applied to prevent a change from being merged label May 27, 2023

kmiller68 force-pushed the eng/BBQJIT-should-support-gc-and-funcref-opcodes branch from 66892b6 to 6eeab0a Compare May 27, 2023 00:54

webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label May 27, 2023

kmiller68 removed the merging-blocked Applied to prevent a change from being merged label May 27, 2023

webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label May 27, 2023

kmiller68 force-pushed the eng/BBQJIT-should-support-gc-and-funcref-opcodes branch from 6eeab0a to 2067201 Compare May 27, 2023 01:06

kmiller68 removed the merging-blocked Applied to prevent a change from being merged label May 27, 2023

kmiller68 force-pushed the eng/BBQJIT-should-support-gc-and-funcref-opcodes branch from 2067201 to 450b85b Compare May 27, 2023 17:31

kmiller68 added merge-queue Applied to send a pull request to merge-queue and removed merge-queue Applied to send a pull request to merge-queue labels May 27, 2023

webkit-commit-queue force-pushed the eng/BBQJIT-should-support-gc-and-funcref-opcodes branch from 450b85b to 6ef70e7 Compare May 28, 2023 15:29

webkit-commit-queue removed the merge-queue Applied to send a pull request to merge-queue label May 28, 2023

webkit-commit-queue merged commit 6ef70e7 into WebKit:main May 28, 2023


		LOG_INSTRUCTION("I31New", value, RESULT(result));

		// Should I be able to load this from an Address? Doesn't matter on ARM tho.

BBQJIT should support gc and funcref opcodes #14013

BBQJIT should support gc and funcref opcodes #14013

Conversation

kmiller68 commented May 18, 2023 • edited by webkit-early-warning-system

webkit-early-warning-system commented May 18, 2023 • edited

webkit-early-warning-system commented May 18, 2023 • edited

webkit-early-warning-system commented May 18, 2023 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

webkit-early-warning-system commented May 19, 2023 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ddegazio May 19, 2023 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

webkit-early-warning-system commented May 20, 2023 • edited

Constellation left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

webkit-early-warning-system commented May 24, 2023 • edited

Choose a reason for hiding this comment

webkit-early-warning-system commented May 26, 2023 • edited

webkit-early-warning-system commented May 27, 2023 • edited

webkit-early-warning-system commented May 27, 2023 • edited

webkit-early-warning-system commented May 27, 2023 • edited

webkit-early-warning-system commented May 27, 2023 • edited

webkit-commit-queue commented May 28, 2023

kmiller68 commented May 18, 2023 •

edited by webkit-early-warning-system

webkit-early-warning-system commented May 18, 2023 •

edited

webkit-early-warning-system commented May 18, 2023 •

edited

webkit-early-warning-system commented May 18, 2023 •

edited

webkit-early-warning-system commented May 19, 2023 •

edited

ddegazio May 19, 2023 •

edited

webkit-early-warning-system commented May 20, 2023 •

edited

webkit-early-warning-system commented May 24, 2023 •

edited

webkit-early-warning-system commented May 26, 2023 •

edited

webkit-early-warning-system commented May 27, 2023 •

edited

webkit-early-warning-system commented May 27, 2023 •

edited

webkit-early-warning-system commented May 27, 2023 •

edited

webkit-early-warning-system commented May 27, 2023 •

edited