Skip to content
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

Fix type propagators truthy test #1980

Open
wants to merge 10 commits into
base: master
Choose a base branch
from
Open

Fix type propagators truthy test #1980

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
a7531fd
Fix type propagators truthy test
kasperl Dec 1, 2023
70a3045
Fix
kasperl Dec 4, 2023
e16ab93
Fix new test
kasperl Dec 4, 2023
f7b2e0f
Add more type propagation tests
kasperl Dec 4, 2023
053dcb5
Add more tests
kasperl Dec 4, 2023
d51ead4
Improve tests
kasperl Dec 4, 2023
c94bcbf
Extend test case
kasperl Dec 5, 2023
88d0532
More testing
kasperl Dec 5, 2023
8cf216e
Merge branch 'master' into fix-demorgan
kasperl Mar 21, 2024
ea38ac5
Merge branch 'fix-demorgan' of github.com:toitlang/toit into fix-demo…
kasperl Mar 21, 2024
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
30 changes: 28 additions & 2 deletions src/compiler/propagation/type_propagator.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1463,14 +1463,40 @@ void MethodTemplate::propagate() {
Program* program = propagator_->program();
if (method_.selector_offset() == program->invoke_bytecode_offset(INVOKE_EQ)) {
ConcreteType null_type = ConcreteType(Smi::value(program->null_class_id()));
ASSERT(!argument(0).is_any()); // Receiver is always a single type.
bool receiver_is_null = argument(0).matches(null_type);
bool argument_is_null = argument(1).matches(null_type);
if (receiver_is_null || argument_is_null) {
stack->push_bool_specific(program, receiver_is_null && argument_is_null);
bool argument_is_any = argument(1).is_any();
if (receiver_is_null) {
// If we know the receiver is null, then we can always compute an
// answer. If the argument is any, we don't know if the result is
// true or false. Otherwise, the result is true if the argument
// is null and false if the argument is non-null.
if (argument_is_any) {
stack->push_bool(program);
} else {
stack->push_bool_specific(program, argument_is_null);
}
ret(propagator_, stack);
delete scope;
return;
}

if (argument_is_null) {
// The receiver isn't null, so if the argument is null we
// know that the result is false.
stack->push_bool_specific(program, false);
ret(propagator_, stack);
delete scope;
return;
} else if (argument_is_any) {
// The receiver isn't null, so unless we know the argument
// cannot be null, we must add both true and false to the
// result but continue analyzing the method.
stack->push_bool(program);
ret(propagator_, stack);
}
Copy link
Member Author

Choose a reason for hiding this comment

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

Suggested change
if (receiver_is_null) {
// If we know the receiver is null, then we can always compute an
// answer. If the argument is any, we don't know if the result is
// true or false. Otherwise, the result is true if the argument
// is null and false if the argument is non-null.
if (argument_is_any) {
stack->push_bool(program);
} else {
stack->push_bool_specific(program, argument_is_null);
}
ret(propagator_, stack);
delete scope;
return;
}
if (argument_is_null) {
// The receiver isn't null, so if the argument is null we
// know that the result is false.
stack->push_bool_specific(program, false);
ret(propagator_, stack);
delete scope;
return;
} else if (argument_is_any) {
// The receiver isn't null, so unless we know the argument
// cannot be null, we must add both true and false to the
// result but continue analyzing the method.
stack->push_bool(program);
ret(propagator_, stack);
}
if (argument_is_any) {
stack->push_bool(program);
ret(propagator_, stack);
if (receiver_is_null) {
delete scope;
return;
}
} else if (receiver_is_null || argument_is_null) {
stack->push_bool_specific(program, receiver_is_null && argument_is_null);
ret(propagator_, stack);
delete scope;
return;
}


for (int i = 0; i < arity(); i++) {
TypeSet argument = stack->get(i);
argument.remove_null(program);
Expand Down
2 changes: 1 addition & 1 deletion src/compiler/propagation/type_set.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ bool TypeSet::can_be_truthy(Program* program) const {
Iterator it(*this, TypeSet::words_per_type(program));
while (it.has_next()) {
unsigned id = it.next();
if (id != null_id || id != false_id) return true;
if (id != null_id && id != false_id) return true;
}
return false;
}
Expand Down
6 changes: 3 additions & 3 deletions tests/type_propagation/gold/deltablue-test.gold
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -772,10 +772,10 @@ ScaleConstraint.recalculate tests/type_propagation/deltablue-test.toit
EqualityConstraint tests/type_propagation/deltablue-test.toit
- argument 0: {EqualityConstraint}
- argument 1: {Strength}
- argument 2: {Null_|Variable}
- argument 2: {Variable}
- argument 3: {Variable}
0[052] - load local, as class, pop 4 - Strength(47 - 48) // {True_}
2[052] - load local, as class, pop 3 - Variable(42 - 43) // {True_|False_}
2[052] - load local, as class, pop 3 - Variable(42 - 43) // {True_}
4[052] - load local, as class, pop 2 - Variable(42 - 43) // {True_}
6[019] - load local 5
7[019] - load local 5
Expand Down Expand Up @@ -1282,7 +1282,7 @@ chain-test tests/type_propagation/deltablue-test.toit
43[032] - load global var lazy G0 // {Strength}
45[000] - load local S6
47[017] - load local 3
48[053] - invoke static EqualityConstraint tests/type_propagation/deltablue-test.toit // [{EqualityConstraint}, {Strength}, {Null_|Variable}, {Variable}] -> {EqualityConstraint}
48[053] - invoke static EqualityConstraint tests/type_propagation/deltablue-test.toit // [{EqualityConstraint}, {Strength}, {Variable}, {Variable}] -> {EqualityConstraint}
51[041] - pop 1
52[015] - load local 1
53[023] - load smi 0
Expand Down
15 changes: 7 additions & 8 deletions tests/type_propagation/gold/deltablue-test.gold-O2
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -719,16 +719,15 @@ ScaleConstraint.recalculate tests/type_propagation/deltablue-test.toit
EqualityConstraint tests/type_propagation/deltablue-test.toit
- argument 0: {EqualityConstraint}
- argument 1: {Strength}
- argument 2: {Null_|Variable}
- argument 2: {Variable}
- argument 3: {Variable}
0[052] - load local, as class, pop 3 - Variable(37 - 38) // {True_|False_}
0[019] - load local 5
1[019] - load local 5
2[019] - load local 5
3[019] - load local 5
4[019] - load local 5
5[019] - load local 5
6[053] - invoke static BinaryConstraint tests/type_propagation/deltablue-test.toit // [{EqualityConstraint}, {Strength}, {Variable}, {Variable}] -> {EqualityConstraint}
9[002] - pop, load local S5
11[088] - return S1 4
4[053] - invoke static BinaryConstraint tests/type_propagation/deltablue-test.toit // [{EqualityConstraint}, {Strength}, {Variable}, {Variable}] -> {EqualityConstraint}
7[002] - pop, load local S5
9[088] - return S1 4

EqualityConstraint.execute tests/type_propagation/deltablue-test.toit
- argument 0: {EqualityConstraint}
Expand Down Expand Up @@ -1211,7 +1210,7 @@ chain-test tests/type_propagation/deltablue-test.toit
39[032] - load global var lazy G0 // {Strength}
41[000] - load local S6
43[017] - load local 3
44[053] - invoke static EqualityConstraint tests/type_propagation/deltablue-test.toit // [{EqualityConstraint}, {Strength}, {Null_|Variable}, {Variable}] -> {EqualityConstraint}
44[053] - invoke static EqualityConstraint tests/type_propagation/deltablue-test.toit // [{EqualityConstraint}, {Strength}, {Variable}, {Variable}] -> {EqualityConstraint}
47[041] - pop 1
48[015] - load local 1
49[023] - load smi 0
Expand Down
4 changes: 2 additions & 2 deletions tests/type_propagation/gold/richards-test.gold
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -688,7 +688,7 @@ WorkerTask.run tests/type_propagation/richards-test.toit
28[013] - store field, pop 1
30[016] - load local 2
31[009] - load field local 20 // [{WorkerTask}] -> {Null_|SmallInteger_}
33[061] - invoke virtual set id // [{Null_|Packet}, {Null_|SmallInteger_}] -> {SmallInteger_}
33[061] - invoke virtual set id // [{Packet}, {Null_|SmallInteger_}] -> {SmallInteger_}
36[002] - pop, load local S2
38[023] - load smi 0
39[061] - invoke virtual set a1 // [{Packet}, {SmallInteger_}] -> {SmallInteger_}
Expand Down Expand Up @@ -745,7 +745,7 @@ HandlerTask.run tests/type_propagation/richards-test.toit
0[016] - load local 2
1[082] - branch if false T36
4[016] - load local 2
5[060] - invoke virtual get kind // [{Null_|Packet}] -> {Null_|SmallInteger_}
5[060] - invoke virtual get kind // [{Packet}] -> {Null_|SmallInteger_}
8[025] - load smi 1
9[062] - invoke eq // [{Null_|SmallInteger_}, {SmallInteger_}] -> {True_|False_}
10[082] - branch if false T26
Expand Down
4 changes: 2 additions & 2 deletions tests/type_propagation/gold/richards-test.gold-O2
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -639,7 +639,7 @@ WorkerTask.run tests/type_propagation/richards-test.toit
26[013] - store field, pop 1
28[016] - load local 2
29[009] - load field local 20 // [{WorkerTask}] -> {Null_|SmallInteger_}
31[061] - invoke virtual set id // [{Null_|Packet}, {Null_|SmallInteger_}] -> {SmallInteger_}
31[061] - invoke virtual set id // [{Packet}, {Null_|SmallInteger_}] -> {SmallInteger_}
34[002] - pop, load local S2
36[023] - load smi 0
37[061] - invoke virtual set a1 // [{Packet}, {SmallInteger_}] -> {SmallInteger_}
Expand Down Expand Up @@ -693,7 +693,7 @@ HandlerTask.run tests/type_propagation/richards-test.toit
0[016] - load local 2
1[082] - branch if false T36
4[016] - load local 2
5[060] - invoke virtual get kind // [{Null_|Packet}] -> {Null_|SmallInteger_}
5[060] - invoke virtual get kind // [{Packet}] -> {Null_|SmallInteger_}
8[025] - load smi 1
9[062] - invoke eq // [{Null_|SmallInteger_}, {SmallInteger_}] -> {True_|False_}
10[082] - branch if false T26
Expand Down
Loading