-
Notifications
You must be signed in to change notification settings - Fork 815
[Custom Descriptors] Optimize away unneeded descriptors in GlobalTypeOptimization #7872
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.
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
Merged
Merged
Changes from all commits
Commits
Show all changes
58 commits
Select commit
Hold shift + click to select a range
56d5165
work
kripken 440016f
work
kripken bdf1045
work
kripken 1e24c1d
work
kripken 1e958b5
work
kripken c3ee741
work
kripken 6086dc9
prep
kripken 7ac2730
work
kripken b826d63
work
kripken 427297f
work
kripken a2c4914
work
kripken 991a129
work
kripken b457ed2
work
kripken 5b25a96
work
kripken b91aef6
work
kripken a5697f3
work
kripken 48c66ca
work
kripken e002c61
work
kripken 5be8c21
work
kripken 33e9e87
work
kripken 3ed905a
work
kripken 7b205cc
work
kripken b62a6ec
work
kripken 98aa913
work
kripken c6f45fe
work
kripken 4f05e6f
work
kripken 1e4d63f
work
kripken 757e50b
work
kripken 42f00fc
work
kripken 9075189
format
kripken cf29d90
work
kripken 4116501
work
kripken 62ef969
work
kripken bd5d8fc
test
kripken 7a51d78
fix
kripken fed87f1
work
kripken a1d7068
wip
kripken 0a4dc43
work
kripken 90f1fb5
work
kripken 6b722ef
work
kripken 9e1409a
work
kripken c774441
work
kripken 08cc60b
feedback: comments
kripken a490a8e
feedback: remove parallel sets
kripken 88340c3
feedback: test field names
kripken 88cc7dc
feedback: comment
kripken 57f120c
tlively's idea?
kripken 8a4286a
Revert "tlively's idea?"
kripken 567ecbd
Optimize siblings
kripken 2950719
fix nested global effects
kripken 03ff3e5
fix
kripken 55e7065
fix casts+test
kripken d406990
test
kripken b240e30
Update test/lit/passes/gto-desc.wast
kripken d0ec85a
Do not propagate a descriptor to a super without one
kripken 276a188
Merge remote-tracking branch 'myself/gto.desc' into gto.desc
kripken 46634b3
TODO
kripken eb266ee
Update src/ir/struct-utils.h
kripken File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -44,6 +44,16 @@ template<typename T> struct StructValues : public std::vector<T> { | |
assert(index < this->size()); | ||
return std::vector<T>::operator[](index); | ||
} | ||
|
||
// Store the descriptor as another field. (This could be a std::optional to | ||
// indicate that the descriptor's existence depends on the type, but that | ||
// would add overhead & code clutter (type checks). If there is no descriptor, | ||
// this will just hang around with the default values, not harming anything | ||
// except perhaps for looking a little odd during debugging. And whenever we | ||
// combine() a non-existent descriptor, we are doing unneeded work, but the | ||
// data here is typically just a few bools, so it is simpler and likely | ||
// faster to just copy those rather than check if the type has a descriptor.) | ||
T desc; | ||
}; | ||
|
||
// Maps heap types to a StructValues for that heap type. | ||
|
@@ -69,6 +79,7 @@ struct StructValuesMap : public std::unordered_map<HeapType, StructValues<T>> { | |
for (Index i = 0; i < info.size(); i++) { | ||
combinedInfos[type][i].combine(info[i]); | ||
} | ||
combinedInfos[type].desc.combine(info.desc); | ||
} | ||
} | ||
|
||
|
@@ -80,6 +91,8 @@ struct StructValuesMap : public std::unordered_map<HeapType, StructValues<T>> { | |
x.dump(o); | ||
o << " "; | ||
}; | ||
o << " desc: "; | ||
vec.desc.dump(o); | ||
o << '\n'; | ||
} | ||
} | ||
|
@@ -129,14 +142,17 @@ struct FunctionStructValuesMap | |
// | ||
// void noteCopy(HeapType type, Index index, T& info); | ||
// | ||
// * Note a read | ||
// * Note a read. | ||
// | ||
// void noteRead(HeapType type, Index index, T& info); | ||
// | ||
// We track information from struct.new and struct.set/struct.get separately, | ||
// because in struct.new we know more about the type - we know the actual exact | ||
// type being written to, and not just that it is of a subtype of the | ||
// instruction's type, which helps later. | ||
// | ||
// Descriptors are treated as fields in that we call the above functions on | ||
// them. We pass DescriptorIndex for their index as a fake value. | ||
template<typename T, typename SubType> | ||
struct StructScanner | ||
: public WalkerPass<PostWalker<StructScanner<T, SubType>>> { | ||
|
@@ -146,6 +162,8 @@ struct StructScanner | |
|
||
SubType& self() { return *static_cast<SubType*>(this); } | ||
|
||
static const Index DescriptorIndex = -1; | ||
|
||
StructScanner(FunctionStructValuesMap<T>& functionNewInfos, | ||
FunctionStructValuesMap<T>& functionSetGetInfos) | ||
: functionNewInfos(functionNewInfos), | ||
|
@@ -168,6 +186,10 @@ struct StructScanner | |
noteExpressionOrCopy(curr->operands[i], heapType, i, infos[i]); | ||
} | ||
} | ||
|
||
if (curr->desc) { | ||
self().noteExpression(curr->desc, heapType, DescriptorIndex, infos.desc); | ||
} | ||
} | ||
|
||
void visitStructSet(StructSet* curr) { | ||
|
@@ -236,6 +258,42 @@ struct StructScanner | |
noteExpressionOrCopy(curr->replacement, heapType, index, info); | ||
} | ||
|
||
void visitRefCast(RefCast* curr) { | ||
if (curr->desc) { | ||
// We may try to read a descriptor from anything arriving in |curr->ref|, | ||
// but the only things that matter are the things we cast to: other types | ||
// can lack a descriptor, and are skipped anyhow. So the only effective | ||
// read is of the cast type. | ||
handleDescRead(curr->getCastType()); | ||
} | ||
} | ||
|
||
void visitBrOn(BrOn* curr) { | ||
if (curr->desc && | ||
(curr->op == BrOnCastDesc || curr->op == BrOnCastDescFail)) { | ||
handleDescRead(curr->getCastType()); | ||
} | ||
} | ||
|
||
void visitRefGetDesc(RefGetDesc* curr) { | ||
// Unlike a cast, anything in |curr->ref| may be read from. | ||
handleDescRead(curr->ref->type); | ||
} | ||
|
||
void handleDescRead(Type type) { | ||
if (type == Type::unreachable) { | ||
return; | ||
} | ||
auto heapType = type.getHeapType(); | ||
if (heapType.isStruct()) { | ||
// Any subtype of the reference here may be read from. | ||
self().noteRead(heapType, | ||
DescriptorIndex, | ||
functionSetGetInfos[this->getFunction()][heapType].desc); | ||
return; | ||
} | ||
} | ||
|
||
void | ||
noteExpressionOrCopy(Expression* expr, HeapType type, Index index, T& info) { | ||
// Look at the value falling through, if it has the exact same type | ||
|
@@ -268,8 +326,8 @@ struct StructScanner | |
FunctionStructValuesMap<T>& functionSetGetInfos; | ||
}; | ||
|
||
// Helper class to propagate information about fields to sub- and/or super- | ||
// classes in the type hierarchy. While propagating it calls a method | ||
// Helper class to propagate information to sub- and/or super- classes in the | ||
// type hierarchy. While propagating it calls a method | ||
// | ||
// to.combine(from) | ||
// | ||
|
@@ -286,18 +344,32 @@ template<typename T> class TypeHierarchyPropagator { | |
|
||
SubTypes subTypes; | ||
|
||
// Propagate given a StructValuesMap, which means we need to take into | ||
// account fields. | ||
void propagateToSuperTypes(StructValuesMap<T>& infos) { | ||
propagate(infos, false, true); | ||
} | ||
|
||
void propagateToSubTypes(StructValuesMap<T>& infos) { | ||
propagate(infos, true, false); | ||
} | ||
|
||
void propagateToSuperAndSubTypes(StructValuesMap<T>& infos) { | ||
propagate(infos, true, true); | ||
} | ||
|
||
// Propagate on a simpler map of structs and infos (that is, not using | ||
// separate values for the fields, as StructValuesMap does). This is useful | ||
// when not tracking individual fields, but something more general about | ||
// types. | ||
using StructMap = std::unordered_map<HeapType, T>; | ||
|
||
void propagateToSuperTypes(StructMap& infos) { | ||
propagate(infos, false, true); | ||
} | ||
void propagateToSubTypes(StructMap& infos) { propagate(infos, true, false); } | ||
void propagateToSuperAndSubTypes(StructMap& infos) { | ||
propagate(infos, true, true); | ||
} | ||
|
||
private: | ||
void propagate(StructValuesMap<T>& combinedInfos, | ||
bool toSubTypes, | ||
|
@@ -320,6 +392,11 @@ template<typename T> class TypeHierarchyPropagator { | |
work.push(*superType); | ||
} | ||
} | ||
// Propagate the descriptor to the super, if the super has one. | ||
if (superType->getDescriptorType() && | ||
superInfos.desc.combine(infos.desc)) { | ||
work.push(*superType); | ||
} | ||
} | ||
} | ||
|
||
|
@@ -333,6 +410,41 @@ template<typename T> class TypeHierarchyPropagator { | |
work.push(subType); | ||
} | ||
} | ||
// Propagate the descriptor. | ||
if (subInfos.desc.combine(infos.desc)) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. And for both supertypes and subtypes we can probably skip combining descriptor values if the original type does not have a descriptor. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. (as above) |
||
work.push(subType); | ||
} | ||
} | ||
} | ||
} | ||
} | ||
|
||
void propagate(StructMap& combinedInfos, bool toSubTypes, bool toSuperTypes) { | ||
UniqueDeferredQueue<HeapType> work; | ||
for (auto& [type, _] : combinedInfos) { | ||
work.push(type); | ||
} | ||
while (!work.empty()) { | ||
auto type = work.pop(); | ||
auto& info = combinedInfos[type]; | ||
|
||
if (toSuperTypes) { | ||
// Propagate to the supertype. | ||
if (auto superType = type.getDeclaredSuperType()) { | ||
auto& superInfo = combinedInfos[*superType]; | ||
if (superInfo.combine(info)) { | ||
work.push(*superType); | ||
} | ||
} | ||
} | ||
|
||
if (toSubTypes) { | ||
// Propagate shared fields to the subtypes. | ||
for (auto subType : subTypes.getImmediateSubTypes(type)) { | ||
auto& subInfo = combinedInfos[subType]; | ||
if (subInfo.combine(info)) { | ||
work.push(subType); | ||
} | ||
} | ||
} | ||
} | ||
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We no longer combine when the descriptor does not exist.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
TypeHierarchyPropagator
doesn't do so below, that's correct, butStructValuesMap
still does.