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

Update ARM feature and CPU detection (supersedes #36464) #36485

Merged
merged 2 commits into from
Jul 9, 2020

Conversation

IanButterworth
Copy link
Member

I messed up #36464 with an accidental blank force push, so that PR can't be reopened. This is the replacement

Updates ARM feature and CPU detection

cc. @yuyichao

@IanButterworth
Copy link
Member Author

I applied these changes on-top of #35318 (without rebase) and still get generic for this graviton2 machine:

   _       _ _(_)_     |  Documentation: https://docs.julialang.org
  (_)     | (_) (_)    |
   _ _   _| |_  __ _   |  Type "?" for help, "]?" for Pkg help.
  | | | | | | |/ _` |  |
  | | |_| | | | (_| |  |  Version 1.5.0-DEV.545 (2020-03-31)
 _/ |\__'_|_|_|\__'_|  |  vc/upgrade_llvm_10/8ae92cc5f0* (fork: 1 commits, 91 days)
|__/                   |

julia> versioninfo()
Julia Version 1.5.0-DEV.545
Commit 8ae92cc5f0* (2020-03-31 01:16 UTC)
Platform Info:
  OS: Linux (aarch64-linux-gnu)
  CPU: unknown
  WORD_SIZE: 64
  LIBM: libopenlibm
  LLVM: libLLVM-10.0.0 (ORCJIT, generic)
Environment:
  JULIA_NUM_THREADS = 16

But there are more features for:

julia> ccall(:jl_dump_host_cpu, Cvoid, ())
CPU: generic
Features: aes, sha2, crc, lse, fullfp16, rdm, rcpc, ccpp, dotprod, ssbs

1.4.2 gives:

julia> ccall(:jl_dump_host_cpu, Cvoid, ())
CPU: generic
Features: crypto, crc, lse, fullfp16, rdm, rcpc, dcpop

@yuyichao yuyichao force-pushed the ib/add_aarch64_cpus branch from 05572fd to cfc5a73 Compare June 30, 2020 21:02
@yuyichao
Copy link
Contributor

.... There was a bug in the error checking. I guess I never had any hardware to test where LLVM is not able to detect... (cortex-a57 only basically....)

The fix is the second commit. You should be able to just cherry pick that for testing (assuming it compiles....)

@yuyichao yuyichao force-pushed the ib/add_aarch64_cpus branch from cfc5a73 to 1b09d1e Compare June 30, 2020 21:06
@ViralBShah ViralBShah added the system:arm ARMv7 and AArch64 label Jul 1, 2020
@IanButterworth IanButterworth mentioned this pull request Jul 1, 2020
3 tasks
@IanButterworth
Copy link
Member Author

Great, now working!

julia> versioninfo()
Julia Version 1.5.0-DEV.545
Commit 8ae92cc5f0* (2020-03-31 01:16 UTC)
Platform Info:
  OS: Linux (aarch64-linux-gnu)
  CPU: unknown
  WORD_SIZE: 64
  LIBM: libopenlibm
  LLVM: libLLVM-10.0.0 (ORCJIT, neoverse-n1)
Environment:
  JULIA_NUM_THREADS = 16

julia> ccall(:jl_dump_host_cpu, Cvoid, ())
CPU: neoverse-n1
Features: aes, sha2, crc, lse, fullfp16, rdm, rcpc, ccpp, dotprod, ssbs, v8_1a, v8_2a

Although I did hit a non-fatal bug when I first generated a function after build (after running above):

ERROR: error: failed to compute relocation: R_AARCH64_PREL64, Invalid data was encountered while parsing the file

@yuyichao
Copy link
Contributor

yuyichao commented Jul 1, 2020

This PR should change nothing regarding relocation, especially if you are not manually specifying any target during compilation.
It is possible though that the LLVM linker or something is not able to handle some features or code patterns caused either by the processor name or the features.

Running it in debugger might give you at least some idea when/where/on which function/code it happens. You can also try specifying the cpu target -C when you start julia to disable features or set a different CPU name, e.g. -C "native,-ssbs" or -C "cortex-a76". If you can identify the feature/CPU that's problematic we can disable it by raising the LLVM version requirement.

@vchuravy
Copy link
Member

vchuravy commented Jul 1, 2020

Those relocation warnings look like #35460

@IanButterworth
Copy link
Member Author

Indeed:

julia> error()
ERROR: error: failed to compute relocation: R_AARCH64_PREL64, Invalid data was encountered while parsing the file
error: failed to compute relocation: R_AARCH64_PREL64, Invalid data was encountered while parsing the file
error: failed to compute relocation: R_AARCH64_PREL64, Invalid data was encountered while parsing the file

@vchuravy
Copy link
Member

vchuravy commented Jul 1, 2020

And you are indeed on LLVM10

@IanButterworth
Copy link
Member Author

IanButterworth commented Jul 7, 2020

@yuyichao what would count as success here? Should we try to run julia tests on some of the added aarch64 CPU’s? Is now the time to do that?

@yuyichao
Copy link
Contributor

yuyichao commented Jul 7, 2020

The PR is already done and is just waiting to see if anyone else have any comments before merging. (though I did make another update due to recent LLVM 11 addition and that I managed to find the TRM for cortex-a78 and cortex-x1.)

It's impossible to test on all CPUs and I don't think that's required for merging. If you do have non cortex-a57, carmel or neoverse-n1 to test that would be nice. And since the commit was in a "finished" state when I first pushed it the test can be done at any time.

yuyichao and others added 2 commits July 8, 2020 12:43
The code was using two different `std::pair` types to pass this info around
as remnant of an earlier version and the auto conversion between the two types
does not retain the meaning we want...

Use a custom struct to hold the info to guard against this kind of errors in the future.
* Clean up old LLVM version check
* Clean up old cores that'll probably not be used (and some that are removed from LLVM)
* Add document about data sources
* Update AArch64 feature list to match changes to the LLVM ones
* ARMv8.4 - ARMv8.6 support
* Add all known cores that I can find and all information I can find about them
* Remove a few A64 only cores from AArch32 CPU list
* Rename Apple cores (following LLVM)
* SVE register size

Co-authored-by: Ian <[email protected]>
@yuyichao yuyichao force-pushed the ib/add_aarch64_cpus branch from 18bd658 to e0e3825 Compare July 8, 2020 16:44
@IanButterworth
Copy link
Member Author

I noticed that #36502 was backported to 1.5-rc2, so wondered if this could be?

cc. @KristofferC

@yuyichao
Copy link
Contributor

yuyichao commented Jul 9, 2020

I mentioned backporting in that one because of the one bug fix in it. But if the whole pr is considered backportable then this one should be the same. The second commit here is also k8nd of a bug fix.

@yuyichao yuyichao merged commit 47ffc00 into JuliaLang:master Jul 9, 2020
@IanButterworth
Copy link
Member Author

I think that's logical, and agree that the 2nd commit is a bug fix. @KristofferC would a backport be acceptable?

@IanButterworth IanButterworth mentioned this pull request Aug 4, 2020
25 tasks
@IanButterworth IanButterworth deleted the ib/add_aarch64_cpus branch March 4, 2022 04:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
system:arm ARMv7 and AArch64
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants