Skip to content

Avoid newly exposing prototypes in GTO #9043

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
tlively wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Avoid newly exposing prototypes in GTO #9043

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 13 additions & 7 deletions src/ir/js-utils.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,18 @@

namespace wasm::JSUtils {

// Whether a field is immutable and a reference to a subtype of externref that
// could hold a JS prototype.
inline bool isPossibleJSPrototypeField(const Field& field) {
if (field.mutable_ != Immutable) {
return false;
}
if (!field.type.isRef()) {
return false;
}
return field.type.getHeapType().isMaybeShared(HeapType::ext);
}

// Whether this is a descriptor struct type whose first field is immutable and a
// subtype of externref.
inline bool hasPossibleJSPrototypeField(HeapType type) {
Expand All @@ -34,13 +46,7 @@ inline bool hasPossibleJSPrototypeField(HeapType type) {
if (fields.empty()) {
return false;
}
if (fields[0].mutable_ == Mutable) {
return false;
}
if (!fields[0].type.isRef()) {
return false;
}
return fields[0].type.getHeapType().isMaybeShared(HeapType::ext);
return isPossibleJSPrototypeField(fields[0]);
}

// Calls flowIn and flowOut on all types that may flow in from or out to JS.
Expand Down
191 changes: 145 additions & 46 deletions src/passes/GlobalTypeOptimization.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,16 +70,20 @@ struct FieldInfo {

struct FieldInfoScanner
: public StructUtils::StructScanner<FieldInfo, FieldInfoScanner> {
std::unordered_map<Function*, std::vector<Type>>& jsExposedTypes;

std::unique_ptr<Pass> create() override {
return std::make_unique<FieldInfoScanner>(functionNewInfos,
functionSetGetInfos);
return std::make_unique<FieldInfoScanner>(
functionNewInfos, functionSetGetInfos, jsExposedTypes);
}

FieldInfoScanner(
StructUtils::FunctionStructValuesMap<FieldInfo>& functionNewInfos,
StructUtils::FunctionStructValuesMap<FieldInfo>& functionSetGetInfos)
StructUtils::FunctionStructValuesMap<FieldInfo>& functionSetGetInfos,
std::unordered_map<Function*, std::vector<Type>>& jsExposedTypes)
: StructUtils::StructScanner<FieldInfo, FieldInfoScanner>(
functionNewInfos, functionSetGetInfos) {}
functionNewInfos, functionSetGetInfos),
jsExposedTypes(jsExposedTypes) {}

void noteExpression(Expression* expr,
HeapType type,
Expand Down Expand Up @@ -116,16 +120,8 @@ struct FieldInfoScanner
// Converting a reference to externref makes the prototype field on its
// descriptor available to be read by JS, if such a field exists.
void visitRefAs(RefAs* curr) {
if (curr->op != ExternConvertAny) {
return;
}
if (!curr->value->type.isRef()) {
return;
}
if (auto desc = curr->value->type.getHeapType().getDescriptorType();
desc && JSUtils::hasPossibleJSPrototypeField(*desc)) {
auto exact = curr->value->type.getExactness();
functionSetGetInfos[getFunction()][{*desc, exact}][0].noteRead();
if (curr->op == ExternConvertAny && curr->value->type.isRef()) {
jsExposedTypes.at(getFunction()).push_back(curr->value->type);
}
}
};
Expand All @@ -139,6 +135,11 @@ struct GlobalTypeOptimization : public Pass {
// rare).
std::unordered_map<HeapType, std::vector<bool>> canBecomeImmutable;

// Descriptor types that are exposed to JS but do _not_ configure prototypes
// for their described types. We must avoid optimizing these types such that
// they start configuring prototypes.
std::unordered_set<HeapType> exposedNoProtoDescs;

// Maps each field to its new index after field removals. That is, this
// takes into account that fields before this one may have been removed,
// which would then reduce this field's index. If a field itself is removed,
Expand All @@ -149,6 +150,28 @@ struct GlobalTypeOptimization : public Pass {
static const Index RemovedField = Index(-1);
std::unordered_map<HeapType, std::vector<Index>> indexesAfterRemovals;

struct IndexAnalysis {
Index newSize = 0;
bool hasPlaceholder = false;

IndexAnalysis(const std::vector<Index>& indexes) {
Index maxIndex = 0;

@kripken kripken Aug 28, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps add some comments to this class and/or properties? Just reading the code, I'd expect newSize to mean the size after removals - is that right? And the input is the mapping of old indexes to new indexes?

@tlively tlively Aug 28, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, newSize is the size after removals and possible placeholder insertion. And yes, the input is the old to new mapping. I'll add comments.

bool hasKept = false;
bool hasIndexZero = false;
for (auto idx : indexes) {
if (idx != RemovedField) {
hasKept = true;
maxIndex = std::max(maxIndex, idx);
if (idx == 0) {
hasIndexZero = true;
}
}
}
newSize = hasKept ? maxIndex + 1 : 0;
hasPlaceholder = hasKept && !hasIndexZero;

@kripken kripken Aug 28, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems a bit "magical" - worth a comment

}
};

void run(Module* module) override {
if (!module->features.hasGC()) {
return;
Expand All @@ -157,21 +180,32 @@ struct GlobalTypeOptimization : public Pass {
Fatal() << "GTO requires --closed-world";
}

std::unordered_map<Function*, std::vector<Type>> jsExposedTypesByFunction;
jsExposedTypesByFunction[nullptr];
for (auto& func : module->functions) {
jsExposedTypesByFunction[func.get()];
}

// Find and analyze struct operations inside each function.
StructUtils::FunctionStructValuesMap<FieldInfo> functionNewInfos(*module),
functionSetGetInfos(*module);
FieldInfoScanner scanner(functionNewInfos, functionSetGetInfos);
FieldInfoScanner scanner(
functionNewInfos, functionSetGetInfos, jsExposedTypesByFunction);
scanner.run(getPassRunner(), module);
scanner.runOnModuleCode(getPassRunner(), module);

// Combine the data from the functions.
functionSetGetInfos.combineInto(combinedSetGetInfos);
std::vector<Type> jsExposedTypes;
for (auto& [_, types] : jsExposedTypesByFunction) {
jsExposedTypes.insert(jsExposedTypes.end(), types.begin(), types.end());
}

SubTypes subTypes(*module);

// Analyze the JS interface to find fields holding configured prototypes
// that cannot be removed.
analyzeJSInterface(*module, subTypes);
analyzeJSInterface(*module, subTypes, jsExposedTypes);

// Propagate information to super and subtypes on set/get infos:
//
Expand Down Expand Up @@ -291,15 +325,16 @@ struct GlobalTypeOptimization : public Pass {
}

// We need to compute the new set of indexes if we are removing fields, or
// if our parent removed fields. In the latter case, our parent may have
// reordered fields even if we ourselves are not removing anything, and we
// must update to match the parent's order.
// if our parent removed fields, or if we might need a placeholder. If we
// have a parent, it may have reordered fields even if we ourselves are
// not removing anything, and we must update to match the parent's order.
auto super = type.getDeclaredSuperType();
auto superHasUpdates = super && indexesAfterRemovals.contains(*super);
if (!removableIndexes.empty() || superHasUpdates) {
// We are removing fields. Reorder them to allow that, as in the general
// case we can only remove fields from the end, so that if our subtypes
// still need the fields they can append them. For example:
bool isExposedNoProto = exposedNoProtoDescs.contains(type);
if (!removableIndexes.empty() || superHasUpdates || isExposedNoProto) {
// We might be removing fields. Reorder them to allow that, as in the
// general case we can only remove fields from the end, so that if our
// subtypes still need the fields they can append them. For example:

@kripken kripken Aug 28, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please comment about the importance of isExposedNoProto to the placeholder issue

//
// type A = { x: i32, y: f64 };
// type B : A = { x: 132, y: f64, z: v128 };
Expand Down Expand Up @@ -392,6 +427,37 @@ struct GlobalTypeOptimization : public Pass {
}
}

// If the type has no supertype (or its supertype has no fields), check
// if its first field becomes prototype-exposing. If so, add a
// placeholder at index 0 and shift all computed indices.
if (isExposedNoProto && (!super || super->getStruct().fields.empty())) {
// Find the field that will become field 0.
Index i = 0;
for (; i < fields.size(); ++i) {
if (indexesAfterRemoval[i] == 0) {
break;
}
}
// Check whether that field would expose a prototype.
if (i < fields.size()) {
Field optimizedField = fields[i];
if (auto it = canBecomeImmutable.find(type);
it != canBecomeImmutable.end() && i < it->second.size() &&
it->second[i]) {
optimizedField.mutable_ = Immutable;

@kripken kripken Aug 28, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do we apply immutability here? That is, why does this duplicate the normal code that turns fields immutable?

@tlively tlively Aug 28, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We need to check if the normal optimization would produce an immutable externref in the first field without our placeholder intervention. If we didn't check what normal optimization would do with immutability here, then we would either end up adding unnecessary placeholders when the first field would become a mutable externref or end up missing necessary placeholders when the first field is optimized from mutable to immutable.

@kripken kripken Aug 28, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can't we do this in that normal code, then?

I mean that it seems odd to have two places in the code that turn things immutable.

}
if (JSUtils::isPossibleJSPrototypeField(optimizedField)) {
// The field exposes a prototype. Increment all field indices to
// make room for a placeholder first field.
for (auto& idx : indexesAfterRemoval) {
if (idx != RemovedField) {
++idx;
}

@kripken kripken Aug 28, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add a comment that we do not add the i8 here, we just make room for it, and that room - the missing index 0 - is the marker we use to identify the need to add the i8 later (is that right?)

}
}
}
}

// Only store the new indexes we computed if we found something
// interesting. We might not, if e.g. our parent removes fields and we
// add them back in the exact order we started with. In such cases,
Expand All @@ -416,7 +482,9 @@ struct GlobalTypeOptimization : public Pass {
}
}

void analyzeJSInterface(Module& wasm, const SubTypes& subTypes) {
void analyzeJSInterface(Module& wasm,
const SubTypes& subTypes,
const std::vector<Type>& jsExposedTypes) {
if (!wasm.features.hasCustomDescriptors()) {
return;
}
Expand All @@ -426,10 +494,16 @@ struct GlobalTypeOptimization : public Pass {
// Mark the relevant prototype field as read and return true iff we newly
// know we have to propagate the exposure to subtypes.
auto noteExposed = [&](HeapType type, Exactness exact = Inexact) -> bool {
if (auto desc = type.getDescriptorType();
desc && JSUtils::hasPossibleJSPrototypeField(*desc)) {
// This field holds a JS-visible prototype. Do not remove it.
combinedSetGetInfos[std::make_pair(*desc, exact)][0].noteRead();
if (auto desc = type.getDescriptorType()) {
if (JSUtils::hasPossibleJSPrototypeField(*desc)) {
// This descriptor configures a JS-visible prototype. Do not remove
// it.
combinedSetGetInfos[std::make_pair(*desc, exact)][0].noteRead();
} else {
// This descriptor does _not_ configure a JS prototype. Do not add
// one.
exposedNoProtoDescs.insert(*desc);
}
}
if (exact == Inexact) {
return subtypesExposed.insert(type).second;
Expand All @@ -449,6 +523,12 @@ struct GlobalTypeOptimization : public Pass {

JSUtils::iterJSInterface(wasm, flowIn, flowOut);

for (auto type : jsExposedTypes) {
if (type.isRef()) {
noteExposed(type.getHeapType(), type.getExactness());
}
}

// Any type that is a subtype of an exposed type is also exposed. Propagate
// from supertypes to subtypes.
std::vector<HeapType> work(subtypesExposed.begin(), subtypesExposed.end());
Expand All @@ -471,6 +551,19 @@ struct GlobalTypeOptimization : public Pass {
}
}
}

// Also propagate exposed descriptors to supertypes so that descriptor
// hierarchies have consistent layouts. Do not propagate to supertypes that
// actually expose a prototype.

@kripken kripken Aug 28, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. Why didn't we need this code before?
  2. Why do we not propagate as per the second line here?

@tlively tlively Aug 28, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We didn't need this code before because exposedNoProtoDescs is a new set that is used just to help determine when to add placeholders. I guess the comment should say "propagate lack of exposed descriptors."

The case where we stop propagating is the edge case where a subtype has nullexternref and the supertype has externref as its immutable first field. We do not want to add a placeholder to the supertype in that case because it actually does expose a prototype. I'll add a comment about that to the code.

for (auto type : subTypes.types) {
if (exposedNoProtoDescs.contains(type)) {
auto curr = type.getDeclaredSuperType();
while (curr && !JSUtils::hasPossibleJSPrototypeField(*curr)) {
exposedNoProtoDescs.insert(*curr);
curr = curr->getDeclaredSuperType();
}
}
}
}

void updateTypes(Module& wasm) {
Expand Down Expand Up @@ -499,17 +592,19 @@ struct GlobalTypeOptimization : public Pass {
auto remIter = parent.indexesAfterRemovals.find(oldStructType);
if (remIter != parent.indexesAfterRemovals.end()) {
auto& indexesAfterRemoval = remIter->second;
Index removed = 0;
IndexAnalysis analysis(indexesAfterRemoval);
auto copy = newFields;
for (Index i = 0; i < newFields.size(); i++) {
newFields.resize(analysis.newSize);
if (analysis.hasPlaceholder) {
newFields[0] = Field(Field::i8, Immutable);
}
for (Index i = 0; i < copy.size(); i++) {
auto newIndex = indexesAfterRemoval[i];
if (newIndex != RemovedField) {
assert(newIndex < newFields.size());
newFields[newIndex] = copy[i];
} else {
removed++;
}
}
newFields.resize(newFields.size() - removed);

// Update field names as well. The Type Rewriter cannot do this for
// us, as it does not know which old fields map to which new ones (it
Expand Down Expand Up @@ -595,26 +690,30 @@ struct GlobalTypeOptimization : public Pass {
auto& operands = curr->operands;
assert(indexesAfterRemoval.size() == operands.size());

Index removed = 0;
IndexAnalysis analysis(indexesAfterRemoval);
std::vector<Expression*> old(operands.begin(), operands.end());
for (Index i = 0; i < operands.size(); ++i) {
auto newIndex = indexesAfterRemoval[i];
if (newIndex != RemovedField) {
assert(newIndex < operands.size());
operands[newIndex] = old[i];
} else {
++removed;
if (indexesAfterRemoval[i] == RemovedField) {
if (!func &&
EffectAnalyzer(getPassOptions(), *getModule(), old[i]).trap) {
removedTrappingInits.push_back(old[i]);
}
}
}
if (removed) {
operands.resize(operands.size() - removed);
} else {
// If we didn't remove anything then we must have reordered (or else
// we have done pointless work).
operands.resize(analysis.newSize);
if (analysis.hasPlaceholder) {
operands[0] = Builder(*getModule()).makeConst(Literal(int32_t(0)));

@kripken kripken Aug 28, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
operands[0] = Builder(*getModule()).makeConst(Literal(int32_t(0)));
// What we put in the i8 placeholder does not matter.
operands[0] = Builder(*getModule()).makeConst(Literal(int32_t(0)));

}
for (Index i = 0; i < old.size(); ++i) {
auto newIndex = indexesAfterRemoval[i];
if (newIndex != RemovedField) {
assert(newIndex < operands.size());
operands[newIndex] = old[i];
}
}
if (analysis.newSize == old.size() && !analysis.hasPlaceholder) {
// If we didn't remove or insert anything then we must have reordered
// (or else we have done pointless work).
assert(indexesAfterRemoval !=
makeIdentity(indexesAfterRemoval.size()));
}
Expand Down Expand Up @@ -697,7 +796,7 @@ struct GlobalTypeOptimization : public Pass {
}
auto& indexesAfterRemoval = iter->second;
auto newIndex = indexesAfterRemoval[index];
assert(newIndex < indexesAfterRemoval.size() ||
assert(newIndex < IndexAnalysis(indexesAfterRemoval).newSize ||
newIndex == RemovedField);
return newIndex;
}
Expand Down
Loading
Loading