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

Imlement rdrand and rdseed intrinsics #325

Closed
newpavlov opened this issue Feb 18, 2018 · 10 comments
Closed

Imlement rdrand and rdseed intrinsics #325

newpavlov opened this issue Feb 18, 2018 · 10 comments

Comments

@newpavlov
Copy link
Contributor

newpavlov commented Feb 18, 2018

  • _rdrand16_step
  • _rdrand32_step
  • _rdrand64_step
  • _rdseed16_step
  • _rdseed32_step
  • _rdseed64_step

It will require addition of two target features rdrand and rdseed to the language.

cc @nagisa

@newpavlov
Copy link
Contributor Author

I don't quite get how to link to the LLVM intrinsic llvm.x86.rdrand.64, IUUC correctly it has two return values i64 and i32. All options which I've tried result in "Intrinsic has incorrect return type!". Can someone help with it?

@nagisa
Copy link
Member

nagisa commented Feb 19, 2018 via email

@nagisa
Copy link
Member

nagisa commented Feb 19, 2018 via email

@alexcrichton
Copy link
Member

@newpavlov I think it's ok to use a asm! block temporarily for this, but yeah as @nagisa mentioned we'll need to add a dedicated intrinsic to rustc for this otherwise to get the ABI to work.

@nagisa
Copy link
Member

nagisa commented Feb 20, 2018 via email

@newpavlov
Copy link
Contributor Author

newpavlov commented Feb 20, 2018

@alexcrichton
Rust already supports rdrnd and rdseed features. But rdrnd is a LLVM feature name, while with pclmulqdq it was decided to use vendor name. What do you think about renaming this feature to rdrand for consistency? Will this change be backwards compatible? We can proabably support both rdrand and rdrnd for the time being, and deprecate rdrnd in the next epoch. This questions was earlier raised by @nagisa, but it was left unanswered.

@nagisa
Copy link
Member

nagisa commented Feb 20, 2018

The change wouldn’t be backwards compatible, but since anything related to it is unstable in the first place, I do not see any reason to do the epoch dance.

Manishearth added a commit to Manishearth/rust that referenced this issue Feb 25, 2018
Rename rdrnd target feature to rdrand

Plus minor cleanup.

Related stdsimd [issue](rust-lang/stdarch#325).
kennytm added a commit to kennytm/rust that referenced this issue Feb 25, 2018
Rename rdrnd target feature to rdrand

Plus minor cleanup.

Related stdsimd [issue](rust-lang/stdarch#325).
@newpavlov
Copy link
Contributor Author

newpavlov commented Apr 5, 2018

@alexcrichton
Do we have to follow Intel intrinsics 1-to-1? It feels weird to have signature _rdrand64_step(val: &mut u64) -> i32 more natural for Rust will be _rdrand64_step() -> (u64, bool) or even _rdrand64_step() -> Option<u64>, plus it's closer to how instruction works. At least we could go with LLVM variant _rdrand64_step() -> (u64, i32).

@gnzlbg
Copy link
Contributor

gnzlbg commented Apr 5, 2018

Do we have to follow Intel intrinsics 1-to-1?

Yep :/

It feels weird to have signature _rdrand64_step(val: &mut u64) -> i32 more natural for Rust will be _rdrand64_step() -> (u64, bool) or even _rdrand64_step() -> Option, plus it's closer to how instruction works. At least we could go with LLVM variant _rdrand64_step() -> (u64, i32).

You can build those in a wrapper outside stdsimd.

@gnzlbg
Copy link
Contributor

gnzlbg commented Apr 5, 2018

@newpavlov FWIW I have such a wrapper here: https://github.com/gnzlbg/typed_arch

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

No branches or pull requests

4 participants