-
Notifications
You must be signed in to change notification settings - Fork 15.4k
[lldb][test] Unify and extend test infrastructure for checking CPU features #153914
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
Conversation
Add new `CPUFeature` class as an abstraction to hide the platform-specific implementations to check for CPU features. Unify local (on host) and remote (on device) test execution by always going through `test.Base.run_platform_command()` which uses LLDB's `platform shell <cmd>` command.
|
@llvm/pr-subscribers-lldb Author: Julian Lettner (yln) ChangesThis addresses limitations in our testing infrastructure for checking CPU features. Before this
Introduce Potential future cleanups: I think Full diff: https://github.com/llvm/llvm-project/pull/153914.diff 5 Files Affected:
diff --git a/lldb/packages/Python/lldbsuite/test/cpu_feature.py b/lldb/packages/Python/lldbsuite/test/cpu_feature.py
new file mode 100644
index 0000000000000..9c0a4102ea944
--- /dev/null
+++ b/lldb/packages/Python/lldbsuite/test/cpu_feature.py
@@ -0,0 +1,58 @@
+"""
+Platform-agnostic helper to query for CPU features.
+"""
+
+import re
+
+
+class CPUFeature:
+ def __init__(self, linux_cpu_info_flag: str, darwin_sysctl_key: str):
+ self.cpu_info_flag = linux_cpu_info_flag
+ self.sysctl_key = darwin_sysctl_key
+
+ def __str__(self):
+ return self.cpu_info_flag
+
+ def is_supported(self, triple, cmd_runner):
+ if re.match(".*-.*-linux", triple):
+ err_msg, res = self._is_supported_linux(cmd_runner)
+ elif re.match(".*-apple-.*", triple):
+ err_msg, res = self._is_supported_darwin(cmd_runner)
+ else:
+ err_msg, res = None, False
+
+ if err_msg:
+ print(f"CPU feature check failed: {err_msg}")
+
+ return res
+
+ def _is_supported_linux(self, cmd_runner):
+ cmd = "cat /proc/cpuinfo"
+ err, retcode, output = cmd_runner(cmd)
+ if err.Fail() or retcode != 0:
+ err_msg = f"cat /proc/cpuinfo failed: {output}"
+ return err_msg, False
+
+ return None, (self.cpu_info_flag in output)
+
+ def _is_supported_darwin(self, cmd_runner):
+ cmd = f"sysctl -n {self.sysctl_key}"
+ err, retcode, output = cmd_runner(cmd)
+ if err.Fail() or retcode != 0:
+ return output, False
+
+ return None, (output.strip() == "1")
+
+
+# List of CPU features
+FPMR = CPUFeature("fpmr", "???")
+GCS = CPUFeature("gcs", "???")
+LASX = CPUFeature("lasx", "???")
+LSX = CPUFeature("lsx", "???")
+MTE = CPUFeature("mte", "???")
+MTE_STORE_ONLY = CPUFeature("mtestoreonly", "???")
+PTR_AUTH = CPUFeature("paca", "hw.optional.arm.FEAT_PAuth2")
+SME = CPUFeature("sme", "hw.optional.arm.FEAT_SME")
+SME_FA64 = CPUFeature("smefa64", "???")
+SME2 = CPUFeature("sme2", "hw.optional.arm.FEAT_SME2")
+SVE = CPUFeature("sve", "???")
diff --git a/lldb/packages/Python/lldbsuite/test/decorators.py b/lldb/packages/Python/lldbsuite/test/decorators.py
index bd10bcc3d6ce0..bc400cd3b8f12 100644
--- a/lldb/packages/Python/lldbsuite/test/decorators.py
+++ b/lldb/packages/Python/lldbsuite/test/decorators.py
@@ -23,6 +23,7 @@
from lldbsuite.support import temp_file
from lldbsuite.test import lldbplatform
from lldbsuite.test import lldbplatformutil
+from lldbsuite.test.cpu_feature import CPUFeature
class DecorateMode:
@@ -1127,24 +1128,13 @@ def skipIfLLVMTargetMissing(target):
return unittest.skipIf(not found, "requires " + target)
-# Call sysctl on darwin to see if a specified hardware feature is available on this machine.
-def skipUnlessFeature(feature):
- def is_feature_enabled():
- if platform.system() == "Darwin":
- try:
- output = subprocess.check_output(
- ["/usr/sbin/sysctl", feature], stderr=subprocess.DEVNULL
- ).decode("utf-8")
- # If 'feature: 1' was output, then this feature is available and
- # the test should not be skipped.
- if re.match(r"%s: 1\s*" % feature, output):
- return None
- else:
- return "%s is not supported on this system." % feature
- except subprocess.CalledProcessError:
- return "%s is not supported on this system." % feature
+def skipUnlessFeature(cpu_feature: CPUFeature):
+ def hasFeature(test_case):
+ if not test_case.isSupported(cpu_feature):
+ return f"Unsupported CPU feature: {cpu_feature}"
+ return None
- return skipTestIfFn(is_feature_enabled)
+ return skipTestIfFn(hasFeature)
def skipIfBuildType(types: list[str]):
diff --git a/lldb/packages/Python/lldbsuite/test/lldbtest.py b/lldb/packages/Python/lldbsuite/test/lldbtest.py
index 0fc85fcc4d2d6..d9cf2689b3d25 100644
--- a/lldb/packages/Python/lldbsuite/test/lldbtest.py
+++ b/lldb/packages/Python/lldbsuite/test/lldbtest.py
@@ -48,6 +48,7 @@
# LLDB modules
import lldb
from . import configuration
+from . import cpu_feature
from . import decorators
from . import lldbplatformutil
from . import lldbtest_config
@@ -1315,39 +1316,6 @@ def isPPC64le(self):
return True
return False
- def getCPUInfo(self):
- triple = self.dbg.GetSelectedPlatform().GetTriple()
-
- # TODO other platforms, please implement this function
- if not re.match(".*-.*-linux", triple):
- return ""
-
- # Need to do something different for non-Linux/Android targets
- cpuinfo_path = self.getBuildArtifact("cpuinfo")
- if configuration.lldb_platform_name:
- self.runCmd(
- 'platform get-file "/proc/cpuinfo" ' + cpuinfo_path, check=False
- )
- if not self.res.Succeeded():
- if self.TraceOn():
- print(
- 'Failed to get /proc/cpuinfo from remote: "{}"'.format(
- self.res.GetOutput().strip()
- )
- )
- print("All cpuinfo feature checks will fail.")
- return ""
- else:
- cpuinfo_path = "/proc/cpuinfo"
-
- try:
- with open(cpuinfo_path, "r") as f:
- cpuinfo = f.read()
- except:
- return ""
-
- return cpuinfo
-
def isAArch64(self):
"""Returns true if the architecture is AArch64."""
arch = self.getArchitecture().lower()
@@ -1360,39 +1328,43 @@ def isARM(self):
self.getArchitecture().lower().startswith("arm")
)
+ def isSupported(self, cpu_feature: cpu_feature.CPUFeature):
+ triple = self.dbg.GetSelectedPlatform().GetTriple()
+ cmd_runner = self.run_platform_command
+ return cpu_feature.is_supported(triple, cmd_runner)
+
def isAArch64SVE(self):
- return self.isAArch64() and "sve" in self.getCPUInfo()
+ return self.isAArch64() and self.isSupported(cpu_feature.SVE)
def isAArch64SME(self):
- return self.isAArch64() and "sme" in self.getCPUInfo()
+ return self.isAArch64() and self.isSupported(cpu_feature.SME)
def isAArch64SME2(self):
# If you have sme2, you also have sme.
- return self.isAArch64() and "sme2" in self.getCPUInfo()
+ return self.isAArch64() and self.isSupported(cpu_feature.SME2)
def isAArch64SMEFA64(self):
# smefa64 allows the use of the full A64 instruction set in streaming
# mode. This is required by certain test programs to setup register
# state.
- cpuinfo = self.getCPUInfo()
- return self.isAArch64() and "sme" in cpuinfo and "smefa64" in cpuinfo
+ return self.isAArch64() and self.isSupported(cpu_feature.SME) and self.isSupported(cpu_feature.SME_FA64)
def isAArch64MTE(self):
- return self.isAArch64() and "mte" in self.getCPUInfo()
+ return self.isAArch64() and self.isSupported(cpu_feature.MTE)
def isAArch64MTEStoreOnly(self):
- return self.isAArch64() and "mtestoreonly" in self.getCPUInfo()
+ return self.isAArch64() and self.isSupported(cpu_feature.MTE_STORE_ONLY)
def isAArch64GCS(self):
- return self.isAArch64() and "gcs" in self.getCPUInfo()
+ return self.isAArch64() and self.isSupported(cpu_feature.GCS)
def isAArch64PAuth(self):
if self.getArchitecture() == "arm64e":
return True
- return self.isAArch64() and "paca" in self.getCPUInfo()
+ return self.isAArch64() and self.isSupported(cpu_feature.PTR_AUTH)
def isAArch64FPMR(self):
- return self.isAArch64() and "fpmr" in self.getCPUInfo()
+ return self.isAArch64() and self.isSupported(cpu_feature.FPMR)
def isAArch64Windows(self):
"""Returns true if the architecture is AArch64 and platform windows."""
@@ -1407,10 +1379,10 @@ def isLoongArch(self):
return arch in ["loongarch64", "loongarch32"]
def isLoongArchLSX(self):
- return self.isLoongArch() and "lsx" in self.getCPUInfo()
+ return self.isLoongArch() and self.isSupported(cpu_feature.LSX)
def isLoongArchLASX(self):
- return self.isLoongArch() and "lasx" in self.getCPUInfo()
+ return self.isLoongArch() and self.isSupported(cpu_feature.LASX)
def isRISCV(self):
"""Returns true if the architecture is RISCV64 or RISCV32."""
diff --git a/lldb/test/API/commands/register/register/register_command/TestRegisters.py b/lldb/test/API/commands/register/register/register_command/TestRegisters.py
index 0134139892794..29d090a279070 100644
--- a/lldb/test/API/commands/register/register/register_command/TestRegisters.py
+++ b/lldb/test/API/commands/register/register/register_command/TestRegisters.py
@@ -21,24 +21,6 @@ def tearDown(self):
self.dbg.GetSelectedTarget().GetProcess().Destroy()
TestBase.tearDown(self)
- # on macOS, detect if the current machine is arm64 and supports SME
- def get_sme_available(self):
- if self.getArchitecture() != "arm64":
- return None
- try:
- sysctl_output = subprocess.check_output(
- ["sysctl", "hw.optional.arm.FEAT_SME"]
- ).decode("utf-8")
- except subprocess.CalledProcessError:
- return None
- m = re.match(r"hw\.optional\.arm\.FEAT_SME: (\w+)", sysctl_output)
- if m:
- if int(m.group(1)) == 1:
- return True
- else:
- return False
- return None
-
@skipIfiOSSimulator
@skipIf(archs=no_match(["amd64", "arm$", "i386", "x86_64"]))
@expectedFailureAll(oslist=["freebsd", "netbsd"], bugnumber="llvm.org/pr48371")
@@ -51,7 +33,7 @@ def test_register_commands(self):
self.log_enable("registers")
error_str_matched = False
- if self.get_sme_available() and self.platformIsDarwin():
+ if self.isAArch64SME() and self.platformIsDarwin():
# On Darwin AArch64 SME machines, we will have unavailable
# registers when not in Streaming SVE Mode/SME, so
# `register read -a` will report that some registers
diff --git a/lldb/test/API/macosx/sme-registers/TestSMERegistersDarwin.py b/lldb/test/API/macosx/sme-registers/TestSMERegistersDarwin.py
index 6f9d055cef506..28bebb3ffeb6e 100644
--- a/lldb/test/API/macosx/sme-registers/TestSMERegistersDarwin.py
+++ b/lldb/test/API/macosx/sme-registers/TestSMERegistersDarwin.py
@@ -1,6 +1,7 @@
import lldb
from lldbsuite.test.lldbtest import *
from lldbsuite.test.decorators import *
+import lldbsuite.test.cpu_feature as cpu_feature
import lldbsuite.test.lldbutil as lldbutil
import os
@@ -9,10 +10,9 @@ class TestSMERegistersDarwin(TestBase):
NO_DEBUG_INFO_TESTCASE = True
mydir = TestBase.compute_mydir(__file__)
- @skipIfRemote
@skipUnlessDarwin
- @skipUnlessFeature("hw.optional.arm.FEAT_SME")
- @skipUnlessFeature("hw.optional.arm.FEAT_SME2")
+ @skipUnlessFeature(cpu_feature.SME)
+ @skipUnlessFeature(cpu_feature.SME2)
# thread_set_state/thread_get_state only avail in macOS 15.4+
@skipIf(macos_version=["<", "15.4"])
def test(self):
|
| # mode. This is required by certain test programs to setup register | ||
| # state. | ||
| cpuinfo = self.getCPUInfo() | ||
| return self.isAArch64() and "sme" in cpuinfo and "smefa64" in cpuinfo |
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.
Note that cpuinfo here is just the contents of /proc/cpuinfo as a string (not a collection of flags). So testing with a prefix/substring will always be trivially true (and is nonsensical), e.g., sme will automatically be true if sme2 or smefa64 is present. I did not change the behavior of this.
One way to fix this would be to parse and merge the CPU flags in cpuinfo and use exact match on that set of flags.
DavidSpickett
left a comment
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.
Always wary of adding more support code before we actually need it, but this seems like a good direction.
Right now, SME is the common thing between Linux and Darwin and, like a lot of things SME, they are a bit different so they naturally had different tests.
I will be adding SME only support for Linux, and maybe I'll be able to use the Darwin tests but the chances are low.
In future when Linux and Darwin both need to test more straightforward things, being able to share the feature checks will be good.
JDevlieghere
left a comment
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.
LGTM for Darwin.
|
@DavidSpickett, thank you for your feedback! 🙏 |
DavidSpickett
left a comment
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.
Fine with me once the comments are addressed.
|
#160675 to improve the proc/cpuinfo parsing. |
…m#153914) This addresses limitations in our testing infrastructure for checking CPU features. Before this * `getCPUInfo()` was Linux-only, and the * `@skipUnlessFeature` decorator was Darwin-only and did not consider the remote (on device) testing use case. Introduce `CPUFeature` class as an abstraction to hide the platform-specific implementations to check for CPU features. Unify local (on host) and remote (on device) test execution by always going through `test.run_platform_command()` which uses LLDB's `platform shell <cmd>` command. Potential future cleanups: I think `@skipUnlessFeature` decorator could be used in place of code like this: ``` if not self.isAArch64SME(): self.skipTest("SME must be present.") if not self.isAArch64SME2(): self.skipTest("SME2 must be present.") ```
This addresses limitations in our testing infrastructure for checking CPU features. Before this
getCPUInfo()was Linux-only, and the@skipUnlessFeaturedecorator was Darwin-only and did not consider the remote (on device) testing use case.Introduce
CPUFeatureclass as an abstraction to hide the platform-specific implementations to check for CPU features. Unify local (on host) and remote (on device) test execution by always going throughtest.run_platform_command()which uses LLDB'splatform shell <cmd>command.Potential future cleanups: I think
@skipUnlessFeaturedecorator could be used in place of code like this: