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

zeroize: add support for wasm's v128 SIMD register #968

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

brxken128
Copy link
Contributor

This PR adds support for zeroing wasm's v128 register. The module is named after the core::arch module, but it does require that target_family = "wasm" so I'm unsure of which naming scheme to stick with - I'm not too familiar with WASM as a whole to be honest.

I'll leave this as a draft until #967 is ready and merged, as the support for zeroing SIMD registers for ... notice will need updating.

};
}

impl_zeroize_for_simd_register!(v128);
Copy link
Member

Choose a reason for hiding this comment

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

With only one register type, the macro is probably overkill, unless you plan to add more

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I agree, I was just adhering to the other implementations as I wasn't sure which style would be preferrential - bad decision on my part (even if it is just be abstracted away once compiled).

After looking further into it, wasm has only a generic v128 type, and no other SIMD registers available (I believe the internal types are defined here though).

I'll scrap the macro as it's definitely overkill for the single addressible register type, although I do wonder how the compiler even handles a type this generic.

On another note, should there be platform-specific tests for (maybe all?) SIMD registers, so that we can validate that they're correctly zeroed out? I'm not too sure I've seen any within the codebase, but it might be good for additional validation/reassurance as even just this single type does seem rather internally-complex. I wouldn't mind giving it a go and opening a separate PR if I can get them working and ensure their correctness.

The tests might be unfeasable for many register types though, given that some are gated behind specific CPU features/extensions - I tried to add (feature gated) avx512 register support, but that's both far too recent (MSRV of 1.73) and I couldn't even get it working on my 7950x, which has more than enough avx512 support available. I probably spent way too long throwing flags at the compiler there haha

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