Skip to content
Merged
Show file tree
Hide file tree
Changes from 12 commits
Commits
Show all changes
146 commits
Select commit Hold shift + click to select a range
51cb0b9
start
kripken Sep 26, 2022
6dc7292
format
kripken Sep 26, 2022
ec0b3d6
builds
kripken Sep 26, 2022
617690a
src/
kripken Sep 26, 2022
f750d67
fix
kripken Sep 26, 2022
a34e470
typo
kripken Sep 26, 2022
afe8b5d
clarify comment
kripken Sep 27, 2022
3af1107
works
kripken Sep 27, 2022
c7c6b93
rename
kripken Sep 27, 2022
fa7a116
moar + fix one
kripken Sep 27, 2022
7927fbe
fix
kripken Sep 27, 2022
e504b0b
fix
kripken Sep 27, 2022
9a057dc
progress
kripken Sep 27, 2022
932da84
comment
kripken Sep 28, 2022
e164411
Merge remote-tracking branch 'origin/main' into cone
kripken Sep 28, 2022
f50a8d4
start
kripken Sep 28, 2022
973586d
format
kripken Sep 28, 2022
8261dc0
test
kripken Sep 28, 2022
650ce4e
hash index too
kripken Sep 28, 2022
c0aa7e4
considered best
kripken Sep 28, 2022
3a1dbfd
Merge remote-tracking branch 'origin/gufa.has' into cone
kripken Sep 28, 2022
85f31a5
test
kripken Sep 28, 2022
81bcf3e
Merge remote-tracking branch 'origin/main' into cone
kripken Sep 29, 2022
43da163
test
kripken Sep 29, 2022
dee3da0
test
kripken Sep 29, 2022
c6bc347
test
kripken Sep 29, 2022
47fffc4
test
kripken Sep 29, 2022
1773856
test
kripken Sep 29, 2022
af84e99
test
kripken Sep 29, 2022
36ebd4c
test
kripken Sep 29, 2022
2b1bfc7
test
kripken Sep 29, 2022
8da6585
test
kripken Sep 29, 2022
fbf99a9
test
kripken Sep 29, 2022
5b577e6
test
kripken Sep 29, 2022
5b9eaba
test
kripken Sep 29, 2022
ad3aefa
test
kripken Sep 29, 2022
126fd22
test
kripken Sep 29, 2022
c435d5a
test
kripken Sep 29, 2022
6624aa3
test
kripken Sep 29, 2022
ec5ccc2
test
kripken Sep 29, 2022
35b43af
test
kripken Sep 29, 2022
a78b734
test
kripken Sep 29, 2022
eda95c9
test
kripken Sep 29, 2022
cfd79c6
test
kripken Sep 29, 2022
6769447
test
kripken Sep 29, 2022
2443410
test
kripken Sep 29, 2022
932b1e7
format
kripken Sep 29, 2022
1765c4f
format
kripken Sep 29, 2022
bbeb87d
Merge remote-tracking branch 'origin/main' into cone
kripken Sep 29, 2022
e855951
progress
kripken Sep 29, 2022
48d99a0
test
kripken Sep 29, 2022
9e742f1
test
kripken Sep 29, 2022
5df1d38
test
kripken Sep 29, 2022
a61239d
progress
kripken Sep 29, 2022
ab75451
test
kripken Sep 29, 2022
7e7d36b
test
kripken Sep 29, 2022
4f2ad9e
test
kripken Sep 29, 2022
416b586
test
kripken Sep 29, 2022
1bcab96
test
kripken Sep 29, 2022
433f4be
test
kripken Sep 29, 2022
680041c
test
kripken Sep 29, 2022
5af82bb
test
kripken Sep 29, 2022
e511255
test
kripken Sep 29, 2022
fdc9930
test
kripken Sep 29, 2022
c9383c7
test
kripken Sep 29, 2022
4680ecd
test
kripken Sep 29, 2022
b1f5fb9
test
kripken Sep 29, 2022
a68dbfe
test
kripken Sep 29, 2022
3cff9cb
format
kripken Sep 29, 2022
fd277a3
test
kripken Sep 29, 2022
9b8e7b6
test
kripken Sep 29, 2022
4662f54
test
kripken Sep 29, 2022
4b50ecc
test
kripken Sep 29, 2022
d4f0e7f
test
kripken Sep 29, 2022
883cad8
test
kripken Sep 29, 2022
953548e
test
kripken Sep 29, 2022
282b678
format
kripken Sep 29, 2022
9a77fed
work
kripken Sep 29, 2022
1217eeb
todo
kripken Sep 30, 2022
aa74ce9
work
kripken Sep 30, 2022
aea36da
fix
kripken Sep 30, 2022
ad825a7
format
kripken Sep 30, 2022
9af078c
test
kripken Sep 30, 2022
bfd3509
test
kripken Sep 30, 2022
8ca04e4
test
kripken Sep 30, 2022
ce84508
test
kripken Sep 30, 2022
44baef0
test
kripken Sep 30, 2022
cb68fe1
test
kripken Sep 30, 2022
0e3c2b2
test
kripken Sep 30, 2022
0fc4e13
test
kripken Sep 30, 2022
54fa5cc
test
kripken Sep 30, 2022
446c2de
work
kripken Sep 30, 2022
c27187e
format
kripken Sep 30, 2022
e830086
format
kripken Sep 30, 2022
6a3a208
builds
kripken Sep 30, 2022
d7152dd
fix
kripken Sep 30, 2022
dc5e63a
format
kripken Sep 30, 2022
643b539
work
kripken Sep 30, 2022
3d4b099
work
kripken Sep 30, 2022
5fc0d71
work
kripken Sep 30, 2022
f5454be
work
kripken Sep 30, 2022
64b7136
test
kripken Sep 30, 2022
3ef1a32
test
kripken Sep 30, 2022
07762db
work
kripken Sep 30, 2022
6fd8c96
work
kripken Sep 30, 2022
f6806ad
test
kripken Sep 30, 2022
18c7348
test
kripken Sep 30, 2022
2bc7826
test
kripken Sep 30, 2022
0a20071
test
kripken Sep 30, 2022
03eb8c5
test
kripken Sep 30, 2022
e46a096
fix
kripken Sep 30, 2022
0dc1cb1
work
kripken Sep 30, 2022
552e2e8
work
kripken Sep 30, 2022
97cd211
work
kripken Sep 30, 2022
3b611f9
format
kripken Sep 30, 2022
d792619
test
kripken Sep 30, 2022
f3bcdc7
fix
kripken Sep 30, 2022
ef917a2
comment
kripken Sep 30, 2022
cbfffd0
Merge remote-tracking branch 'origin/fix' into cone
kripken Sep 30, 2022
781f043
Merge remote-tracking branch 'origin/main' into cone
kripken Oct 3, 2022
3cf100d
move
kripken Oct 3, 2022
5808d74
comment
kripken Oct 3, 2022
6102dca
simpler
kripken Oct 3, 2022
8618c0a
comments
kripken Oct 3, 2022
a2269de
fix
kripken Oct 3, 2022
824c864
todo
kripken Oct 3, 2022
2290fd2
test
kripken Oct 3, 2022
c898267
typo
kripken Oct 3, 2022
d24fc0b
fix
kripken Oct 3, 2022
9999985
Revert "fix":wq
kripken Oct 3, 2022
9737075
rename
kripken Oct 5, 2022
1f272ab
cleanup
kripken Oct 5, 2022
b280cc1
fix typo
kripken Oct 6, 2022
6f182f8
Merge remote-tracking branch 'origin/main' into cone
kripken Oct 7, 2022
85e2ac3
null lub handling after recent null changes
kripken Oct 7, 2022
415a8ea
test update
kripken Oct 7, 2022
05ea12a
fix
kripken Oct 7, 2022
d87fc2b
fix
kripken Oct 7, 2022
78002dd
clenup
kripken Oct 7, 2022
fd520f8
clenup
kripken Oct 7, 2022
2e0e687
fix comment
kripken Oct 7, 2022
6926cb1
Update src/ir/possible-contents.cpp
kripken Oct 7, 2022
9cfdc3f
Merge remote-tracking branch 'origin/cone' into cone
kripken Oct 7, 2022
9117223
comment
kripken Oct 7, 2022
83d720a
comment
kripken Oct 7, 2022
a1245ad
Merge remote-tracking branch 'origin/main' into cone
kripken Oct 11, 2022
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
118 changes: 59 additions & 59 deletions src/ir/possible-contents.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,15 @@ void PossibleContents::combine(const PossibleContents& other) {
return;
}

