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

add BMI2 instruction pdep #328

Merged
merged 1 commit into from
Jan 27, 2023
Merged

add BMI2 instruction pdep #328

merged 1 commit into from
Jan 27, 2023

Conversation

kcning
Copy link
Contributor

@kcning kcning commented Jan 13, 2023

No description provided.

@@ -621,6 +622,10 @@ Definition x86_PEXT sz (v1 v2: word sz): ex_tpl (w_ty sz) :=
let _ := check_size_32_64 sz in
ok (@pextr sz v1 v2).

Definition x86_PDEP sz (v1 v2: word sz): ex_tpl (w_ty sz) :=
let _ := check_size_32_64 sz in
ok (@pextr sz v1 v2).
Copy link
Member

Choose a reason for hiding this comment

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

This line looks wrong.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, your are using the semantic of pext to define the semantic of pdep.
Do you need help to be able to properly define the semantic of pdep in Coq and Easycrypt?

Copy link
Contributor Author

@kcning kcning Jan 19, 2023

Choose a reason for hiding this comment

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

Yes, some help would be much appreciated. I've been discussing with @sarranz about it and was told that the semantic isn't correct. Supporting it however doesn't seem trivial in his opinion: https://www.felixcloutier.com/x86/pdep

Copy link
Contributor

Choose a reason for hiding this comment

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

I think it is now ready to be merged.
Review is welcome

Copy link
Contributor Author

Choose a reason for hiding this comment

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

thank you!

@vbgl
Copy link
Member

vbgl commented Jan 20, 2023

Why is there an “-r” at the end of the names of the *pdepr functions?

@vbgl
Copy link
Member

vbgl commented Jan 23, 2023

Please also add a note to the CHANGELOG.

@bgregoir
Copy link
Contributor

@kcning can you add a note in the CHANGELOG as required by Vincent ?

@vbgl
Copy link
Member

vbgl commented Jan 24, 2023

And also please remove the trailing -rs if they are here by mistake.

@vbgl vbgl self-assigned this Jan 25, 2023
Co-authored-by: Benjamin Grēgoire <Benjamin.Gregoire@inria.fr>
@vbgl
Copy link
Member

vbgl commented Jan 27, 2023

I’ve removed the trailing -rs.

CI: https://gitlab.com/jasmin-lang/jasmin/-/pipelines/759329323

@vbgl vbgl merged commit 2b59cd5 into jasmin-lang:main Jan 27, 2023
@vbgl
Copy link
Member

vbgl commented Jan 27, 2023

Together with Antoine, we’ve tested all day the Coq semantics and are pretty sure that it not too wrong.

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.

None yet

3 participants