This is an archive of the discontinued LLVM Phabricator instance.

[X86] Make Feature64Bit useful
ClosedPublic

Authored by craig.topper on Aug 24 2018, 12:35 PM.

Details

Summary

We now only add +64bit to the CPU string for "generic" CPU. All other CPU names are assumed to have the feature flag already set if they support 64-bit. I've remove the implies from CMPXCHG8 so that Feature64Bit only comes in via CPUs or user passing -mattr=+64bit.

I've changed the assert to a report_fatal_error so it's not lost in Release builds.

The test updates are to fix things that tripped error now.

Diff Detail

Event Timeline

craig.topper created this revision.Aug 24 2018, 12:35 PM

We could use the feature bit to try to catch invalid CPU specifications (like trying to use -mcpu=pentium on a 64-bit target).

craig.topper retitled this revision from [X86] Remove Feature64Bit to [X86] Make Feature64Bit useful.
craig.topper edited the summary of this revision. (Show Details)

Updated cpus.ll test to also check for this new error. Had to add a function body to get it to create the X86Subtarget correctly to make the check and had to fix some of the old RUN lines to not pass a 64-bit triple with a 32-bit cpu when they expect no error.

Forgot to drop an InstCombine patch from my tree before uploading that last patch.

This revision is now accepted and ready to land.Aug 29 2018, 2:20 PM
This revision was automatically updated to reflect the committed changes.
anna added a subscriber: anna.Sep 24 2018, 6:21 AM

With this change, we now break the code gen in KVMs that do not have the "+64bit" feature tagged (although it is capable of generating 64 bit code). Is there a way to identify the feature for KVMs?

This is the cpuinfo for the KVM where code gen was failing with the Fatal Error: 64-bit code requested on a subtarget that doesn't support it!

cat /proc/cpuinfo:
> processor       : 0
> vendor_id       : AuthenticAMD
> cpu family      : 6
> model           : 13
> model name      : QEMU Virtual CPU version 1.5.3
> stepping        : 3
> microcode       : 0x1000065
> cpu MHz         : 2599.998
> cache size      : 512 KB
> physical id     : 0
> siblings        : 1
> core id         : 0
> cpu cores       : 1
> apicid          : 0
> initial apicid  : 0
> fpu             : yes
> fpu_exception   : yes
> cpuid level     : 4
> wp              : yes
> flags           : fpu de pse tsc msr pae mce cx8 apic sep mtrr pge mca cmov pat pse36 clflush mmx fxsr sse sse2 syscall nx lm nopl pni cx16 hypervisor lahf_lm abm sse4a 3dnowprefetch vmmcall
> bugs            : fxsave_leak sysret_ss_attrs
> bogomips        : 5199.99
> TLB size        : 1024 4K pages
> cache_alignment : 64
> address sizes   : 48 bits physical, 48 bits virtual
> power management:

@anna, the "lm" in the flags from the cpuinfo should be the 64bit flag. Is this failing with clang, or llc, or some other program that uses llvm libraries?

anna added a comment.Sep 24 2018, 11:27 AM

@anna, the "lm" in the flags from the cpuinfo should be the 64bit flag. Is this failing with clang, or llc, or some other program that uses llvm libraries?

Interesting. It's failing with our Azul JIT (Falcon) which uses the llvm libraries.
We populate the features using llvm's sys::getHostCPUFeatures and set the mcpu and mattr in the EngineBuilder.
Note that if I manually add "+64bit" in the feature population (after all features retrieved by getHostCPUFeatures have been added into the mattr), there is no code gen failure on the KVM.

We may not be inferring +64bit in getHostCPUFeatures at first glance. I'll check more thoroughly.

@anna, hopefully I fixed your issue in r342914

anna added a comment.Sep 24 2018, 5:28 PM

@anna, hopefully I fixed your issue in r342914

great, thanks @craig.topper! It fixed my issue.