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

Display human readable text for AArch64 vendors #13825

Merged
merged 14 commits into from Dec 17, 2019
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
62 changes: 54 additions & 8 deletions lib/spack/llnl/util/cpu/detect.py
Expand Up @@ -109,20 +109,64 @@ def sysctl(*args):
'model name': sysctl('-n', 'machdep.cpu.brand_string')
}

# Super hacky way to deal with slight representation differences
# Would be better to somehow consider these "identical"
if 'sse4.1' in info['flags']:
adjust_raw_flags(info)
alalazo marked this conversation as resolved.
Show resolved Hide resolved

return info


def adjust_raw_flags(info):
"""Adjust the flags detected on the system to homogenize
slightly different representations.
"""
# Flags detected on Darwin turned to their linux counterpart
flags = info.get('flags', [])
tgamblin marked this conversation as resolved.
Show resolved Hide resolved
if 'sse4.1' in flags:
info['flags'] += ' sse4_1'
if 'sse4.2' in info['flags']:
if 'sse4.2' in flags:
info['flags'] += ' sse4_2'
if 'avx1.0' in info['flags']:
if 'avx1.0' in flags:
info['flags'] += ' avx'
if 'clfsopt' in info['flags']:
if 'clfsopt' in flags:
info['flags'] += ' clflushopt'
if 'xsave' in info['flags']:
if 'xsave' in flags:
info['flags'] += ' xsavec xsaveopt'
alalazo marked this conversation as resolved.
Show resolved Hide resolved

return info

def adjust_raw_vendor(info):
"""Adjust the vendor field to make it human readable"""
# Mapping numeric codes to vendor (ARM). This list is a merge from
# different sources:
#
# https://github.com/karelzak/util-linux/blob/master/sys-utils/lscpu-arm.c
# https://developer.arm.com/docs/ddi0487/latest/arm-architecture-reference-manual-armv8-for-armv8-a-architecture-profile
# https://github.com/gcc-mirror/gcc/blob/master/gcc/config/aarch64/aarch64-cores.def
# https://patchwork.kernel.org/patch/10524949/
arm_vendor = {
'0x41': 'ARM',
'0x42': 'Broadcom',
'0x43': 'Cavium',
'0x44': 'DEC',
'0x46': 'Fujitsu',
'0x48': 'HiSilicon',
'0x49': 'Infineon Technologies AG',
'0x4d': 'Motorola',
'0x4e': 'Nvidia',
'0x50': 'APM',
'0x51': 'Qualcomm',
'0x53': 'Samsung',
'0x56': 'Marvell',
'0x61': 'Apple',
'0x66': 'Faraday',
'0x68': 'HXT',
'0x69': 'Intel'
}
tgamblin marked this conversation as resolved.
Show resolved Hide resolved
if 'CPU implementer' not in info:
return

for code, vendor_name in arm_vendor.items():
if info['CPU implementer'] == code:
info['CPU implementer'] = vendor_name
break
alalazo marked this conversation as resolved.
Show resolved Hide resolved


def raw_info_dictionary():
Expand All @@ -139,6 +183,8 @@ def raw_info_dictionary():
warnings.warn(str(e))

if info:
adjust_raw_flags(info)
adjust_raw_vendor(info)
break

return info
Expand Down
50 changes: 38 additions & 12 deletions lib/spack/llnl/util/cpu/microarchitectures.json
Expand Up @@ -1163,7 +1163,7 @@
},
"thunderx2": {
"from": "aarch64",
"vendor": "0x43",
"vendor": "Cavium",
"features": [
"fp",
"asimd",
Expand Down Expand Up @@ -1196,15 +1196,28 @@
"flags": "-mcpu=thunderx2t99"
}
],
"clang": {
"versions": ":",
"flags": "-march=armv8-a -mcpu=generic"
}
"clang": [
{
"versions": ":6.9",
"family": "aarch64",
"flags": "-march={family} -mattr=+v8.1a,+fp-armv8,+neon,+crc,+crypto"
},
{
"versions": "7:8.9",
"family": "aarch64",
"flags": "-march={family} -mattr=+v8.1a,+fp-armv8,+aes,+neon,+sha2,+crc,+crypto"
},
{
"versions": "9:",
"family": "aarch64",
"flags": "-march={family} -mcpu=thunderx2t99"
}
]
Copy link
Contributor

@t-karatsu t-karatsu Nov 27, 2019

Choose a reason for hiding this comment

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

Thanks @alalazo, but these options raise errors following:

[ogura@langtx202 bin]$ ./clang test.c -march=aarch64 -mattr=+v8.1a,+fp-armv8,+neon,+crc,+crypto
clang-8: error: unknown argument: '-mattr=+v8.1a,+fp-armv8,+neon,+crc,+crypto'
clang-8: error: the clang compiler does not support '-march=aarch64'

So, options for llc and clang seem different. I considered options for clang compiler commands. Currently I will Check whether it works normally.

diff --git a/lib/spack/llnl/util/cpu/microarchitectures.json b/lib/spack/llnl/util/cpu/microarchitectures.json
index 3b58899..e37327a 100644
--- a/lib/spack/llnl/util/cpu/microarchitectures.json
+++ b/lib/spack/llnl/util/cpu/microarchitectures.json
@@ -1196,10 +1196,16 @@
             "flags": "-mcpu=thunderx2t99"
           }
         ],
