Skip to content

Cast to bool by comparing to zero #343

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

Jump to bottom
Merged
merged 1 commit into from
Nov 23, 2018
Merged

Cast to bool by comparing to zero #343

merged 1 commit into from
Nov 23, 2018

Conversation

dcodeIO
Copy link
Member

@dcodeIO dcodeIO commented Nov 23, 2018

See: #342

@dcodeIO dcodeIO requested a review from MaxGraey November 23, 2018 13:24
dcodeIO
dcodeIO commented Nov 23, 2018
View reviewed changes
src/program.ts
// TODO: this is inefficient because it has to read a string
let global = assert(this.currentFunction.program.elementsLookup.get(assert(getGetGlobalName(expr))));
assert(global.kind == ElementKind.GLOBAL);
return canConversionOverflow(assert((<Global>global).type), type);
Copy link
Member Author

@dcodeIO dcodeIO Nov 23, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

Not related to the casting itself, more an unrelated fix for reading from an i32 global before converting to an integer smaller than the global's type. Surfaced in the bool test.

@MaxGraey
Copy link
Member

btw, why binaryen use i32.ne/i64.ne with zero constant instead just one opcode i32.eqz/i64.eqz?

@dcodeIO
Copy link
Member Author

dcodeIO commented Nov 23, 2018

eqz checks for "== 0" while we need "!= 0". When optimizing, Binaryen converts to eqz if the surrounding code allows it, i.e. double negate.

@MaxGraey
Copy link
Member

I mean cases when we have i32.ne + i32.eqz which is actually (x != 0) == 0 and could be simplify to x == 0 (i32.eqz). Like this

@dcodeIO
Copy link
Member Author

dcodeIO commented Nov 23, 2018 •
edited
Loading

This seems to be from untouched output, which is expected (especially in combination with assert which is particularly odd because it usually involves a condition that is then tested for != 0). Should be better once optimized.

@MaxGraey
Copy link
Member

I see. It seems binaryen even better optimize != 0 then & 1

@dcodeIO dcodeIO merged commit c30c62e into master Nov 23, 2018
@dcodeIO dcodeIO mentioned this pull request Nov 23, 2018
Closed
willemneal pushed a commit to willemneal/assemblyscript that referenced this pull request Dec 26, 2018
@dcodeIO
Cast to bool by comparing to zero (AssemblyScript#343)
5897dbf
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@MaxGraey MaxGraey MaxGraey approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@dcodeIO @MaxGraey