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

Use the core::arch intrinsics where possible #42

Merged
merged 1 commit into from
Sep 5, 2018

Conversation

GabrielMajeri
Copy link
Contributor

The addition of some intrinsics in core::arch means that we no longer have to use inline assembly in some cases.

I've changed rdtsc's signature to match the one in core::arch & Intel's docs.

Please tell me if there's some other core::arch function which I've missed and we could use it in this crate.

src/lib.rs Show resolved Hide resolved
src/time.rs Outdated Show resolved Hide resolved
@GabrielMajeri
Copy link
Contributor Author

I've opened rust-lang/stdarch#559 to track the issue with _rdtsc's return type. It seems that this is due to a bug in Intel's intrinsics specification. I've reported the issue to Intel.

For now, this PR will use mem::transmute to convert the i64 to u64.

@gz
Copy link
Owner

gz commented Sep 5, 2018

Sounds good, thanks for the pull request.

@gz gz merged commit 42fd623 into gz:master Sep 5, 2018
@GabrielMajeri GabrielMajeri deleted the core-arch branch September 5, 2018 17:17

asm!("rdtsc" : "={eax}" (low), "={edx}" (high));
((high as u64) << 32) | (low as u64)
mem::transmute(_rdtsc())
Copy link
Contributor

Choose a reason for hiding this comment

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

Isn't it much better to use as u64 for this sort of thing?

Copy link
Owner

Choose a reason for hiding this comment

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

It is... It's fixed on master now thanks for raising this concern.

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.

3 participants