[Lldb-commits] [PATCH] D96463: [LLDB] Arm64/Linux test case for MTE and Pointer Authentication regset

[Muhammad Omair Javaid via Phabricator via lldb-commits]

omjavaid added inline comments.


================
Comment at: lldb/test/API/commands/register/register/aarch64_dynamic_regset/TestArm64DynamicRegsets.py:96
+            if 'Scalable Vector Extension Registers' in registerSet.GetName():
+                self.assertTrue(self.isAArch64SVE(
+                ), 'LLDB enabled AArch64 SVE register set when it was disabled by target.')
----------------
DavidSpickett wrote:
> Move the closing `)` onto the first line. Also I'd indent the line below like:
> ```
> self.assertTrue(self.isAArch64SVE(), 
>     'LLDB enabled AArch64 SVE register set when it was disabled by target.')
> self.....
> ```
> 
> (applies to the next two as well)
Ack.


================
Comment at: lldb/test/API/commands/register/register/aarch64_dynamic_regset/TestArm64DynamicRegsets.py:106
+            if 'Pointer Authentication Registers' in registerSet.GetName():
+                self.assertTrue(self.isAArch64MTE(
+                ), 'LLDB enabled AArch64 Pointer Authentication register set when it was disabled by target.')
----------------
DavidSpickett wrote:
> Should be `isAArch64PAuth`
Ack.


================
Comment at: lldb/test/API/commands/register/register/aarch64_dynamic_regset/main.c:81-82
+  unsigned int sve_is_enabled = hwcap & HWCAP_SVE;
+  unsigned int pauth_is_enabled = hwcap & HWCAP_PACA;
+  unsigned int mte_is_enabled = hwcap2 & HWCAP2_MTE;
+
----------------
DavidSpickett wrote:
> These are unused since we get it from the cpuinfo, then you can inline the sve check in the if below.
Ack.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D96463/new/

https://reviews.llvm.org/D96463