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

src/itx.rs: Deduplicate w/ generics #683

Merged
merged 6 commits into from
Mar 20, 2024
Merged

Conversation

thedataking
Copy link
Collaborator

@thedataking thedataking commented Jan 17, 2024

First commit emits debug info for assembly on debug builds. This aids testing and debugging. Closes #25.

NOTE to self: need to update the macro docs in bitdepths.rs if the overall approach looks good.

@thedataking thedataking changed the title src/itx.rs: Deduplicate w/generics src/itx.rs: Deduplicate w/ generics Jan 17, 2024
@thedataking thedataking force-pushed the perl/deduplicate-mod-itx branch 2 times, most recently from 6ab7b27 to 5d2ea22 Compare January 17, 2024 05:31
@thedataking thedataking force-pushed the perl/deduplicate-mod-itx branch 3 times, most recently from 5bbdc09 to 150dacb Compare March 15, 2024 02:56
This was referenced Mar 15, 2024
@thedataking thedataking force-pushed the perl/deduplicate-mod-itx branch 5 times, most recently from 4da4ab3 to a52dcd4 Compare March 19, 2024 03:38
@thedataking thedataking marked this pull request as ready for review March 19, 2024 06:58
Copy link
Collaborator

@kkysen kkysen left a comment

Choose a reason for hiding this comment

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

I haven't thoroughly reviewed the macros and initialization logic (comparing them to C), as you said you looked at that pretty thoroughly already. I can if you think you could've missed something, though. Besides that, I had a few other small comments. Also, the debug info improvements are very nice.

include/common/bitdepth.rs Outdated Show resolved Hide resolved
include/common/bitdepth.rs Outdated Show resolved Hide resolved
src/itx.rs Outdated Show resolved Hide resolved
src/itx.rs Outdated Show resolved Hide resolved
Copy link
Collaborator

@randomPoison randomPoison left a comment

Choose a reason for hiding this comment

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

The translation of the C macros looks correct to me. Is there a reason you didn't setup macros for assigning the C fallback functions though? I'm assuming it's correct because what we should have should be the direct result of expanding the macros in C and should have been the same between the 8bpc and 16bpc files, so if you copied it out from one of them then it should be correct here. But it's hard to verify without having the macros setup the same way.

src/itx.rs Outdated Show resolved Hide resolved
@kkysen
Copy link
Collaborator

kkysen commented Mar 20, 2024

We can later change the fallbacks to return Self, which will check that we initializedeverything. So I'm less worried about them vs the asm ones.

@thedataking
Copy link
Collaborator Author

Is there a reason you didn't setup macros for assigning the C fallback functions though?

Derp, I forgot to push that change; sorry about that .

Copy link
Collaborator

@randomPoison randomPoison left a comment

Choose a reason for hiding this comment

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

Updated macros for initializing the fallback fns look good too 👍 Thanks for adding those.

@thedataking thedataking merged commit 5256bf0 into main Mar 20, 2024
36 checks passed
@thedataking thedataking deleted the perl/deduplicate-mod-itx branch March 20, 2024 22:46
thedataking added a commit that referenced this pull request Mar 21, 2024
This reverts commit 18469a1 because
blocking issue #683 has now been resolved.
thedataking added a commit that referenced this pull request Mar 22, 2024
This reverts commit 18469a1 because
blocking issue #683 has now been resolved.
thedataking added a commit that referenced this pull request Mar 22, 2024
This reverts commit 18469a1 because
blocking issue #683 has now been resolved.
@kkysen kkysen mentioned this pull request Mar 22, 2024
folkertdev pushed a commit that referenced this pull request Mar 25, 2024
This reverts commit 18469a1 because
blocking issue #683 has now been resolved.
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.

Fix duplicated _tmpl.rs files
3 participants