search

shepmaster / cupid

Get information about the x86 and x86_64 processor
MIT License
34 stars 9 forks source link

Use `cfg-if` to ensure it compiles (and tests) on all platforms. #1

Closed huonw closed 9 years ago

huonw commented 9 years ago

It's useless on non-x86 ones, but at least it successfully compiles.

huonw commented 9 years ago

On a somewhat related note, compiling for a 32-bit platform gives:

src/lib.rs:30:41: 30:51 warning: literal out of range for isize, #[warn(overflowing_literals)] on by default
src/lib.rs:30     ExtendedFunctionInformation       = 0x80000000,
                                                      ^~~~~~~~~~
src/lib.rs:31:41: 31:51 warning: literal out of range for isize, #[warn(overflowing_literals)] on by default
src/lib.rs:31     ExtendedProcessorSignature        = 0x80000001,
                                                      ^~~~~~~~~~
src/lib.rs:32:41: 32:51 warning: literal out of range for isize, #[warn(overflowing_literals)] on by default
src/lib.rs:32     BrandString1                      = 0x80000002,
                                                      ^~~~~~~~~~
src/lib.rs:33:41: 33:51 warning: literal out of range for isize, #[warn(overflowing_literals)] on by default
src/lib.rs:33     BrandString2                      = 0x80000003,
                                                      ^~~~~~~~~~
src/lib.rs:34:41: 34:51 warning: literal out of range for isize, #[warn(overflowing_literals)] on by default
src/lib.rs:34     BrandString3                      = 0x80000004,
                                                      ^~~~~~~~~~
src/lib.rs:36:41: 36:51 warning: literal out of range for isize, #[warn(overflowing_literals)] on by default
src/lib.rs:36     CacheLine                         = 0x80000006,
                                                      ^~~~~~~~~~
src/lib.rs:37:41: 37:51 warning: literal out of range for isize, #[warn(overflowing_literals)] on by default
src/lib.rs:37     TimeStampCounter                  = 0x80000007,
                                                      ^~~~~~~~~~
src/lib.rs:38:41: 38:51 warning: literal out of range for isize, #[warn(overflowing_literals)] on by default
src/lib.rs:38     PhysicalAddressSize               = 0x80000008,
                                                      ^~~~~~~~~~
src/lib.rs:30:41: 30:51 warning: literal out of range for isize, #[warn(overflowing_literals)] on by default
src/lib.rs:30     ExtendedFunctionInformation       = 0x80000000,
                                                      ^~~~~~~~~~
src/lib.rs:31:41: 31:51 warning: literal out of range for isize, #[warn(overflowing_literals)] on by default
src/lib.rs:31     ExtendedProcessorSignature        = 0x80000001,
                                                      ^~~~~~~~~~
src/lib.rs:32:41: 32:51 warning: literal out of range for isize, #[warn(overflowing_literals)] on by default
src/lib.rs:32     BrandString1                      = 0x80000002,
                                                      ^~~~~~~~~~
src/lib.rs:33:41: 33:51 warning: literal out of range for isize, #[warn(overflowing_literals)] on by default
src/lib.rs:33     BrandString2                      = 0x80000003,
                                                      ^~~~~~~~~~
src/lib.rs:34:41: 34:51 warning: literal out of range for isize, #[warn(overflowing_literals)] on by default
src/lib.rs:34     BrandString3                      = 0x80000004,
                                                      ^~~~~~~~~~
src/lib.rs:36:41: 36:51 warning: literal out of range for isize, #[warn(overflowing_literals)] on by default
src/lib.rs:36     CacheLine                         = 0x80000006,
                                                      ^~~~~~~~~~
src/lib.rs:37:41: 37:51 warning: literal out of range for isize, #[warn(overflowing_literals)] on by default
src/lib.rs:37     TimeStampCounter                  = 0x80000007,
                                                      ^~~~~~~~~~
src/lib.rs:38:41: 38:51 warning: literal out of range for isize, #[warn(overflowing_literals)] on by default
src/lib.rs:38     PhysicalAddressSize               = 0x80000008,
                                                      ^~~~~~~~~~

which presumably aren't so great? Maybe it needs a #[repr(u32)]? (I'm unsure of the details of this instruction.)

shepmaster commented 9 years ago

Maybe it needs a #[repr(u32)]

Yeah, probably. An isize is definitely not what I wanted...

It's useless on non-x86 ones

This is absolutely true, but I'm missing why the existing cfg attributes are not sufficient. Shouldn't compiling on non-x86 simply select the version that always returns None?

huonw commented 9 years ago

The asm! in cpuid is x86-specific, e.g. the cpuid mnemoic is meaningless (there's no machine instruction for it), and the register constraints only make sense on x86 too.

huonw commented 9 years ago

Incidentally, compiling for arm-unknown-linux-gnueabihf before this PR gives an LLVM assertion:

rustc: /home/huon/rust2/src/llvm/include/llvm/CodeGen/FunctionLoweringInfo.h:169: unsigned int llvm::FunctionLoweringInfo::InitializeRegForValue(const llvm::Value*): Assertion `R == 0 && "Already initialized this value register!"' failed.
Build failed, waiting for other jobs to finish...
rustc: /home/huon/rust2/src/llvm/include/llvm/CodeGen/FunctionLoweringInfo.h:169: unsigned int llvm::FunctionLoweringInfo::InitializeRegForValue(const llvm::Value*): Assertion `R == 0 && "Already initialized this value register!"' failed.
Could not compile `cupid`.

(Rebased.)

shepmaster commented 9 years ago

Ah, I see. The problem this fixes really has to do with the fact that the inline assembly is still being compiled on a non x86 platform, which causes issues as you note. cfg-if isn't really giving much more functionality than the previous attribute guards, it's putting the cpuid function in the guards that helps. I'm still a bit meh at a whole crate for what seems like a simple and hing, but it is a simple crate at least!

Thanks!

shepmaster commented 9 years ago

For my own education, how are you set up to compile to ARM? Having some knowledge like that in my back pocket could come in useful!

huonw commented 9 years ago

The problem this fixes really has to do with the fact that the inline assembly is still being compiled on a non x86 platform, which causes issues as you note. cfg-if isn't really giving much more functionality than the previous attribute guards, it's putting the cpuid function in the guards that helps

Oh, yeah, cfg-if is just to avoid having to write the guard any(target_arch = ...) 4 times. Sorry for the confusion!

For my own education, how are you set up to compile to ARM? Having some knowledge like that in my back pocket could come in useful!

I'm building Rust after ./configure --target=arm-unknown-linux-gnueabihf, which I believe requires installing a toolchain (I just installed various packages via apt like binutils-arm-linux-gnueabihf and gcc-4.9-arm-linux-gnueabihf not sure of exactly which are required and which are pointless). I then use multirust's --link-local to point to my compilation directories, and manually copy a cargo binary into it.

It might be easier if one installs it properly, but I'm actively working on cross-architecture stuff (SIMD) so working out of the build directory makes most sense for me. (Having nicer cross-compilation is also an immediate goal of a few of the Moz employees on Rust, so hopefully this will quickly become nicer...)