auto lub = Type::getLeastUpperBound(type, otherType);
if (lub == Type::none) {
// The types are not in the same hierarchy.
value = Many();
return;
}

// From here we can assume there is a useful LUB.

// Nulls can be combined in by just adding nullability to a type.
if (isNull() || other.isNull()) {
// Only one of them can be null here, since we already handled the case
Expand All @@ -110,43 +119,35 @@ void PossibleContents::combine(const PossibleContents& other) {
}
}

// Before we give up and return Many, try to find a ConeType that describes
// both inputs.
auto lub = Type::getLeastUpperBound(type, otherType);
if (lub != Type::none) {
// We found a shared ancestor. Next we need to find how big a cone we need:
// the cone must be big enough to contain both the inputs.
auto depth = getCone().depth;
auto otherDepth = other.getCone().depth;
Index newDepth;
if (depth == FullDepth || otherDepth == FullDepth) {
// At least one has full (infinite) depth, so we know the new depth must
// be the same.
newDepth = FullDepth;
} else {
// The depth we need under the lub is how far from the lub we are, plus
// the depth of our cone.
// TODO: we could make a single loop that also does the LUB, at the same
// time, and also avoids calling getDepth() which loops once more?
auto depthFromRoot = type.getHeapType().getDepth();
auto otherDepthFromRoot = otherType.getHeapType().getDepth();
auto lubDepthFromRoot = lub.getHeapType().getDepth();
assert(lubDepthFromRoot <= depthFromRoot);
assert(lubDepthFromRoot <= otherDepthFromRoot);
Index depthUnderLub = depthFromRoot - lubDepthFromRoot + depth;
Index otherDepthUnderLub =
otherDepthFromRoot - lubDepthFromRoot + otherDepth;

// The total cone must be big enough to contain all the above.
newDepth = std::max(depthUnderLub, otherDepthUnderLub);
}

value = ConeType{lub, newDepth};
return;
}