-        "clang": {
-          "versions": ":",
-          "flags": "-march=armv8-a -mcpu=generic"
-        }
+        "clang": [
+          {
+            "versions": "3.9:",
+            "flags": "-march=armv8.1-a+crc+crypto"
+          },
+          {
+            "versions": "5.0:",
+            "flags": "-mcpu=thunderx2t99"
+          }
+        ]
       }
     },
     "a64fx": {
@@ -1246,10 +1252,16 @@
             "flags": "-arch=armv8.2a+crc+aes+sh2+fp16+sve -msve-vector-bits=512"
           }
         ],
-        "clang": {
-          "versions": ":",
-          "flags": "-march=armv8-a -mcpu=generic"
-        }
+        "clang": [
+          {
+            "versions": "3.9:",
+            "flags": "-march=armv8.2-a+crc+crypto+fp16"
+          },
+          {
+            "versions": "5.0:",
+            "flags": "-march=armv8.2-a+crc+crypto+fp16+sve"
+          }
+        ]
       }
     },
     "arm": {

I am sorry if there is a lack of consideration here.

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks @t-karatsu. You're completely correct. I think we need to revisit also the flags that are passed to x86-64. I tried to disassemble some vectorizable code and it seems clang does not give the same meaning to flags that are also used by its assembler. I'll try to see if I can find some reference for it and eventually post here what I'll find. @tgamblin FYI

Copy link
Member Author

Choose a reason for hiding this comment

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

For reference, when I researched this I couldn't find a good source of information. Even clang manuals suggest options that are either not recognized on command line or don't work as described. The best source of information I could find at the time was this SO answer.

Copy link
Member Author

Choose a reason for hiding this comment

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

@becker33 I think you might also be interested in the discussion above.

Copy link
Member Author

Choose a reason for hiding this comment

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

Further update: I found this discussion on how clang opt and llc relate to each other (and goes on a 4 stage manual build to use them). There's also this other question that looks exactly what we need, but has no answer.

I'll continue looking, in the meanwhile I posted a message to cfe users - let's see if something comes out from there.

Copy link
Contributor

Choose a reason for hiding this comment

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

For reference, when I researched this I couldn't find a good source of information.

How about clang preprocessor test code?
https://github.com/llvm/llvm-project/blob/master/clang/test/Preprocessor/aarch64-target-features.c

Copy link
Member Author

Choose a reason for hiding this comment

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

@tkameyama @t-karatsu I got access today to a thunderx2 machine. Will try out the flags there.

How about clang preprocessor test code?

That should work thanks. The front-end driver seems consistent with GCC in terms of flags which is also what is briefly written here. Will update this PR asap and ping you when done.

}
},
"a64fx": {
"from": "aarch64",
"vendor": "0x46",
"vendor": "Fujitsu",
"features": [
"fp",
"asimd",
Expand Down Expand Up @@ -1239,17 +1252,30 @@
},
{
"versions": "7:7.9",
"flags": "-arch=armv8.2a+crc+crypt+fp16"
"flags": "-arch=armv8.2a+crc+crypto+fp16"
alalazo marked this conversation as resolved.
Show resolved Hide resolved
alalazo marked this conversation as resolved.
Show resolved Hide resolved
},
{
"versions": "8:",
"flags": "-arch=armv8.2a+crc+aes+sh2+fp16+sve -msve-vector-bits=512"
"flags": "-arch=armv8.2a+crc+aes+sha2+fp16+sve -msve-vector-bits=512"
alalazo marked this conversation as resolved.
Show resolved Hide resolved
alalazo marked this conversation as resolved.
Show resolved Hide resolved
}
],
"clang": {
"versions": ":",
"flags": "-march=armv8-a -mcpu=generic"
}
"clang": [
{
"versions": ":4.9",
"family": "aarch64",
"flags": "-march={aarch64} -mattr=+v8.2a,+crc,+crypto,+fullfp16"
},
{
"versions": "5:6.9",
"family": "aarch64",
"flags": "-march={aarch64} -mattr=+v8.2a,+crc,+crypto,+fullfp16,+sve"
},
{
"versions": "7:",
"family": "aarch64",
"flags": "-march={aarch64} -mattr=+v8.2a,+crc,+crypto,+fullfp16,+sve,+sha2"
}
]
}
},
"arm": {
Expand Down
3 changes: 2 additions & 1 deletion lib/spack/spack/test/llnl/util/cpu.py
Expand Up @@ -218,7 +218,8 @@ def test_target_json_schema():
('icelake', 'clang', '8.0.0', '-march=x86-64 -mcpu=icelake-client'),
('zen2', 'clang', '9.0.0', '-march=x86-64 -mcpu=znver2'),
('power9le', 'clang', '8.0.0', '-march=ppc64le -mcpu=pwr9'),
('thunderx2', 'clang', '6.0.0', '-march=armv8-a -mcpu=generic'),
('thunderx2', 'clang', '6.0.0',
'-march=aarch64 -mattr=+v8.1a,+fp-armv8,+neon,+crc,+crypto'),
# Test Intel on Intel CPUs
('sandybridge', 'intel', '17.0.2', '-march=corei7-avx -mtune=corei7-avx'),
('sandybridge', 'intel', '18.0.5',
Expand Down