-
Notifications
You must be signed in to change notification settings - Fork 216
Pass correct -march option on RISC-V systems when optarch toolchain option is enabled
#5029
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
base: develop
Are you sure you want to change the base?
Conversation
optarch toolchain option is enabled
optarch toolchain option is enabled-march option on RISC-V systems when optarch toolchain option is enabled
easybuild/tools/systemtools.py
Outdated
| os_type = get_os_type() | ||
| if os_type == LINUX: | ||
| if is_readable(PROC_CPUINFO_FP): | ||
| cmd = "cat /proc/cpuinfo | grep isa | head -1 | cut -d':' -f2" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Rather than relying on a pipeline, can't we just read /proc/cpuinfo as a file (with read_file), and then use a regular expression to grab the isa part?
Also, grep isa is a bit too loose (on an x86_64 system, it matches misalignsse in flags, for example).
How portable is this across CPU families?
If it's not, we should make this function specific to RISC-V, and return None on non-RISC-V systems?
We should also add some tests when introducing a new function like this...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should probably be risc-v specific (changing the function name to something like get_isa_riscv)
Atleast on my intel CPU the closest thing would be the flags : ...
Details
processor : 0
vendor_id : GenuineIntel
cpu family : 6
model : 183
model name : 13th Gen Intel(R) Core(TM) i9-13900K
stepping : 1
microcode : 0x12f
cpu MHz : 1053.547
cache size : 36864 KB
physical id : 0
siblings : 32
core id : 0
cpu cores : 24
apicid : 0
initial apicid : 0
fpu : yes
fpu_exception : yes
cpuid level : 32
wp : yes
flags : fpu vme de pse tsc msr pae mce cx8 apic sep mtrr pge mca cmov pat pse36 clflush dts acpi mmx fxsr sse sse2 ss ht tm pbe syscall nx pdpe1gb rdtscp lm constant_tsc art arch_perfmon pebs bts rep_good nopl xtopology nonstop_tsc cpu
id aperfmperf tsc_known_freq pni pclmulqdq dtes64 monitor ds_cpl vmx smx est tm2 ssse3 sdbg fma cx16 xtpr pdcm pcid sse4_1 sse4_2 x2apic movbe popcnt tsc_deadline_timer aes xsave avx f16c rdrand lahf_lm abm 3dnowprefetch cpuid_fault epb ssbd ibr
s ibpb stibp ibrs_enhanced tpr_shadow flexpriority ept vpid ept_ad fsgsbase tsc_adjust bmi1 avx2 smep bmi2 erms invpcid rdseed adx smap clflushopt clwb intel_pt sha_ni xsaveopt xsavec xgetbv1 xsaves split_lock_detect user_shstk avx_vnni dtherm i
da arat pln pts hwp hwp_notify hwp_act_window hwp_epp hwp_pkg_req hfi vnmi umip pku ospke waitpkg gfni vaes vpclmulqdq tme rdpid movdiri movdir64b fsrm md_clear serialize pconfig arch_lbr ibt flush_l1d arch_capabilities
vmx flags : vnmi preemption_timer posted_intr invvpid ept_x_only ept_ad ept_1gb flexpriority apicv tsc_offset vtpr mtf vapic ept vpid unrestricted_guest vapic_reg vid ple shadow_vmcs ept_mode_based_exec tsc_scaling usr_wait_pause
bugs : spectre_v1 spectre_v2 spec_store_bypass swapgs eibrs_pbrsb rfds bhi
bogomips : 5990.40
clflush size : 64
cache_alignment : 64
address sizes : 46 bits physical, 48 bits virtual
power management:
| if os_type == LINUX: | ||
| if is_readable(PROC_CPUINFO_FP): | ||
| _log.debug("Trying to determine ISA string on Linux via %s", PROC_CPUINFO_FP) | ||
| proc_cpuinfo = read_file(PROC_CPUINFO_FP) | ||
| isa_regex = re.compile(r"^isa\s*:\s*(.*)") | ||
| res = isa_regex.search(proc_cpuinfo) | ||
| if res: | ||
| isa_string = res.group(1) | ||
| _log.debug("Found ISA string using regex '%s': %s", isa_regex.pattern, isa_string) | ||
| else: | ||
| _log.debug("Failed to determine ISA string from %s", PROC_CPUINFO_FP) | ||
| else: | ||
| _log.debug("%s not found to determine ISA string", PROC_CPUINFO_FP) | ||
| else: | ||
| raise SystemToolsException("Could not determine ISA string (OS: %s)" % os_type) | ||
| return isa_string |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think raising an exception here will cause EB to fail on any non-LINUX systems, as the function calling this needs to always be resolved.
Maybe a cleaner solution would be to evaluate this at run-time instead of at the class declaration
Also i am wondering if defaulting to an empty string is the right thing to do.
This would cause -march= to be added causing some obfuscated failures while compiling.
Would it be better to fallback to something like the generic of rv64gc? The alternative would be to give a clear failure (but also would need a solution for the aforementioned problem of this function being called in the class declaration)
'-march=native" is not a valid value when building in RISC-V.
Pass the ISA string with the supported extensions of the target platform instead.