Skip to content

Add unions in decoded instruction and operands #326

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 3 commits into from
Apr 21, 2022

Conversation

athre0z
Copy link
Member

@athre0z athre0z commented Apr 20, 2022

This PR adds anonymous unions to the operand struct and the vector-extension structs within the raw struct, reducing the struct size quite substantially. For the operands, I also moved the type field immediately in front of the union, which should allow us to represent it as a safe union in the Rust bindings without need for calling an accessor.

Anonymous unions are a C11 feature, so I had to bump the "required compiler standard" in the process. Because MSVC doesn't properly support C11, we instead rely on the non-standard extension here, silencing the corresponding warning.

@athre0z athre0z added C-enhancement Category: Enhancement of existing features A-decoder Area: Decoder P-medium Priority: Medium A-build Area: Build system labels Apr 20, 2022
@athre0z athre0z force-pushed the refactor/unions-in-public-api branch 2 times, most recently from 42309dc to abb2f2e Compare April 21, 2022 10:33
@athre0z athre0z marked this pull request as ready for review April 21, 2022 10:42
@athre0z athre0z requested a review from flobernd April 21, 2022 10:42
Copy link
Member

@flobernd flobernd left a comment

Choose a reason for hiding this comment

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

LGTM!

athre0z added 2 commits April 21, 2022 17:31
Because Windows still has trouble with compiling C11 code 11 years after the
standard was released, we instead rely on the non-standard extension of
anonymous unions being supported by MSVC since basically forever. Doing so
produces a warning in kernel builds, so we need to shut that up.
@athre0z athre0z force-pushed the refactor/unions-in-public-api branch from abb2f2e to 6474ac7 Compare April 21, 2022 15:31
@athre0z athre0z merged commit 0cf3c96 into master Apr 21, 2022
@athre0z athre0z deleted the refactor/unions-in-public-api branch April 21, 2022 15:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-build Area: Build system A-decoder Area: Decoder C-enhancement Category: Enhancement of existing features P-medium Priority: Medium
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants