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

tests: Cover all immediate numbers #584

Merged
merged 1 commit into from
Jan 12, 2023
Merged

Conversation

howjmay
Copy link
Contributor

@howjmay howjmay commented Jan 11, 2023

This PR use X-Macro skill to iterate all the possible immediate numbers for the intrinsics that allow immediate numbers only.

closes #582

@howjmay howjmay marked this pull request as draft January 11, 2023 13:51
@howjmay
Copy link
Contributor Author

howjmay commented Jan 11, 2023

@jserv Do you think this PR is a good idea? I think even though this PR covers all the possible immediate numbers, the implementation become less readable and harder to implement.
However, in the meantime, I don't think the tests will be ones that changed frequently, or even won't be changed at all in the future.

tests/impl.cpp Outdated Show resolved Hide resolved
@jserv
Copy link
Member

jserv commented Jan 11, 2023

@jserv Do you think this PR is a good idea? I think even though this PR covers all the possible immediate numbers, the implementation become less readable and harder to implement. However, in the meantime, I don't think the tests will be ones that changed frequently, or even won't be changed at all in the future.

Alternatively, we can even generate test cases via shell scripts with more flexibility.

@howjmay
Copy link
Contributor Author

howjmay commented Jan 11, 2023

@jserv Do you think this PR is a good idea? I think even though this PR covers all the possible immediate numbers, the implementation become less readable and harder to implement. However, in the meantime, I don't think the tests will be ones that changed frequently, or even won't be changed at all in the future.

Alternatively, we can even generate test cases via shell scripts with more flexibility.

May I ask for more details?

@jserv
Copy link
Member

jserv commented Jan 11, 2023

Alternatively, we can even generate test cases via shell scripts with more flexibility.
May I ask for more details?

Check this: https://github.com/IBM/microprobe/tree/master/targets/riscv/examples
There are various template-based generators.

@howjmay
Copy link
Contributor Author

howjmay commented Jan 11, 2023

Alternatively, we can even generate test cases via shell scripts with more flexibility.
May I ask for more details?

Check this: https://github.com/IBM/microprobe/tree/master/targets/riscv/examples There are various template-based generators.

I didn't see the templates for SSE. I saw only PowerPC and RISCV only. Not sure how can we generate tests by this. I may need more research

@howjmay
Copy link
Contributor Author

howjmay commented Jan 11, 2023

So the current change is an acceptable one or I should discard it?

@jserv jserv marked this pull request as ready for review January 12, 2023 02:18
@jserv jserv changed the title test: Cover all immediate numbers in tests tests: Cover all immediate numbers Jan 12, 2023
@jserv jserv merged commit 5f1ed7f into DLTcollab:master Jan 12, 2023
@howjmay
Copy link
Contributor Author

howjmay commented Jan 12, 2023

I will extend the same implementation to the other instrinsics with immediate numbers

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.

Cover complete immediate numbers in tests
2 participants