diff --git a/src/passes/GlobalStructInference.cpp b/src/passes/GlobalStructInference.cpp index 166c3f58fee..824d3731dca 100644 --- a/src/passes/GlobalStructInference.cpp +++ b/src/passes/GlobalStructInference.cpp @@ -351,13 +351,10 @@ struct GlobalStructInference : public Pass { if (!global->mutable_ && !global->imported()) { if (auto* structNew = global->init->dynCast()) { auto value = readFromStructNew(structNew, fieldIndex, field); - // TODO: handle non-constant values using unnesting, as below. - if (value.isConstant()) { - replaceCurrent(value.getConstant().makeExpression(wasm)); - return; - } - // Otherwise, continue to try the main type-based optimization - // below. + // We know the exact global being read here. + value.globals.push_back(global->name); + replaceCurrent(getReadValue(value, fieldIndex, field, curr)); + return; } } } @@ -433,52 +430,6 @@ struct GlobalStructInference : public Pass { } } - // Helper for optimization: Given a Value, returns what we should read - // for it. - auto getReadValue = [&](const Value& value) -> Expression* { - Expression* ret; - if (value.isConstant()) { - // This is known to be a constant, so simply emit an expression for - // that constant, and handle if the field is packed. - ret = value.getConstant().makeExpression(wasm); - if (field) { - ret = Bits::makePackedFieldGet( - ret, *field, curr->cast()->signed_, wasm); - } - } else { - // Otherwise, this is non-constant, so we are in the situation where - // we want to un-nest the value out of the struct.new it is in. Note - // that for later work, as we cannot add a global in parallel. - - // There can only be one global in a value that is not constant, - // which is the global we want to read from. - assert(value.globals.size() == 1); - - // Create a global.get with temporary name, leaving only the - // updating of the name to later work. - auto* get = builder.makeGlobalGet(value.globals[0], - value.getExpression()->type); - - globalsToUnnest.emplace_back( - GlobalToUnnest{value.globals[0], fieldIndex, get}); - - ret = get; - } - - // If the type is more refined, we must refinalize. For example, we - // might have a struct.get that normally returns anyref, and know that - // field contains null, so we return nullref. - if (ret->type != curr->type) { - refinalize = true; - } - - // This value replaces the struct.get, so it should have the same - // source location. - debuginfo::copyOriginalToReplacement(curr, ret, getFunction()); - - return ret; - }; - // We have some globals (at least 2), and so must have at least one // value. And we have already exited if we have more than 2 values (see // the early return above) so that only leaves 1 and 2. @@ -489,7 +440,7 @@ struct GlobalStructInference : public Pass { // not need a fence. replaceCurrent(builder.makeSequence( builder.makeDrop(builder.makeRefAs(RefAsNonNull, ref)), - getReadValue(values[0]))); + getReadValue(values[0], fieldIndex, field, curr))); return; } assert(values.size() == 2); @@ -513,8 +464,8 @@ struct GlobalStructInference : public Pass { auto checkGlobal = values[0].globals[0]; // Compute the left and right values before the next line, as the order // of their execution matters (they may note globals for un-nesting). - auto* left = getReadValue(values[0]); - auto* right = getReadValue(values[1]); + auto* left = getReadValue(values[0], fieldIndex, field, curr); + auto* right = getReadValue(values[1], fieldIndex, field, curr); // Note that we must trap on null, so add a ref.as_non_null here. As // before, the get cannot have synchronized with anything. Expression* getGlobal = @@ -556,6 +507,64 @@ struct GlobalStructInference : public Pass { } return value; } + + // Given a Value, returns what we should read for it. + Expression* getReadValue(const Value& value, + Index fieldIndex, + std::optional& field, + Expression* curr) { + auto& wasm = *getModule(); + Builder builder(wasm); + + Expression* ret; + if (value.isConstant()) { + // This is known to be a constant, so simply emit an expression for + // that constant, and handle if the field is packed. + ret = value.getConstant().makeExpression(wasm); + if (field) { + ret = Bits::makePackedFieldGet( + ret, *field, curr->cast()->signed_, wasm); + } + } else { + // Otherwise, this is non-constant, so we are in the situation where + // we want to un-nest the value out of the struct.new it is in. Note + // that for later work, as we cannot add a global in parallel. + + // There can only be one global in a value that is not constant, + // which is the global we want to read from. + assert(value.globals.size() == 1); + + // Create a global.get with temporary name, leaving only the + // updating of the name to later work. + auto* get = builder.makeGlobalGet(value.globals[0], + value.getExpression()->type); + + globalsToUnnest.emplace_back( + GlobalToUnnest{value.globals[0], fieldIndex, get}); + + ret = get; + } + + // We must add a cast to non-null in some cases: A read of a null + // descriptor returns a non-null value, so if there was a null in the + // global, that would not validate by itself. + if (ret->type.isNullable() && curr->type.isNonNullable()) { + ret = builder.makeRefAs(RefAsNonNull, ret); + } + + // If the type is more refined, we must refinalize. For example, we + // might have a struct.get that normally returns anyref, and know that + // field contains null, so we return nullref. + if (ret->type != curr->type) { + refinalize = true; + } + + // This value replaces the struct.get, so it should have the same + // source location. + debuginfo::copyOriginalToReplacement(curr, ret, getFunction()); + + return ret; + } }; // Find the optimization opportunitites in parallel. diff --git a/test/lit/passes/gsi-nontype.wast b/test/lit/passes/gsi-nontype.wast index efe9082de4e..01831b2bff3 100644 --- a/test/lit/passes/gsi-nontype.wast +++ b/test/lit/passes/gsi-nontype.wast @@ -103,8 +103,8 @@ ) ) -;; As above, but the value in the struct.new is not constant, so we cannot -;; optimize. +;; As above, but the value in the struct.new is not constant. We must un-nest +;; it into another global. (module ;; CHECK: (type $table (struct (field anyref))) (type $table (struct anyref)) @@ -114,8 +114,10 @@ ;; CHECK: (import "a" "b" (global $imported funcref)) (import "a" "b" (global $imported funcref)) + ;; CHECK: (global $table.unnested.0 (ref (exact $table)) (struct.new_default $table)) + ;; CHECK: (global $table (ref $table) (struct.new $table - ;; CHECK-NEXT: (struct.new_default $table) + ;; CHECK-NEXT: (global.get $table.unnested.0) ;; CHECK-NEXT: )) (global $table (ref $table) (struct.new $table @@ -125,9 +127,7 @@ ;; CHECK: (func $test (type $1) ;; CHECK-NEXT: (drop - ;; CHECK-NEXT: (struct.get $table 0 - ;; CHECK-NEXT: (global.get $table) - ;; CHECK-NEXT: ) + ;; CHECK-NEXT: (global.get $table.unnested.0) ;; CHECK-NEXT: ) ;; CHECK-NEXT: ) (func $test @@ -204,3 +204,66 @@ ) ) +;; A packed field. +(module + ;; CHECK: (type $A (struct (field i8))) + (type $A (struct (field i8))) + + ;; CHECK: (type $1 (func)) + + ;; CHECK: (global $global (ref $A) (struct.new $A + ;; CHECK-NEXT: (i32.const -1) + ;; CHECK-NEXT: )) + (global $global (ref $A) (struct.new $A + (i32.const -1) + )) + + ;; CHECK: (func $test (type $1) + ;; CHECK-NEXT: (drop + ;; CHECK-NEXT: (i32.and + ;; CHECK-NEXT: (i32.const -1) + ;; CHECK-NEXT: (i32.const 255) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + (func $test + ;; This should be 255, not -1. + (drop + (struct.get_u $A 0 + (global.get $global) + ) + ) + ) +) + +;; When reading a null descriptor, we must cast to non-null. +(module + (rec + ;; CHECK: (rec + ;; CHECK-NEXT: (type $A (sub (descriptor $A.desc (struct)))) + (type $A (sub (descriptor $A.desc (struct)))) + ;; CHECK: (type $A.desc (sub (describes $A (struct)))) + (type $A.desc (sub (describes $A (struct)))) + ) + + ;; CHECK: (type $2 (func (result (ref $A.desc)))) + + ;; CHECK: (global $global (ref $A) (struct.new_default $A + ;; CHECK-NEXT: (ref.null none) + ;; CHECK-NEXT: )) + (global $global (ref $A) (struct.new_default $A + (ref.null none) + )) + + ;; CHECK: (func $test (type $2) (result (ref $A.desc)) + ;; CHECK-NEXT: (ref.as_non_null + ;; CHECK-NEXT: (ref.null none) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + (func $test (result (ref $A.desc)) + (ref.get_desc $A + (global.get $global) + ) + ) +) +