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.

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

aot: disable musttail for mips #2457

Merged
merged 1 commit into from
Aug 15, 2023
Merged

Conversation

yamt
Copy link
Collaborator

@yamt yamt commented Aug 14, 2023

While i'm not familiar with mips abi, it's reported that it (at least sometimes) doesn't work.
This fixes #2412

While i'm not familiar with mips abi, it's reported that
it (at least sometimes) doesn't work.
This fixes bytecodealliance#2412
@Zzzabiyaka
Copy link
Contributor

Zzzabiyaka commented Aug 14, 2023

I'm not familiar with the context so the questions can be silly enough:

  • Did we have must tail in 1.2.2 release?
  • If so, are we sure no performance regression can happen because of disabling it

By the way, we have MIPS env so we can help with testing this PR if it's needed

@yamt
Copy link
Collaborator Author

yamt commented Aug 14, 2023

I'm not familiar with the context so the questions can be silly enough:

* Did we have must tail in 1.2.2 release?

* If so, are we sure no performance regression can happen because of disabling it

stack check logic has been moved to a separate function for better estimation.
where available, musttail is used by a stack-check function to call the corresponding user function.

in 1.2.2, stack check logic were sprinkled in user functions inline.
musttail was not used. (as there was no function call.)

i expect some performance penalty, comparing to 1.2.2.
cf. #2244 (comment)

By the way, we have MIPS env so we can help with testing this PR if it's needed

it's welcome.

@wenyongh
Copy link
Contributor

@eloparco @yamt Does this PR fix issue #2412? Shall I merge this PR?

@yamt
Copy link
Collaborator Author

yamt commented Aug 15, 2023

@eloparco @yamt Does this PR fix issue #2412? Shall I merge this PR?

yes. at least this fixes the reported aot compilation error.

@wenyongh wenyongh merged commit 0f18051 into bytecodealliance:main Aug 15, 2023
368 checks passed
@wenyongh
Copy link
Contributor

@eloparco @yamt Does this PR fix issue #2412? Shall I merge this PR?

yes. at least this fixes the reported aot compilation error.

OK, let's merge it.

Zzzabiyaka pushed a commit to Zzzabiyaka/wasm-micro-runtime that referenced this pull request Aug 15, 2023
Zzzabiyaka pushed a commit to Zzzabiyaka/wasm-micro-runtime that referenced this pull request Aug 15, 2023
victoryang00 pushed a commit to victoryang00/wamr-aot-gc-checkpoint-restore that referenced this pull request May 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

LLVM ERROR: failed to perform tail call elimination with MIPS AOT
3 participants