// Nothing else possible combines in an interesting way; emit a Many.
value = Many();
// Find a ConeType that describes both inputs, using the shared ancestor which
// is the LUB. We need to find how big a cone we need: the cone must be big
// enough to contain both the inputs.
auto depth = getCone().depth;
auto otherDepth = other.getCone().depth;
Index newDepth;
if (depth == FullDepth || otherDepth == FullDepth) {
// At least one has full (infinite) depth, so we know the new depth must
// be the same.
newDepth = FullDepth;
} else {
// The depth we need under the lub is how far from the lub we are, plus
// the depth of our cone.
// TODO: we could make a single loop that also does the LUB, at the same
// time, and also avoids calling getDepth() which loops once more?
auto depthFromRoot = type.getHeapType().getDepth();
auto otherDepthFromRoot = otherType.getHeapType().getDepth();
auto lubDepthFromRoot = lub.getHeapType().getDepth();
assert(lubDepthFromRoot <= depthFromRoot);
assert(lubDepthFromRoot <= otherDepthFromRoot);
Index depthUnderLub = depthFromRoot - lubDepthFromRoot + depth;
Index otherDepthUnderLub =
otherDepthFromRoot - lubDepthFromRoot + otherDepth;

// The total cone must be big enough to contain all the above.
newDepth = std::max(depthUnderLub, otherDepthUnderLub);
}

value = ConeType{lub, newDepth};
}

void PossibleContents::intersectWithFullCone(const PossibleContents& other) {
Expand All @@ -166,6 +167,9 @@ void PossibleContents::intersectWithFullCone(const PossibleContents& other) {
return;
}

// There is an intersection here. Note that this implies |this| is a reference
// type, as it has an intersection with |other| which is a full cone type
// (which must be a reference type).
auto type = getType();
auto otherType = other.getType();
auto heapType = type.getHeapType();
Copy link
Member

Choose a reason for hiding this comment

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

The case of None and Many are handled above, but can this be a Literal or Global? In that case, what it doesn't have a heap type?

Copy link
Member Author

Choose a reason for hiding this comment

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

This does look unclear, I'll add a comment. The reason is the intersection with another reference type is not empty, so this must be a reference type itself.

Expand All @@ -190,12 +194,12 @@ void PossibleContents::intersectWithFullCone(const PossibleContents& other) {
return;
}

// If the heap types are not compatible then the intersection is either
// nothing or a null.
// If the heap types are not compatible then they are in separate hierarchies
// and there is no intersection.
auto isSubType = HeapType::isSubType(heapType, otherHeapType);
auto otherIsSubType = HeapType::isSubType(otherHeapType, heapType);
if (!isSubType && !otherIsSubType) {
setNoneOrNull();
value = None();
return;
}

Expand All @@ -213,10 +217,7 @@ void PossibleContents::intersectWithFullCone(const PossibleContents& other) {
// refactoring.
}
Copy link
Member

Choose a reason for hiding this comment

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

What about other.isLiteral() || other.isGlobal()? I guess those don't matter because we only call this with cone types? It would be good to add a TODO here. Alternatively, we could give this whole function a more specific name like intersectWithCone.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, atm we just have other being a cone type. I can rename the method that way. If we expand it later we can always rename it back.

Copy link
Member

Choose a reason for hiding this comment

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

That would be helpful, thanks!


// An interesting non-empty intersection that is a new cone which differs from
// both the original ones. (This must be an intersection of cones, since by
// assumption |other| is a cone, and another cone is the only shape that can
// have a non-empty intersection with it that differs from them both.)
// Intersect the cones, as there is no more specific information we can use.
auto depthFromRoot = heapType.getDepth();
auto otherDepthFromRoot = otherHeapType.getDepth();

Expand Down Expand Up @@ -249,8 +250,8 @@ void PossibleContents::intersectWithFullCone(const PossibleContents& other) {
// heapType ..
// \
*/
// E.g. if |this| is a cone of size 10, and |otherHeapType| is an immediate
// subtype of |this|, then the new cone must be of size 9.
// E.g. if |this| is a cone of depth 10, and |otherHeapType| is an immediate
// subtype of |this|, then the new cone must be of depth 9.
auto newDepth = getCone().depth;
if (newHeapType == otherHeapType) {
assert(depthFromRoot <= otherDepthFromRoot);
Expand Down Expand Up @@ -292,26 +293,21 @@ bool PossibleContents::haveIntersection(const PossibleContents& a,

// From here on we focus on references.

if (aType.isNullable() && bType.isNullable()) {
// Null is possible on both sides. Assume that an intersection can exist,
// but we could be more precise here and check if the types belong to
// different hierarchies, in which case the nulls would differ TODO. For
// now we only use this API from the RefEq logic, so this is fully precise.
auto aHeapType = aType.getHeapType();
auto bHeapType = bType.getHeapType();

if (aType.isNullable() && bType.isNullable() &&
aHeapType.getBottom() == bHeapType.getBottom()) {
// A compatible null is possible on both sides.
return true;
}

// We ruled out a null on both sides, so at least one is non-nullable. If the
// other is a null then no chance for an intersection remains.
// We ruled out having a compatible null on both sides. If one is simply a
// null then no chance for an intersection remains.
if (a.isNull() || b.isNull()) {
return false;
}

// From here on we focus on references and can ignore the case of null - any
// intersection must be of a non-null value, so we can focus on the heap
// types.
auto aHeapType = aType.getHeapType();
auto bHeapType = bType.getHeapType();

auto aSubB = HeapType::isSubType(aHeapType, bHeapType);
auto bSubA = HeapType::isSubType(bHeapType, aHeapType);
if (!aSubB && !bSubA) {
Expand All @@ -320,6 +316,10 @@ bool PossibleContents::haveIntersection(const PossibleContents& a,
return false;
}

// From here on we focus on references and can ignore the case of null - any
// intersection must be of a non-null value, so we can focus on the heap
// types.

auto aDepthFromRoot = aHeapType.getDepth();
auto bDepthFromRoot = bHeapType.getDepth();

Expand Down
3 changes: 2 additions & 1 deletion src/passes/GUFA.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -244,7 +244,8 @@ struct GUFAOptimizer
if (refType.isRef()) {
// We have some knowledge of the type here. Use that to optimize: RefTest
// returns 1 if the input is of a subtype of the intended type, that is,
// we are looking for a type in that cone of types.
// we are looking for a type in that cone of types. (Note that we use a
// non-nullable cone since only a non-null can pass the test.)
auto intendedContents =
PossibleContents::fullConeType(Type(curr->intendedType, NonNullable));
Copy link
Member

Choose a reason for hiding this comment

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

Would be helpful to have a short comment on why this is NonNullable (i.e., otherwise it can trap)

Copy link
Member Author

Choose a reason for hiding this comment

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

Added.


Expand Down
36 changes: 27 additions & 9 deletions test/gtest/possible-contents.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -284,6 +284,13 @@ TEST_F(PossibleContentsTest, TestOracleMinimal) {
void assertHaveIntersection(PossibleContents a, PossibleContents b) {
EXPECT_TRUE(PossibleContents::haveIntersection(a, b));
EXPECT_TRUE(PossibleContents::haveIntersection(b, a));
#if BINARYEN_TEST_DEBUG
if (!PossibleContents::haveIntersection(a, b) ||
!PossibleContents::haveIntersection(b, a)) {
std::cout << "\nFailure: no intersection:\n" << a << '\n' << b << '\n';
abort();
}
#endif
Comment on lines +287 to +293
Copy link
Member

Choose a reason for hiding this comment

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

Why do we need this? If this is the case, doesn't this crash in EXPECT_TRUE above anyway?

Copy link
Member Author

Choose a reason for hiding this comment

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

EXPECT_TRUE doesn't print out the values being operated on, so we just get "expected true on line 286" and that's it. We don't even know which call to this function caused the problem. The extra logging makes it easy to debug these.

}
void assertLackIntersection(PossibleContents a, PossibleContents b) {
EXPECT_FALSE(PossibleContents::haveIntersection(a, b));
Expand All @@ -304,10 +311,12 @@ TEST_F(PossibleContentsTest, TestIntersection) {
assertLackIntersection(exactI32, exactAnyref);
assertLackIntersection(i32Zero, exactAnyref);

// But nullable ones can - the null can be the intersection.
assertHaveIntersection(exactFuncSignatureType, exactAnyref);
// But nullable ones can - the null can be the intersection, if they are not
// in separate hierarchies.
assertHaveIntersection(exactFuncSignatureType, funcNull);
assertHaveIntersection(anyNull, funcNull);

assertLackIntersection(exactFuncSignatureType, exactAnyref);
assertLackIntersection(anyNull, funcNull);

// Identical types might.
assertHaveIntersection(exactI32, exactI32);
Expand All @@ -326,12 +335,8 @@ TEST_F(PossibleContentsTest, TestIntersection) {
assertHaveIntersection(funcGlobal, exactNonNullFuncSignatureType);
assertHaveIntersection(nonNullFuncGlobal, exactNonNullFuncSignatureType);

// Neither is a subtype of the other, but nulls are possible, so a null can be
// the intersection.
assertHaveIntersection(funcGlobal, anyGlobal);

// Without null on one side, we cannot intersect.
assertLackIntersection(nonNullFuncGlobal, anyGlobal);
// Separate hierarchies.
assertLackIntersection(funcGlobal, anyGlobal);
}

TEST_F(PossibleContentsTest, TestIntersectWithCombinations) {
Expand Down Expand Up @@ -662,6 +667,15 @@ TEST_F(PossibleContentsTest, TestStructCones) {
assertLackIntersection(PossibleContents::coneType(nnA, 1),
PossibleContents::coneType(nnD, 100));

// Neither is a subtype of the other, but nulls are possible, so a null can be
// the intersection.
assertHaveIntersection(PossibleContents::fullConeType(nullA),
PossibleContents::fullConeType(nullE));

// Without null on one side, we cannot intersect.
assertLackIntersection(PossibleContents::fullConeType(nnA),
PossibleContents::fullConeType(nullE));

// Computing intersections is supported with a full cone type.
assertIntersection(none, PossibleContents::fullConeType(nnA), none);
assertIntersection(many,
Expand Down Expand Up @@ -729,6 +743,10 @@ TEST_F(PossibleContentsTest, TestStructCones) {
PossibleContents::fullConeType(signature),
PossibleContents::fullConeType(signature));

// Incompatible hierarchies have no intersection.
assertIntersection(
literalNullA, PossibleContents::fullConeType(funcref), none);

// Subcontents. This API only supports the case where one of the inputs is a
// full cone type.
// First, compare exact types to such a cone.
Expand Down
4 changes: 2 additions & 2 deletions test/lit/passes/gufa-refs.wast
Original file line number Diff line number Diff line change
Expand Up @@ -2615,11 +2615,11 @@
;; CHECK: (func $test-cones (type $i32_=>_none) (param $x i32)
;; CHECK-NEXT: (drop
;; CHECK-NEXT: (ref.cast_static $struct
;; CHECK-NEXT: (select (result anyref)
;; CHECK-NEXT: (select (result (ref null $struct))
;; CHECK-NEXT: (struct.new $struct
;; CHECK-NEXT: (i32.const 0)
;; CHECK-NEXT: )
;; CHECK-NEXT: (ref.null any)
;; CHECK-NEXT: (ref.null none)
;; CHECK-NEXT: (local.get $x)
;; CHECK-NEXT: )
;; CHECK-NEXT: )
Expand Down