Skip to content

Fix branching instructions in the interpreter#3062

Merged
abrown merged 8 commits intobytecodealliance:mainfrom
afonso360:interpreter-branches
Jul 19, 2021
Merged

Fix branching instructions in the interpreter#3062
abrown merged 8 commits intobytecodealliance:mainfrom
afonso360:interpreter-branches

Conversation

@afonso360
Copy link
Copy Markdown
Contributor

Hey, branches were causing the interpreter to crash.

@github-actions github-actions bot added the cranelift Issues related to the Cranelift code generator label Jul 4, 2021
@afonso360 afonso360 force-pushed the interpreter-branches branch from 15ac5ff to 771c1ab Compare July 4, 2021 21:38
@abrown
Copy link
Copy Markdown
Member

abrown commented Jul 6, 2021

@afonso360, what do you make of this failure?

@afonso360
Copy link
Copy Markdown
Contributor Author

Hey, I think this is a bug in the aarch64 backend. I'll try to have a look at this today if possible.

@github-actions github-actions bot added the cranelift:area:aarch64 Issues related to AArch64 backend. label Jul 6, 2021
@afonso360 afonso360 force-pushed the interpreter-branches branch 3 times, most recently from 11890d2 to 490be9b Compare July 6, 2021 22:15
Copy link
Copy Markdown
Member

@abrown abrown left a comment

Choose a reason for hiding this comment

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

LGTM but let's wait for a review on the aarch64 part and some small fixes noted below.

@afonso360 afonso360 force-pushed the interpreter-branches branch from 4fe7212 to e991271 Compare July 8, 2021 20:03
@afonso360 afonso360 requested review from abrown and cfallin July 9, 2021 21:04
Copy link
Copy Markdown
Member

@abrown abrown left a comment

Choose a reason for hiding this comment

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

All my comments are resolved; I think this should be merged if we get a 👍 from @cfallin or @akirilov-arm on the aarch64 side.

@afonso360 afonso360 force-pushed the interpreter-branches branch from e991271 to 074de80 Compare July 13, 2021 09:55
@afonso360 afonso360 force-pushed the interpreter-branches branch 2 times, most recently from 08b44b2 to bd3ad19 Compare July 15, 2021 23:28
@afonso360 afonso360 force-pushed the interpreter-branches branch from bd3ad19 to bf551ba Compare July 16, 2021 10:57
@afonso360 afonso360 requested a review from akirilov-arm July 16, 2021 11:00
Copy link
Copy Markdown
Contributor

@akirilov-arm akirilov-arm left a comment

Choose a reason for hiding this comment

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

The AArch64-related changes look good to me; there are a few nits that you can ignore.

@afonso360 afonso360 force-pushed the interpreter-branches branch from bf551ba to b04bcfd Compare July 16, 2021 18:14
@abrown abrown merged commit 3a38400 into bytecodealliance:main Jul 19, 2021
@afonso360 afonso360 deleted the interpreter-branches branch September 2, 2021 13:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cranelift:area:aarch64 Issues related to AArch64 backend. cranelift Issues related to the Cranelift code generator

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants