Skip to content

[WebAssembly] Add missing tests from #133289 #133938

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

Merged
merged 1 commit into from
Apr 1, 2025
Merged

Conversation

sbc100
Copy link
Collaborator

@sbc100 sbc100 commented Apr 1, 2025

No description provided.

@sbc100 sbc100 requested a review from dschuff April 1, 2025 16:53
@dschuff
Copy link
Member

dschuff commented Apr 1, 2025

I didn't think about this before, but as an alternative to duplicating everything in the other test, we could try just splitting out the libcalls that take or return pointers. That would mostly be the ones that return structs (frexp, modf). The rest should really just be duplicates (even though the test output would be different since the loads and stores to the stack would be different). WDYT?

@sbc100
Copy link
Collaborator Author

sbc100 commented Apr 1, 2025

I didn't think about this before, but as an alternative to duplicating everything in the other test, we could try just splitting out the libcalls that take or return pointers. That would mostly be the ones that return structs (frexp, modf). The rest should really just be duplicates (even though the test output would be different since the loads and stores to the stack would be different). WDYT?

Yes, it looks like all the tests that have loads and stores would need to be split out,. It looks like maybe 1/2 of the tests would need to duplicated?

@sbc100
Copy link
Collaborator Author

sbc100 commented Apr 1, 2025

I tried splitting out the ptr-size-specific tests and it turned out to be everything except @unordf/@unordt/@ordt.

Maybe you can figure out a better way? But maybe we just land this in the interim?

@dschuff
Copy link
Member

dschuff commented Apr 1, 2025

Oh right, the ones that return fp128 would have to be split out too. So that's more than half, maybe not worth it then.

@dschuff
Copy link
Member

dschuff commented Apr 1, 2025

SO this LGTM, but can you maybe add a comment to the top of this file and libcalls.ll that they should be kept in sync?

@sbc100 sbc100 merged commit a30caa6 into llvm:main Apr 1, 2025
9 of 12 checks passed
@sbc100 sbc100 deleted the frexpf branch April 1, 2025 17:47
Ankur-0429 pushed a commit to Ankur-0429/llvm-project that referenced this pull request Apr 2, 2025
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.

2 participants