[Lldb-commits] [PATCH] D155269: [lldb][AArch64] Add SME streaming vector length pseudo register

[David Spickett via Phabricator via lldb-commits]

DavidSpickett added a comment.

> Could you offer higher abstractions? Show me the current SVME vector length? 
> Show me the current SVME mode?

Adding it to `process status` is along those lines, we have stuff like the 
number of addressable bits there right now. Overall I prefer the registers 
route just for visibility but we'll see what the early users find.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D155269

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D155269: [lldb][AArch64] Add SME streaming vector length pseudo register

[Thorsten via Phabricator via lldb-commits]

tschuett added a comment.

I would never question giving low-level access to the registers. As you 
mentioned less experienced users could accidentally switch between the modes 
with knowing.




Comment at: 
lldb/test/API/commands/register/register/aarch64_sve_registers/rw_access_dynamic_resize/TestSVEThreadedDynamic.py:262
+self.do_svg_test(process, supported_vg, supported_vg)
\ No newline at end of file


Newline please


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D155269

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D155269: [lldb][AArch64] Add SME streaming vector length pseudo register

[David Spickett via Phabricator via lldb-commits]

DavidSpickett added a comment.

> I would never question giving low-level access to the registers.

Well in your defense, both `svg` and `svcr` will actually be pseudo registers. 
So the user isn't getting access to the "real" ones either way, we're emulating 
the behaviour with ptrace commands.

> As you mentioned less experienced users could accidentally switch between the 
> modes with knowing.

If we follow the kernel to the letter you can also mode switch by writing 
floating point registers while in streaming mode. That currently doesn't happen 
due to the way we model `v` registers as subsets of `z` but I might have to 
change that and if I do, that's another potential pitfall.

Ideally we would have as few routes to mode switch via the debugger as 
possible. Writing to the streaming vector control register is the single route 
I'd support given the choice.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D155269

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D155256: Add fs_base/gs_base support for Linux

[David Spickett via Phabricator via lldb-commits]

DavidSpickett added a comment.

Caveat: I have 0 prior knowledge about these registers.

What's the testing story here? I see one for fs_base on a live process but none 
for gs_base and neither for core files. If one test can hit all the code paths 
those would hit, then fine, but otherwise this needs more coverage.

Side note: please put a tag in the commit title like `[lldb][x86] ...`. It 
helps people like me who skim through the logs looking for build problems later.




Comment at: 
lldb/source/Plugins/Process/Utility/RegisterInfos_x86_64_with_base.h:71
+ {kind1, kind2, kind3, kind4,  
\
+  x86_64_with_base::lldb_##reg },  
 \
+  nullptr, nullptr, nullptr,   
\

Has this file been put through clang-format? These lines look off.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D155256

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D155269: [lldb][AArch64] Add SME streaming vector length pseudo register

[Jason Molenda via Phabricator via lldb-commits]

jasonmolenda added a comment.

In D155269#4509130 , @DavidSpickett 
wrote:

> 



> Ideally we would have as few routes to mode switch via the debugger as 
> possible. Writing to the streaming vector control register is the single 
> route I'd support given the choice.

I wonder about function calls when in streaming mode, where someone might not 
even realize they're in it.  Does lldb-server support QSaveRegisterState / 
QRestoreRegisterState around inferior function calls, or does lldb use the g/G 
packet (or write all the registers individually)?  g/G are probably going to 
fetch / write all the floating point registers and reset the mode if you did a 
function call while in streaming mode?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D155269

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D155107: Add support for llvm::MCInstPrinter::setPrintBranchImmAsAddress

[David Spickett via Phabricator via lldb-commits]

DavidSpickett added a comment.

Also you can test it as long as you put the right skipif annotations on it as 
it'll be architecture specific. Or use a corefile and just check that the 
backend for that architecture is enabled.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D155107

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D155269: [lldb][AArch64] Add SME streaming vector length pseudo register

[David Spickett via Phabricator via lldb-commits]

DavidSpickett added a comment.

I think in https://reviews.llvm.org/D154926, 
`lldb/test/API/commands/register/register/aarch64_sve_registers/rw_access_static_config/TestSVERegisters.py`
 addresses this. If what you mean is you are stopped in streaming mode, you 
evaluate an expression that may call a function which takes you into another 
mode.

If not, give me an example and I'll try to test it. This is the first I'm 
hearing of QSaveRegisterState / QRestoreRegisterState.

> g/G are probably going to fetch / write all the floating point registers and 
> reset the mode if you did a function call while in streaming mode?

We'd have to order them carefully I expect. Or say something like if we're 
restoring floating point and SVE registers, just ignore the floating point 
because we're about to supersede it.

I am not 100% sure that one cannot implement streaming SVE as a completely 
separate register state, I will be checking that today. If you can then it will 
complicate things in theory.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D155269

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] b71ac7e - [lldb/test] Fix command-disassemble-mixed.c

[Pavel Labath via lldb-commits]

Author: Pavel Labath
Date: 2023-07-18T10:17:09+02:00
New Revision: b71ac7eea4c5851203432dde94241d56301a9398

URL: 
https://github.com/llvm/llvm-project/commit/b71ac7eea4c5851203432dde94241d56301a9398
DIFF: 
https://github.com/llvm/llvm-project/commit/b71ac7eea4c5851203432dde94241d56301a9398.diff

LOG: [lldb/test] Fix command-disassemble-mixed.c

Add it to lit.local.cfg so that it's actually run, and change it to
(properly) use the %clang_host substitution.

Added: 


Modified: 
lldb/test/Shell/Commands/command-disassemble-mixed.c
lldb/test/Shell/Commands/lit.local.cfg

Removed: 




diff  --git a/lldb/test/Shell/Commands/command-disassemble-mixed.c 
b/lldb/test/Shell/Commands/command-disassemble-mixed.c
index d15832cabd8402..4c4f6408d9b20e 100644
--- a/lldb/test/Shell/Commands/command-disassemble-mixed.c
+++ b/lldb/test/Shell/Commands/command-disassemble-mixed.c
@@ -1,6 +1,6 @@
 // invalid mixed disassembly line
 
-// RUN: %clang -g %s -o %t
+// RUN: %clang_host -g %s -o %t
 // RUN: %lldb %t -o "dis -m -n main" -o "exit" | FileCheck %s
 
 // CHECK: int main

diff  --git a/lldb/test/Shell/Commands/lit.local.cfg 
b/lldb/test/Shell/Commands/lit.local.cfg
index b83eee443fccac..0a7d1d473b23f1 100644
--- a/lldb/test/Shell/Commands/lit.local.cfg
+++ b/lldb/test/Shell/Commands/lit.local.cfg
@@ -1,4 +1,4 @@
-config.suffixes = ['.s', '.test', '.yaml']
+config.suffixes = ['.s', '.test', '.yaml', '.c']
 
 # This is needed by command-target-create-resolve-exe.test
 config.substitutions.append(('%{PATH}', config.environment['PATH']))



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D155269: [lldb][AArch64] Add SME streaming vector length pseudo register

[Jason Molenda via Phabricator via lldb-commits]

jasonmolenda added a comment.

In D155269#4509364 , @DavidSpickett 
wrote:

> I think in https://reviews.llvm.org/D154926, 
> `lldb/test/API/commands/register/register/aarch64_sve_registers/rw_access_static_config/TestSVERegisters.py`
>  addresses this. If what you mean is you are stopped in streaming mode, you 
> evaluate an expression that may call a function which takes you into another 
> mode.

Oh it sounds like you've already been thinking about this, no worries.  I was 
thinking about the mere act of restoring the register state.  When lldb does a 
function call, it saves all of the register contents, does the function call, 
then restores the registers.  This was traditionally done with the g/G 
(read/write the entire register context), if that is supported by the stub.  
Alternatively registers can all be read/written individually.  As a 
simplification of all of this, and to avoid using g/G, we added 
QSaveRegisterState which tells the stub (debugserver etc) to save the current 
register context, and then after the inferior function call has completed, 
QRestoreRegisterState to restore them all.

In the process of restoring / writing the registers, I expect we will try to 
write the floating point register contents into the process which would drop it 
out of SSVE mode?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D155269

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D155269: [lldb][AArch64] Add SME streaming vector length pseudo register

[David Spickett via Phabricator via lldb-commits]

DavidSpickett added a comment.

> As a simplification of all of this, and to avoid using g/G, we added 
> QSaveRegisterState which tells the stub (debugserver etc) to save the current 
> register context, and then after the inferior function call has completed, 
> QRestoreRegisterState to restore them all.

Looks like we do support that in lldb-server, I just hadn't come across it 
because I've been down at the native process level. I've updated 
`ReadAllRegisterValues` down there so it will restore to whatever the saved 
mode was.

> In the process of restoring / writing the registers, I expect we will try to 
> write the floating point register contents into the process which would drop 
> it out of SSVE mode?

Right, yes it would if we follow this statement from the kernel docs:

  Note that when SME is present and streaming SVE mode is in use the FPSIMD 
subset of registers will be read via NT_ARM_SVE and NT_ARM_SVE writes will exit 
streaming mode in the target.

I am talking to our kernel folks to understand the background to that. I 
suspect that it may be the case that for example, writing to the bottom 128 
bits of streaming mode z0 may not be reflected in the SIMD unit's v0. Or at 
least, one could build a core that acted that way.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D155269

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D155269: [lldb][AArch64] Add SME streaming vector length pseudo register

[David Spickett via Phabricator via lldb-commits]

DavidSpickett added a comment.

> I suspect that it may be the case that for example, writing to the bottom 128 
> bits of streaming mode z0 may not be reflected in the SIMD unit's v0. Or at 
> least, one could build a core that acted that way.

But the user would be very confused by this given that if you are stopped here 
in streaming mode:

  mov v0.d[0] x0

That instruction would actually see the bottom 128 bits of streaming z0, even 
if elsewhere there is another, inactive v0 register in the core. If the user 
then does `register write v0 {}` I doubt they would expect it to mode 
switch and write to a whole different v0, it should update z0.

So even if on a hardware level this configuration is possible, I don't think 
it's good to have the debugger act this way. Better that we think of this from 
the perspective of a running instruction, what would it see and therefore what 
would the user expect to happen.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D155269

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D155117: Platform qemu-user: Build path to qemu automatically if not specified

[Ted Woodward via Phabricator via lldb-commits]

ted added a comment.

In D155117#4505538 , @labath wrote:

> I am wondering if we actually need the second step (the architecture setting) 
> here. The main reason it exists is the usage in `GetSupportedArchitectures` 
> (which is called before a target is created) it seems like the value derived 
> from the target should always be more correct. WDYT? What are the values you 
> get in steps 2 and 3 for your use case?

I don't think we need to specify the architecture, because I think we can 
always get it from the triple. There might be a case where someone is using a 
qemu that isn't named the same as the Triple ArchName, but that case could be 
covered by emulator-path.

In my case, I'm loading a target, then running, letting the platform get 
ArchName from the target Triple, then turning that into qemu-riscv32 (or 
riscv64), and launching that from my PATH.

An example (with my experimental RISC-V ABI plugin):

> bin/lldb

(lldb) platform select qemu-user

  Platform: qemu-user
Triple: x86_64-*-linux-gnu

OS Version: 5.4.0 (5.4.0-136-generic)

  Hostname: 127.0.0.1

WorkingDir: /local/mnt/ted/8.8/riscv

  Kernel: #153~18.04.1-Ubuntu SMP Wed Nov 30 15:47:57 UTC 2022

(lldb) file ~/lldb_test/factrv32
Current executable set to '/usr2/tedwood/lldb_test/factrv32' (riscv32).
(lldb) b main
Breakpoint 1: where = factrv32`main + 28 at factorial.c:32:8, address = 
0x000104ea
(lldb) r
Process 1 launched: '/usr2/tedwood/lldb_test/factrv32' (riscv32)
Process 1 stopped

- thread #1, stop reason = breakpoint 1.1 frame #0: 0x000104ea 
factrv32`main(argc=1, argv=0x408005a4) at factorial.c:32:8 29  } 30 
 */ 31

-> 32 base = 10;

  33
  34  printf("Factorial of %d is %d\n", base, factorial(base));
  35  return 0;

(lldb) target list
Current targets:

- target #0: /usr2/tedwood/lldb_test/factrv32 ( arch=riscv32-*-linux, 
platform=qemu-user, pid=1, state=stopped )

(lldb)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D155117

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D155269: [lldb][AArch64] Add SME streaming vector length pseudo register

[David Spickett via Phabricator via lldb-commits]

DavidSpickett added a comment.

> I am talking to our kernel folks to understand the background to that.

The result is that yes cores an implement it as separate state but as mentioned 
here, taking that into account in lldb would be rather confusing in 99% of 
situations. If we simply want to read what instructions in the current context 
will see, using the bottom 128 bits of the Z registers is fine.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D155269

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D155107: Add support for llvm::MCInstPrinter::setPrintBranchImmAsAddress

[Ted Woodward via Phabricator via lldb-commits]

ted added a comment.

In D155107#4504667 , @jasonmolenda 
wrote:

> Isn't it better to print branches within a function as an offset, given that 
> our disassembly format by default lists the offset of each instruction.  So 
> instead of looking for a 6-digit long hex address, you're looking for a 
> decimal offset in the output?  I'm not sure if you disagree with my opinion, 
> or if you have an example where it is clearer to have an address that I'm not 
> seeing.

llvm-objdump calls setPrintImmAsAddress(true). I think we should do the same. I 
also think that it looks better. @clayborg here is some riscv32 disassembly 
with and without the change:

with:

  (lldb) dis -n factorial
  factrv32`factorial:
  0x1047c <+0>:  addi   sp, sp, -32
  0x1047e <+2>:  sw ra, 28(sp)
  0x10480 <+4>:  sw s0, 24(sp)
  0x10482 <+6>:  addi   s0, sp, 32
  0x10484 <+8>:  sw a0, -16(s0)
  0x10488 <+12>: lw a0, -16(s0)
  0x1048c <+16>: li a1, 1
  0x1048e <+18>: beqa0, a1, 0x10496
  0x10492 <+22>: j  0x104a4
  0x10496 <+26>: j  0x1049a
  0x1049a <+30>: li a0, 1
  0x1049c <+32>: sw a0, -12(s0)
  0x104a0 <+36>: j  0x104c2
  0x104a4 <+40>: lw a0, -16(s0)
  0x104a8 <+44>: sw a0, -20(s0)
  0x104ac <+48>: addi   a0, a0, -1
  0x104ae <+50>: jal0x1047c
  0x104b0 <+52>: mv a1, a0
  0x104b2 <+54>: lw a0, -20(s0)
  0x104b6 <+58>: mula0, a0, a1
  0x104ba <+62>: sw a0, -12(s0)
  0x104be <+66>: j  0x104c2
  0x104c2 <+70>: lw a0, -12(s0)
  0x104c6 <+74>: lw ra, 28(sp)
  0x104c8 <+76>: lw s0, 24(sp)
  0x104ca <+78>: addi   sp, sp, 32
  0x104cc <+80>: ret

without:

  factrv32`factorial:
  0x1047c <+0>:  addi   sp, sp, -32
  0x1047e <+2>:  sw ra, 28(sp)
  0x10480 <+4>:  sw s0, 24(sp)
  0x10482 <+6>:  addi   s0, sp, 32
  0x10484 <+8>:  sw a0, -16(s0)
  0x10488 <+12>: lw a0, -16(s0)
  0x1048c <+16>: li a1, 1
  0x1048e <+18>: beqa0, a1, 8
  0x10492 <+22>: j  18
  0x10496 <+26>: j  4
  0x1049a <+30>: li a0, 1
  0x1049c <+32>: sw a0, -12(s0)
  0x104a0 <+36>: j  34
  0x104a4 <+40>: lw a0, -16(s0)
  0x104a8 <+44>: sw a0, -20(s0)
  0x104ac <+48>: addi   a0, a0, -1
  0x104ae <+50>: jal-50
  0x104b0 <+52>: mv a1, a0
  0x104b2 <+54>: lw a0, -20(s0)
  0x104b6 <+58>: mula0, a0, a1
  0x104ba <+62>: sw a0, -12(s0)
  0x104be <+66>: j  4
  0x104c2 <+70>: lw a0, -12(s0)
  0x104c6 <+74>: lw ra, 28(sp)
  0x104c8 <+76>: lw s0, 24(sp)
  0x104ca <+78>: addi   sp, sp, 32
  0x104cc <+80>: ret

Addresses 0x10492 and 0x10496 are jumps. It's much easier, IMO, to see what the 
targets are with the change than without.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D155107

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D154926: [lldb][AArch64] Add support for SME's SVE streaming mode registers

[David Spickett via Phabricator via lldb-commits]

DavidSpickett updated this revision to Diff 541520.
DavidSpickett added a comment.

Turns out I was misinterpreting this setence from the kernel docs:

  Note that when SME is present and streaming SVE mode is in use the FPSIMD 
subset of registers will be read via NT_ARM_SVE and NT_ARM_SVE writes will exit 
streaming mode in the target.

https://kernel.org/doc/html/v6.2/arm64/sve.html

I read this as "should be read" not, "will be read". The intent of the statement
is to make you aware that the register sets are connected in that one can effect
the other.

However, our strategy of using the bottom part of the Z registers to read the V
registers is still valid as long as we do not want to switch modes, which we 
never
do.

So I've done what Omair suggested and reverted to a single set of state for
the non-streaming and streaming modes. With m_sve_state to tell the difference.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D154926

Files:
  lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp
  lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.h
  lldb/source/Plugins/Process/Utility/RegisterInfoPOSIX_arm64.cpp
  lldb/source/Plugins/Process/Utility/RegisterInfoPOSIX_arm64.h
  
lldb/test/API/commands/register/register/aarch64_sve_registers/rw_access_dynamic_resize/TestSVEThreadedDynamic.py
  
lldb/test/API/commands/register/register/aarch64_sve_registers/rw_access_dynamic_resize/main.c
  
lldb/test/API/commands/register/register/aarch64_sve_registers/rw_access_static_config/Makefile
  
lldb/test/API/commands/register/register/aarch64_sve_registers/rw_access_static_config/TestSVERegisters.py
  
lldb/test/API/commands/register/register/aarch64_sve_registers/rw_access_static_config/main.c
  lldb/test/API/commands/register/register/aarch64_sve_simd_registers/Makefile
  
lldb/test/API/commands/register/register/aarch64_sve_simd_registers/TestSVESIMDRegisters.py
  lldb/test/API/commands/register/register/aarch64_sve_simd_registers/main.c

Index: lldb/test/API/commands/register/register/aarch64_sve_simd_registers/main.c
===
--- /dev/null
+++ lldb/test/API/commands/register/register/aarch64_sve_simd_registers/main.c
@@ -0,0 +1,108 @@
+#include 
+#include 
+
+void write_simd_regs() {
+#define WRITE_SIMD(NUM)\
+  asm volatile("MOV v" #NUM ".d[0], %0\n\t"\
+   "MOV v" #NUM ".d[1], %0\n\t" ::"r"(NUM))
+
+  WRITE_SIMD(0);
+  WRITE_SIMD(1);
+  WRITE_SIMD(2);
+  WRITE_SIMD(3);
+  WRITE_SIMD(4);
+  WRITE_SIMD(5);
+  WRITE_SIMD(6);
+  WRITE_SIMD(7);
+  WRITE_SIMD(8);
+  WRITE_SIMD(9);
+  WRITE_SIMD(10);
+  WRITE_SIMD(11);
+  WRITE_SIMD(12);
+  WRITE_SIMD(13);
+  WRITE_SIMD(14);
+  WRITE_SIMD(15);
+  WRITE_SIMD(16);
+  WRITE_SIMD(17);
+  WRITE_SIMD(18);
+  WRITE_SIMD(19);
+  WRITE_SIMD(20);
+  WRITE_SIMD(21);
+  WRITE_SIMD(22);
+  WRITE_SIMD(23);
+  WRITE_SIMD(24);
+  WRITE_SIMD(25);
+  WRITE_SIMD(26);
+  WRITE_SIMD(27);
+  WRITE_SIMD(28);
+  WRITE_SIMD(29);
+  WRITE_SIMD(30);
+  WRITE_SIMD(31);
+}
+
+unsigned verify_simd_regs() {
+  uint64_t got_low = 0;
+  uint64_t got_high = 0;
+  uint64_t target = 0;
+
+#define VERIFY_SIMD(NUM)   \
+  do { \
+got_low = 0;   \
+got_high = 0;  \
+asm volatile("MOV %0, v" #NUM ".d[0]\n\t"  \
+ "MOV %1, v" #NUM ".d[1]\n\t"  \
+ : "=r"(got_low), "=r"(got_high)); \
+target = NUM + 1;  \
+if ((got_low != target) || (got_high != target))   \
+  return 1;\
+  } while (0)
+
+  VERIFY_SIMD(0);
+  VERIFY_SIMD(1);
+  VERIFY_SIMD(2);
+  VERIFY_SIMD(3);
+  VERIFY_SIMD(4);
+  VERIFY_SIMD(5);
+  VERIFY_SIMD(6);
+  VERIFY_SIMD(7);
+  VERIFY_SIMD(8);
+  VERIFY_SIMD(9);
+  VERIFY_SIMD(10);
+  VERIFY_SIMD(11);
+  VERIFY_SIMD(12);
+  VERIFY_SIMD(13);
+  VERIFY_SIMD(14);
+  VERIFY_SIMD(15);
+  VERIFY_SIMD(16);
+  VERIFY_SIMD(17);
+  VERIFY_SIMD(18);
+  VERIFY_SIMD(19);
+  VERIFY_SIMD(20);
+  VERIFY_SIMD(21);
+  VERIFY_SIMD(22);
+  VERIFY_SIMD(23);
+  VERIFY_SIMD(24);
+  VERIFY_SIMD(25);
+  VERIFY_SIMD(26);
+  VERIFY_SIMD(27);
+  VERIFY_SIMD(28);
+  VERIFY_SIMD(29);
+  VERIFY_SIMD(30);
+  VERIFY_SIMD(31);
+
+  return 0;
+}
+
+int main() {
+#ifdef SSVE
+  asm volatile("msr  s0_3_c4_c7_3, xzr" /*smstart*/);
+#elif defined SVE
+  // Make the non-streaming SVE registers active.
+  asm volatile("cpy  z0.b, p0

[Lldb-commits] [PATCH] D154926: [lldb][AArch64] Add support for SME's SVE streaming mode registers

[David Spickett via Phabricator via lldb-commits]

DavidSpickett updated this revision to Diff 541526.
DavidSpickett added a comment.

Address some comments from the previous version.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D154926

Files:
  lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp
  lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.h
  lldb/source/Plugins/Process/Utility/RegisterInfoPOSIX_arm64.cpp
  lldb/source/Plugins/Process/Utility/RegisterInfoPOSIX_arm64.h
  
lldb/test/API/commands/register/register/aarch64_sve_registers/rw_access_dynamic_resize/TestSVEThreadedDynamic.py
  
lldb/test/API/commands/register/register/aarch64_sve_registers/rw_access_dynamic_resize/main.c
  
lldb/test/API/commands/register/register/aarch64_sve_registers/rw_access_static_config/Makefile
  
lldb/test/API/commands/register/register/aarch64_sve_registers/rw_access_static_config/TestSVERegisters.py
  
lldb/test/API/commands/register/register/aarch64_sve_registers/rw_access_static_config/main.c
  lldb/test/API/commands/register/register/aarch64_sve_simd_registers/Makefile
  
lldb/test/API/commands/register/register/aarch64_sve_simd_registers/TestSVESIMDRegisters.py
  lldb/test/API/commands/register/register/aarch64_sve_simd_registers/main.c

Index: lldb/test/API/commands/register/register/aarch64_sve_simd_registers/main.c
===
--- /dev/null
+++ lldb/test/API/commands/register/register/aarch64_sve_simd_registers/main.c
@@ -0,0 +1,108 @@
+#include 
+#include 
+
+void write_simd_regs() {
+#define WRITE_SIMD(NUM)\
+  asm volatile("MOV v" #NUM ".d[0], %0\n\t"\
+   "MOV v" #NUM ".d[1], %0\n\t" ::"r"(NUM))
+
+  WRITE_SIMD(0);
+  WRITE_SIMD(1);
+  WRITE_SIMD(2);
+  WRITE_SIMD(3);
+  WRITE_SIMD(4);
+  WRITE_SIMD(5);
+  WRITE_SIMD(6);
+  WRITE_SIMD(7);
+  WRITE_SIMD(8);
+  WRITE_SIMD(9);
+  WRITE_SIMD(10);
+  WRITE_SIMD(11);
+  WRITE_SIMD(12);
+  WRITE_SIMD(13);
+  WRITE_SIMD(14);
+  WRITE_SIMD(15);
+  WRITE_SIMD(16);
+  WRITE_SIMD(17);
+  WRITE_SIMD(18);
+  WRITE_SIMD(19);
+  WRITE_SIMD(20);
+  WRITE_SIMD(21);
+  WRITE_SIMD(22);
+  WRITE_SIMD(23);
+  WRITE_SIMD(24);
+  WRITE_SIMD(25);
+  WRITE_SIMD(26);
+  WRITE_SIMD(27);
+  WRITE_SIMD(28);
+  WRITE_SIMD(29);
+  WRITE_SIMD(30);
+  WRITE_SIMD(31);
+}
+
+unsigned verify_simd_regs() {
+  uint64_t got_low = 0;
+  uint64_t got_high = 0;
+  uint64_t target = 0;
+
+#define VERIFY_SIMD(NUM)   \
+  do { \
+got_low = 0;   \
+got_high = 0;  \
+asm volatile("MOV %0, v" #NUM ".d[0]\n\t"  \
+ "MOV %1, v" #NUM ".d[1]\n\t"  \
+ : "=r"(got_low), "=r"(got_high)); \
+target = NUM + 1;  \
+if ((got_low != target) || (got_high != target))   \
+  return 1;\
+  } while (0)
+
+  VERIFY_SIMD(0);
+  VERIFY_SIMD(1);
+  VERIFY_SIMD(2);
+  VERIFY_SIMD(3);
+  VERIFY_SIMD(4);
+  VERIFY_SIMD(5);
+  VERIFY_SIMD(6);
+  VERIFY_SIMD(7);
+  VERIFY_SIMD(8);
+  VERIFY_SIMD(9);
+  VERIFY_SIMD(10);
+  VERIFY_SIMD(11);
+  VERIFY_SIMD(12);
+  VERIFY_SIMD(13);
+  VERIFY_SIMD(14);
+  VERIFY_SIMD(15);
+  VERIFY_SIMD(16);
+  VERIFY_SIMD(17);
+  VERIFY_SIMD(18);
+  VERIFY_SIMD(19);
+  VERIFY_SIMD(20);
+  VERIFY_SIMD(21);
+  VERIFY_SIMD(22);
+  VERIFY_SIMD(23);
+  VERIFY_SIMD(24);
+  VERIFY_SIMD(25);
+  VERIFY_SIMD(26);
+  VERIFY_SIMD(27);
+  VERIFY_SIMD(28);
+  VERIFY_SIMD(29);
+  VERIFY_SIMD(30);
+  VERIFY_SIMD(31);
+
+  return 0;
+}
+
+int main() {
+#ifdef SSVE
+  asm volatile("msr  s0_3_c4_c7_3, xzr" /*smstart*/);
+#elif defined SVE
+  // Make the non-streaming SVE registers active.
+  asm volatile("cpy  z0.b, p0/z, #1\n\t");
+#endif
+  // else test plain SIMD access.
+
+  write_simd_regs();
+
+  return verify_simd_regs(); // Set a break point here.
+}
Index: lldb/test/API/commands/register/register/aarch64_sve_simd_registers/TestSVESIMDRegisters.py
===
--- /dev/null
+++ lldb/test/API/commands/register/register/aarch64_sve_simd_registers/TestSVESIMDRegisters.py
@@ -0,0 +1,108 @@
+"""
+Test that LLDB correctly reads and writes AArch64 SIMD registers in SVE,
+streaming SVE and normal SIMD modes.
+
+There are a few operating modes and we use different strategies for each:
+* Without SVE, in SIMD mode - read the SIMD regset.
+* With SVE, but SVE is inactive - read the SVE regset, but get SIMD da

[Lldb-commits] [PATCH] D154926: [lldb][AArch64] Add support for SME's SVE streaming mode registers

[David Spickett via Phabricator via lldb-commits]

DavidSpickett marked 3 inline comments as done.
DavidSpickett added inline comments.



Comment at: 
lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp:422
+m_sve_state = SVEState::Unknown;
+m_sve_state_data.Invalidate();
+m_ssve_state_data.Invalidate();

omjavaid wrote:
> shouldnt we also invalidate all other dynamic regsets here?
This is not needed anymore as we're no longer going to trigger a mode switch 
here. When we write to FPSIMD, we'll use the bottom 128 bits of the streaming 
mode Z register. Instead of going back to NT_ARM_SVE, which would cause a mode 
switch.



Comment at: 
lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.h:116
+
+  SVEStateData m_sve_state_data;
+  SVEStateData m_ssve_state_data;

omjavaid wrote:
> When we are in streaming mode normal state data will be invalid? If yes then 
> can we convert this into a pointer which should be pointing to a valid state 
> data based on current state?
I've reverted to the previous scheme.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D154926

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D154926: [lldb][AArch64] Add support for SME's SVE streaming mode registers

[David Spickett via Phabricator via lldb-commits]

DavidSpickett marked 4 inline comments as done.
DavidSpickett added inline comments.



Comment at: 
lldb/test/API/commands/register/register/aarch64_sve_simd_registers/TestSVESIMDRegisters.py:1
+"""
+Test that LLDB correctly reads and writes AArch64 SIMD registers in SVE,

I wrote this test when I thought we had to read/write SIMD via NT_ARM_SVE 
during streaming mode, so its main reason for existing has gone now.

However, I think it's worth keeping because no other test checks SIMD 
read/write as literally as this does. And if it turns out I did part of this 
incorrectly, we have tests to refer to.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D154926

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D154926: [lldb][AArch64] Add support for SME's SVE streaming mode registers

[David Spickett via Phabricator via lldb-commits]

DavidSpickett added a comment.

On the off chance anyone was going to try and run this, you'll need a kernel 
that includes 
https://lore.kernel.org/lkml/20230713-arm64-fix-sve-sme-vl-change-v1-3-129dd8611...@kernel.org/T/.
 This fixes a bug found while writing these tests.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D154926

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D154927: [lldb][AArch64] Add SME's streaming vector control register

[David Spickett via Phabricator via lldb-commits]

DavidSpickett updated this revision to Diff 541540.
DavidSpickett added a comment.

Rebase, fix typo.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D154927

Files:
  lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp
  lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.h
  lldb/source/Plugins/Process/Utility/RegisterInfoPOSIX_arm64.cpp
  lldb/source/Plugins/Process/Utility/RegisterInfoPOSIX_arm64.h
  
lldb/test/API/commands/register/register/aarch64_sve_registers/rw_access_static_config/TestSVERegisters.py

Index: lldb/test/API/commands/register/register/aarch64_sve_registers/rw_access_static_config/TestSVERegisters.py
===
--- lldb/test/API/commands/register/register/aarch64_sve_registers/rw_access_static_config/TestSVERegisters.py
+++ lldb/test/API/commands/register/register/aarch64_sve_registers/rw_access_static_config/TestSVERegisters.py
@@ -22,7 +22,12 @@
 reg_value.GetByteSize(), expected, 'Verify "%s" == %i' % (name, expected)
 )
 
-def check_sve_regs_read(self, z_reg_size):
+def check_sve_regs_read(self, z_reg_size, expected_mode):
+if self.isAArch64SME():
+expected_value = "1" if expected_mode == Mode.SSVE else "0"
+self.expect("register read svcr", substrs=[
+"0x000" + expected_value])
+
 p_reg_size = int(z_reg_size / 8)
 
 for i in range(32):
@@ -162,7 +167,7 @@
 
 vg_reg_value = sve_registers.GetChildMemberWithName("vg").GetValueAsUnsigned()
 z_reg_size = vg_reg_value * 8
-self.check_sve_regs_read(z_reg_size)
+self.check_sve_regs_read(z_reg_size, start_mode)
 
 # Evaluate simple expression and print function expr_eval_func address.
 self.expect("expression expr_eval_func", substrs=["= 0x"])
@@ -174,7 +179,7 @@
 
 # We called a jitted function above which must not have changed SVE
 # vector length or register values.
-self.check_sve_regs_read(z_reg_size)
+self.check_sve_regs_read(z_reg_size, start_mode)
 
 self.check_sve_regs_read_after_write(z_reg_size)
 
Index: lldb/source/Plugins/Process/Utility/RegisterInfoPOSIX_arm64.h
===
--- lldb/source/Plugins/Process/Utility/RegisterInfoPOSIX_arm64.h
+++ lldb/source/Plugins/Process/Utility/RegisterInfoPOSIX_arm64.h
@@ -106,6 +106,8 @@
 
   void AddRegSetTLS();
 
+  void AddRegSetSME();
+
   uint32_t ConfigureVectorLength(uint32_t sve_vq);
 
   bool VectorSizeIsValid(uint32_t vq) {
@@ -127,6 +129,7 @@
   bool IsPAuthReg(unsigned reg) const;
   bool IsMTEReg(unsigned reg) const;
   bool IsTLSReg(unsigned reg) const;
+  bool IsSMEReg(unsigned reg) const;
 
   uint32_t GetRegNumSVEZ0() const;
   uint32_t GetRegNumSVEFFR() const;
@@ -136,6 +139,7 @@
   uint32_t GetPAuthOffset() const;
   uint32_t GetMTEOffset() const;
   uint32_t GetTLSOffset() const;
+  uint32_t GetSMEOffset() const;
 
 private:
   typedef std::map>
@@ -163,6 +167,7 @@
   std::vector pauth_regnum_collection;
   std::vector m_mte_regnum_collection;
   std::vector m_tls_regnum_collection;
+  std::vector m_sme_regnum_collection;
 };
 
 #endif
Index: lldb/source/Plugins/Process/Utility/RegisterInfoPOSIX_arm64.cpp
===
--- lldb/source/Plugins/Process/Utility/RegisterInfoPOSIX_arm64.cpp
+++ lldb/source/Plugins/Process/Utility/RegisterInfoPOSIX_arm64.cpp
@@ -81,6 +81,9 @@
 static lldb_private::RegisterInfo g_register_infos_tls[] = {
 DEFINE_EXTENSION_REG(tpidr)};
 
+static lldb_private::RegisterInfo g_register_infos_sme[] = {
+DEFINE_EXTENSION_REG(svcr)};
+
 // Number of register sets provided by this context.
 enum {
   k_num_gpr_registers = gpr_w28 - gpr_x0 + 1,
@@ -89,6 +92,7 @@
   k_num_mte_register = 1,
   k_num_tls_register = 1,
   k_num_pauth_register = 2,
+  k_num_sme_register = 1,
   k_num_register_sets_default = 2,
   k_num_register_sets = 3
 };
@@ -196,6 +200,9 @@
 static const lldb_private::RegisterSet g_reg_set_tls_arm64 = {
 "Thread Local Storage Registers", "tls", k_num_tls_register, nullptr};
 
+static const lldb_private::RegisterSet g_reg_set_sme_arm64 = {
+"Scalable Matrix Extension Registers", "sme", k_num_sme_register, nullptr};
+
 RegisterInfoPOSIX_arm64::RegisterInfoPOSIX_arm64(
 const lldb_private::ArchSpec &target_arch, lldb_private::Flags opt_regsets)
 : lldb_private::RegisterInfoAndSetInterface(target_arch),
@@ -240,6 +247,9 @@
   // done as a dynamic set.
   AddRegSetTLS();
 
+  if (m_opt_regsets.AllSet(eRegsetMaskSSVE))
+AddRegSetSME();
+
   m_register_info_count = m_dynamic_reg_infos.size();
   m_register_info_p = m_dynamic_reg_infos.data();
   m_register_set_p = m_dynamic_reg_sets.data();
@@ -338,6 +348,21 @@
   m_dyn

[Lldb-commits] [PATCH] D154927: [lldb][AArch64] Add SME's streaming vector control register

[David Spickett via Phabricator via lldb-commits]

DavidSpickett marked an inline comment as done.
DavidSpickett added inline comments.



Comment at: 
lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp:1003
+  // Bit 2 indicates whether the array storage is active (not yet implemented).
+  m_sme_ctrl_reg = m_sve_state == SVEState::Streaming;
+  return {};

omjavaid wrote:
> is there a need here to check if m_sve_state is valid?
The only "invalid" state we can check at the moment is SVEState::Unknown. Which 
would translate here to a value of 0 in the register, which isn't incorrect, 
it's just pesimistic.

By the time you're printing this register you'd likely have checked the state 
fully. Maybe it would come back as SVEState::Disabled, but in that case you 
wouldn't have SME anyway so this register isn't available to you.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D154927

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D154927: [lldb][AArch64] Add SME's streaming vector control register

[David Spickett via Phabricator via lldb-commits]

DavidSpickett updated this revision to Diff 541542.
DavidSpickett marked an inline comment as done.
DavidSpickett added a comment.

Add missing newline.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D154927

Files:
  lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp
  lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.h
  lldb/source/Plugins/Process/Utility/RegisterInfoPOSIX_arm64.cpp
  lldb/source/Plugins/Process/Utility/RegisterInfoPOSIX_arm64.h
  
lldb/test/API/commands/register/register/aarch64_sve_registers/rw_access_static_config/TestSVERegisters.py

Index: lldb/test/API/commands/register/register/aarch64_sve_registers/rw_access_static_config/TestSVERegisters.py
===
--- lldb/test/API/commands/register/register/aarch64_sve_registers/rw_access_static_config/TestSVERegisters.py
+++ lldb/test/API/commands/register/register/aarch64_sve_registers/rw_access_static_config/TestSVERegisters.py
@@ -22,7 +22,12 @@
 reg_value.GetByteSize(), expected, 'Verify "%s" == %i' % (name, expected)
 )
 
-def check_sve_regs_read(self, z_reg_size):
+def check_sve_regs_read(self, z_reg_size, expected_mode):
+if self.isAArch64SME():
+expected_value = "1" if expected_mode == Mode.SSVE else "0"
+self.expect("register read svcr", substrs=[
+"0x000" + expected_value])
+
 p_reg_size = int(z_reg_size / 8)
 
 for i in range(32):
@@ -162,7 +167,7 @@
 
 vg_reg_value = sve_registers.GetChildMemberWithName("vg").GetValueAsUnsigned()
 z_reg_size = vg_reg_value * 8
-self.check_sve_regs_read(z_reg_size)
+self.check_sve_regs_read(z_reg_size, start_mode)
 
 # Evaluate simple expression and print function expr_eval_func address.
 self.expect("expression expr_eval_func", substrs=["= 0x"])
@@ -174,7 +179,7 @@
 
 # We called a jitted function above which must not have changed SVE
 # vector length or register values.
-self.check_sve_regs_read(z_reg_size)
+self.check_sve_regs_read(z_reg_size, start_mode)
 
 self.check_sve_regs_read_after_write(z_reg_size)
 
@@ -206,4 +211,4 @@
 @skipIf(archs=no_match(["aarch64"]))
 @skipIf(oslist=no_match(["linux"]))
 def test_registers_expr_read_write_ssve_sve(self):
-self.sve_registers_read_write_impl(Mode.SSVE, Mode.SVE)
\ No newline at end of file
+self.sve_registers_read_write_impl(Mode.SSVE, Mode.SVE)
Index: lldb/source/Plugins/Process/Utility/RegisterInfoPOSIX_arm64.h
===
--- lldb/source/Plugins/Process/Utility/RegisterInfoPOSIX_arm64.h
+++ lldb/source/Plugins/Process/Utility/RegisterInfoPOSIX_arm64.h
@@ -106,6 +106,8 @@
 
   void AddRegSetTLS();
 
+  void AddRegSetSME();
+
   uint32_t ConfigureVectorLength(uint32_t sve_vq);
 
   bool VectorSizeIsValid(uint32_t vq) {
@@ -127,6 +129,7 @@
   bool IsPAuthReg(unsigned reg) const;
   bool IsMTEReg(unsigned reg) const;
   bool IsTLSReg(unsigned reg) const;
+  bool IsSMEReg(unsigned reg) const;
 
   uint32_t GetRegNumSVEZ0() const;
   uint32_t GetRegNumSVEFFR() const;
@@ -136,6 +139,7 @@
   uint32_t GetPAuthOffset() const;
   uint32_t GetMTEOffset() const;
   uint32_t GetTLSOffset() const;
+  uint32_t GetSMEOffset() const;
 
 private:
   typedef std::map>
@@ -163,6 +167,7 @@
   std::vector pauth_regnum_collection;
   std::vector m_mte_regnum_collection;
   std::vector m_tls_regnum_collection;
+  std::vector m_sme_regnum_collection;
 };
 
 #endif
Index: lldb/source/Plugins/Process/Utility/RegisterInfoPOSIX_arm64.cpp
===
--- lldb/source/Plugins/Process/Utility/RegisterInfoPOSIX_arm64.cpp
+++ lldb/source/Plugins/Process/Utility/RegisterInfoPOSIX_arm64.cpp
@@ -81,6 +81,9 @@
 static lldb_private::RegisterInfo g_register_infos_tls[] = {
 DEFINE_EXTENSION_REG(tpidr)};
 
+static lldb_private::RegisterInfo g_register_infos_sme[] = {
+DEFINE_EXTENSION_REG(svcr)};
+
 // Number of register sets provided by this context.
 enum {
   k_num_gpr_registers = gpr_w28 - gpr_x0 + 1,
@@ -89,6 +92,7 @@
   k_num_mte_register = 1,
   k_num_tls_register = 1,
   k_num_pauth_register = 2,
+  k_num_sme_register = 1,
   k_num_register_sets_default = 2,
   k_num_register_sets = 3
 };
@@ -196,6 +200,9 @@
 static const lldb_private::RegisterSet g_reg_set_tls_arm64 = {
 "Thread Local Storage Registers", "tls", k_num_tls_register, nullptr};
 
+static const lldb_private::RegisterSet g_reg_set_sme_arm64 = {
+"Scalable Matrix Extension Registers", "sme", k_num_sme_register, nullptr};
+
 RegisterInfoPOSIX_arm64::RegisterInfoPOSIX_arm64(
 const lldb_private::ArchSpec &target_arch, lldb_private::Flags opt_regsets)
 : lldb_private::RegisterInfoAndSetInterfa

[Lldb-commits] [PATCH] D154927: [lldb][AArch64] Add SME's streaming vector control register

[David Spickett via Phabricator via lldb-commits]

DavidSpickett added inline comments.



Comment at: 
lldb/test/API/commands/register/register/aarch64_sve_registers/rw_access_static_config/TestSVERegisters.py:30
+"0x000" + expected_value])
+
 p_reg_size = int(z_reg_size / 8)

This is the check promised in the previous patch. It ensures that when we 
return from evaluating the expression we do restore the previous operating mode.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D154927

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D154930: [lldb][AArch64] Add the tpidr2 TLS register that comes with SME

[David Spickett via Phabricator via lldb-commits]

DavidSpickett updated this revision to Diff 541548.
DavidSpickett added a comment.

Add "Buffer" to method names.

enabled -> present in skipped messages. I realised that "enabled" is ambiguous
does it mean enabled in the CPU or in the process, present is more clearly
meaning is it on the CPU at all.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D154930

Files:
  lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp
  lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.h
  lldb/source/Plugins/Process/Utility/RegisterInfoPOSIX_arm64.cpp
  lldb/source/Plugins/Process/Utility/RegisterInfoPOSIX_arm64.h
  lldb/test/API/linux/aarch64/tls_register/Makefile
  lldb/test/API/linux/aarch64/tls_register/TestAArch64LinuxTLSRegister.py
  lldb/test/API/linux/aarch64/tls_register/main.c
  lldb/test/API/linux/aarch64/tls_registers/Makefile
  lldb/test/API/linux/aarch64/tls_registers/TestAArch64LinuxTLSRegisters.py
  lldb/test/API/linux/aarch64/tls_registers/main.c

Index: lldb/test/API/linux/aarch64/tls_registers/main.c
===
--- /dev/null
+++ lldb/test/API/linux/aarch64/tls_registers/main.c
@@ -0,0 +1,57 @@
+#include 
+#include 
+
+uint64_t get_tpidr(void) {
+  uint64_t tpidr = 0;
+  __asm__ volatile("mrs %0, tpidr_el0" : "=r"(tpidr));
+  return tpidr;
+}
+
+uint64_t get_tpidr2(void) {
+  uint64_t tpidr2 = 0;
+  // S3_3_C13_C0_5 means tpidr2, and will work with older tools.
+  __asm__ volatile("mrs %0, S3_3_C13_C0_5" : "=r"(tpidr2));
+  return tpidr2;
+}
+
+void set_tpidr(uint64_t value) {
+  __asm__ volatile("msr tpidr_el0, %0" ::"r"(value));
+}
+
+void set_tpidr2(uint64_t value) {
+  __asm__ volatile("msr S3_3_C13_C0_5, %0" ::"r"(value));
+}
+
+int main(int argc, char *argv[]) {
+  uint64_t (*getter)(void) = get_tpidr;
+  void (*setter)(uint64_t) = set_tpidr;
+
+  // Expect just the register number.
+  if (argc != 2)
+return 1;
+
+  switch (argv[1][0]) {
+  case '1':
+break;
+  case '2':
+getter = get_tpidr2;
+setter = set_tpidr2;
+break;
+  default:
+return 1;
+  }
+
+  uint64_t original = getter();
+  uint64_t pattern = 0x1122334455667788;
+  setter(pattern);
+
+  // Set break point at this line.
+  // lldb will now set its own pattern for us to find.
+
+  uint64_t new_value = getter();
+  volatile bool reg_was_set = new_value == 0x8877665544332211;
+
+  setter(original);
+
+  return 0; // Set break point 2 at this line.
+}
Index: lldb/test/API/linux/aarch64/tls_registers/TestAArch64LinuxTLSRegisters.py
===
--- lldb/test/API/linux/aarch64/tls_registers/TestAArch64LinuxTLSRegisters.py
+++ lldb/test/API/linux/aarch64/tls_registers/TestAArch64LinuxTLSRegisters.py
@@ -1,5 +1,5 @@
 """
-Test lldb's ability to read and write the AArch64 TLS register tpidr.
+Test lldb's ability to read and write the AArch64 TLS registers.
 """
 
 import lldb
@@ -8,12 +8,10 @@
 from lldbsuite.test import lldbutil
 
 
-class AArch64LinuxTLSRegister(TestBase):
+class AArch64LinuxTLSRegisters(TestBase):
 NO_DEBUG_INFO_TESTCASE = True
 
-@skipUnlessArch("aarch64")
-@skipUnlessPlatform(["linux"])
-def test_tls(self):
+def setup(self, register_name):
 self.build()
 self.runCmd("file " + self.getBuildArtifact("a.out"), CURRENT_EXECUTABLE_SET)
 
@@ -31,6 +29,8 @@
 num_expected_locations=1,
 )
 
+self.runCmd("settings set target.run-args {}".format(
+{"tpidr": 1, "tpidr2": 2}[register_name]))
 self.runCmd("run", RUN_SUCCEEDED)
 
 if self.process().GetState() == lldb.eStateExited:
@@ -42,19 +42,24 @@
 substrs=["stopped", "stop reason = breakpoint"],
 )
 
+def check_tls_reg(self, register_name):
+self.setup(register_name)
+
 # Since we can't predict what the value will be, the program has set
 # a target value for us to find.
 
 regs = self.thread().GetSelectedFrame().GetRegisters()
 tls_regs = regs.GetFirstValueByName("Thread Local Storage Registers")
 self.assertTrue(tls_regs.IsValid(), "No TLS registers found.")
-tpidr = tls_regs.GetChildMemberWithName("tpidr")
-self.assertTrue(tpidr.IsValid(), "No tpidr register found.")
+tls_reg = tls_regs.GetChildMemberWithName(register_name)
+self.assertTrue(tls_reg.IsValid(), "{} register not found.".format(
+register_name))
 
-self.assertEqual(tpidr.GetValueAsUnsigned(), 0x1122334455667788)
+self.assertEqual(tls_reg.GetValueAsUnsigned(), 0x1122334455667788)
 
 # Set our own value for the program to find.
-self.expect("register write tpidr 0x{:x}".format(0x8877665544332211))
+self.expect("register write {} 0x{:x}".format(
+register_name, 0x8877665544332211))
 self.expect("conti

[Lldb-commits] [PATCH] D154930: [lldb][AArch64] Add the tpidr2 TLS register that comes with SME

[David Spickett via Phabricator via lldb-commits]

DavidSpickett updated this revision to Diff 541570.
DavidSpickett added a comment.

Note the behaviour of tpidr2 on a system without SME.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D154930

Files:
  lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp
  lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.h
  lldb/source/Plugins/Process/Utility/RegisterInfoPOSIX_arm64.cpp
  lldb/source/Plugins/Process/Utility/RegisterInfoPOSIX_arm64.h
  lldb/test/API/linux/aarch64/tls_register/Makefile
  lldb/test/API/linux/aarch64/tls_register/TestAArch64LinuxTLSRegister.py
  lldb/test/API/linux/aarch64/tls_register/main.c
  lldb/test/API/linux/aarch64/tls_registers/Makefile
  lldb/test/API/linux/aarch64/tls_registers/TestAArch64LinuxTLSRegisters.py
  lldb/test/API/linux/aarch64/tls_registers/main.c

Index: lldb/test/API/linux/aarch64/tls_registers/main.c
===
--- /dev/null
+++ lldb/test/API/linux/aarch64/tls_registers/main.c
@@ -0,0 +1,59 @@
+#include 
+#include 
+
+uint64_t get_tpidr(void) {
+  uint64_t tpidr = 0;
+  __asm__ volatile("mrs %0, tpidr_el0" : "=r"(tpidr));
+  return tpidr;
+}
+
+uint64_t get_tpidr2(void) {
+  uint64_t tpidr2 = 0;
+  // S3_3_C13_C0_5 means tpidr2, and will work with older tools.
+  __asm__ volatile("mrs %0, S3_3_C13_C0_5" : "=r"(tpidr2));
+  return tpidr2;
+}
+
+void set_tpidr(uint64_t value) {
+  __asm__ volatile("msr tpidr_el0, %0" ::"r"(value));
+}
+
+void set_tpidr2(uint64_t value) {
+  __asm__ volatile("msr S3_3_C13_C0_5, %0" ::"r"(value));
+}
+
+int main(int argc, char *argv[]) {
+  uint64_t (*getter)(void) = get_tpidr;
+  void (*setter)(uint64_t) = set_tpidr;
+
+  // Expect just the register number.
+  if (argc != 2)
+return 1;
+
+  // Note that accessing tpidr2 on a system without it will SIGILL. That is why
+  // we have this option instead of trying to set both regardless.
+  switch (argv[1][0]) {
+  case '1':
+break;
+  case '2':
+getter = get_tpidr2;
+setter = set_tpidr2;
+break;
+  default:
+return 1;
+  }
+
+  uint64_t original = getter();
+  uint64_t pattern = 0x1122334455667788;
+  setter(pattern);
+
+  // Set break point at this line.
+  // lldb will now set its own pattern for us to find.
+
+  uint64_t new_value = getter();
+  volatile bool reg_was_set = new_value == 0x8877665544332211;
+
+  setter(original);
+
+  return 0; // Set break point 2 at this line.
+}
Index: lldb/test/API/linux/aarch64/tls_registers/TestAArch64LinuxTLSRegisters.py
===
--- lldb/test/API/linux/aarch64/tls_registers/TestAArch64LinuxTLSRegisters.py
+++ lldb/test/API/linux/aarch64/tls_registers/TestAArch64LinuxTLSRegisters.py
@@ -1,5 +1,5 @@
 """
-Test lldb's ability to read and write the AArch64 TLS register tpidr.
+Test lldb's ability to read and write the AArch64 TLS registers.
 """
 
 import lldb
@@ -8,12 +8,10 @@
 from lldbsuite.test import lldbutil
 
 
-class AArch64LinuxTLSRegister(TestBase):
+class AArch64LinuxTLSRegisters(TestBase):
 NO_DEBUG_INFO_TESTCASE = True
 
-@skipUnlessArch("aarch64")
-@skipUnlessPlatform(["linux"])
-def test_tls(self):
+def setup(self, register_name):
 self.build()
 self.runCmd("file " + self.getBuildArtifact("a.out"), CURRENT_EXECUTABLE_SET)
 
@@ -31,6 +29,8 @@
 num_expected_locations=1,
 )
 
+self.runCmd("settings set target.run-args {}".format(
+{"tpidr": 1, "tpidr2": 2}[register_name]))
 self.runCmd("run", RUN_SUCCEEDED)
 
 if self.process().GetState() == lldb.eStateExited:
@@ -42,19 +42,24 @@
 substrs=["stopped", "stop reason = breakpoint"],
 )
 
+def check_tls_reg(self, register_name):
+self.setup(register_name)
+
 # Since we can't predict what the value will be, the program has set
 # a target value for us to find.
 
 regs = self.thread().GetSelectedFrame().GetRegisters()
 tls_regs = regs.GetFirstValueByName("Thread Local Storage Registers")
 self.assertTrue(tls_regs.IsValid(), "No TLS registers found.")
-tpidr = tls_regs.GetChildMemberWithName("tpidr")
-self.assertTrue(tpidr.IsValid(), "No tpidr register found.")
+tls_reg = tls_regs.GetChildMemberWithName(register_name)
+self.assertTrue(tls_reg.IsValid(), "{} register not found.".format(
+register_name))
 
-self.assertEqual(tpidr.GetValueAsUnsigned(), 0x1122334455667788)
+self.assertEqual(tls_reg.GetValueAsUnsigned(), 0x1122334455667788)
 
 # Set our own value for the program to find.
-self.expect("register write tpidr 0x{:x}".format(0x8877665544332211))
+self.expect("register write {} 0x{:x}".format(
+register_name, 0x8877665544332211))
 self.expect("continue")
 
   

[Lldb-commits] [PATCH] D154930: [lldb][AArch64] Add the tpidr2 TLS register that comes with SME

[David Spickett via Phabricator via lldb-commits]

DavidSpickett added inline comments.



Comment at: 
lldb/test/API/linux/aarch64/tls_registers/TestAArch64LinuxTLSRegisters.py:73
+
+@skipUnlessArch("aarch64")
+@skipUnlessPlatform(["linux"])

DavidSpickett wrote:
> omjavaid wrote:
> > These three tests have a lot of commonalities may be merge them into one 
> > testing the whole logic. Doesn't look like we are getting much out of 
> > emitting three tests from this fairly basic test class.
> The tradeoff is execution time vs. a HWCAP check in the program file and a 
> bunch of ifs in Python.
> 
> Let me see what I can do, but I'm leaning toward the implementation 
> complexity outweighing the performance gained.
I've looked into this. A major thing to note is that reading tpidr2 (or rather, 
the system register operands that represent it) on a system without SME will 
SIGILL. I tried this on a Mountain Jade system.

This means we cannot have the program file set both regardless and only test 
what we expect to be present. We still need the option to the program and still 
need to run it again each time we want to check a different register.

(I guess you could have an option for each register, but again we're trading 
complexity here)

That does mean that the program file is the same between the 3 tests, so you 
could merge them to only build once. That said I think the time saved 
rebuilding a small example is not much when put against the pile of `if` you 
would need to use to merge the 3 tests into one (and therefore have to unpick 
later).

I could merge `test_tpidr2_no_sme` into `test_tpidr` but again, I'd rather have 
the clear separation of it being its own test than add an `if not 
self.isAArch64SME...` in there.

If I'm addressing completely different aspects than you had considered here, 
let me know and I'll fix those.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D154930

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D155256: Add fs_base/gs_base support for Linux

[jeffrey tan via Phabricator via lldb-commits]

yinghuitan updated this revision to Diff 541597.
yinghuitan added a comment.

Address review comments:

- clang-format
- Add coredump tests
- Add test against gs_base register


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D155256

Files:
  lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_x86_64.cpp
  lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_x86_64.h
  lldb/source/Plugins/Process/Utility/CMakeLists.txt
  lldb/source/Plugins/Process/Utility/NativeRegisterContextDBReg_x86.h
  lldb/source/Plugins/Process/Utility/RegisterContextLinux_x86_64.cpp
  lldb/source/Plugins/Process/Utility/RegisterContextPOSIX_x86.cpp
  lldb/source/Plugins/Process/Utility/RegisterContextPOSIX_x86.h
  lldb/source/Plugins/Process/Utility/RegisterContext_x86.h
  lldb/source/Plugins/Process/Utility/RegisterInfos_x86_64_with_base.h
  lldb/source/Plugins/Process/Utility/RegisterInfos_x86_64_with_base_shared.cpp
  lldb/source/Plugins/Process/Utility/RegisterInfos_x86_64_with_base_shared.h
  lldb/source/Plugins/Process/Utility/lldb-x86-register-enums.h
  lldb/source/Plugins/Process/elf-core/CMakeLists.txt
  lldb/source/Plugins/Process/elf-core/RegisterContextLinuxCore_x86_64.cpp
  lldb/source/Plugins/Process/elf-core/RegisterContextLinuxCore_x86_64.h
  lldb/source/Plugins/Process/elf-core/ThreadElfCore.cpp
  lldb/test/API/commands/register/register/register_command/TestRegisters.py
  lldb/test/API/functionalities/postmortem/elf-core/TestLinuxCore.py

Index: lldb/test/API/functionalities/postmortem/elf-core/TestLinuxCore.py
===
--- lldb/test/API/functionalities/postmortem/elf-core/TestLinuxCore.py
+++ lldb/test/API/functionalities/postmortem/elf-core/TestLinuxCore.py
@@ -623,6 +623,38 @@
 
 self.expect("register read --all")
 
+@skipUnlessPlatform(["linux"])
+@skipIf(archs=no_match(["x86_64"]))
+def test_fs_gs_base(self):
+"""Tests fs_base/gs_base registers can be read from linux coredump."""
+target = self.dbg.CreateTarget(None)
+process = target.LoadCore("linux-x86_64.core")
+self.assertTrue(process, PROCESS_IS_VALID)
+
+error = lldb.SBError()
+bytesread = process.ReadMemory(0x400FF0, 20, error)
+
+thread = process.GetThreadAtIndex(0)
+self.assertTrue(thread.IsValid(), "current thread is valid")
+
+current_frame = thread.GetFrameAtIndex(0)
+self.assertTrue(current_frame.IsValid(), "current frame is valid")
+
+reg_fs_base = current_frame.FindRegister("fs_base")
+reg_gs_base = current_frame.FindRegister("gs_base")
+self.assertTrue(reg_fs_base.IsValid(), "fs_base is not available")
+self.assertTrue(reg_gs_base.IsValid(), "gs_base is not available")
+# The fs_base/gs_base registers in linux-x86_64.core are both zero.
+# Use "eu-readelf -n linux-x86_64.core" to verify.
+self.assertEqual(
+reg_fs_base.GetValueAsSigned(-1), 0, f"fs_base read is different from expected"
+)
+self.assertEqual(
+reg_gs_base.GetValueAsSigned(-1), 0, f"gs_base read is different from expected"
+)
+self.dbg.DeleteTarget(target)
+
+
 def check_memory_regions(self, process, region_count):
 region_list = process.GetMemoryRegions()
 self.assertEqual(region_list.GetSize(), region_count)
Index: lldb/test/API/commands/register/register/register_command/TestRegisters.py
===
--- lldb/test/API/commands/register/register/register_command/TestRegisters.py
+++ lldb/test/API/commands/register/register/register_command/TestRegisters.py
@@ -606,3 +606,44 @@
 
 # This has an alternative name according to the ABI.
 self.expect("register info x30", substrs=["Name: lr (x30)"])
+
+@skipUnlessPlatform(["linux"])
+@skipIf(archs=no_match(["x86_64"]))
+def test_fs_gs_base(self):
+"""Tests fs_base register can be read and equals to pthread_self() return value."""
+self.build()
+target = self.createTestTarget()
+# Launch the process and stop.
+self.expect("run", PROCESS_STOPPED, substrs=["stopped"])
+
+process = target.GetProcess()
+
+thread = process.GetThreadAtIndex(0)
+self.assertTrue(thread.IsValid(), "current thread is valid")
+
+current_frame = thread.GetFrameAtIndex(0)
+self.assertTrue(current_frame.IsValid(), "current frame is valid")
+
+reg_fs_base = current_frame.FindRegister("fs_base")
+self.assertTrue(reg_fs_base.IsValid(), "fs_base is not available")
+reg_gs_base = current_frame.FindRegister("gs_base")
+self.assertTrue(reg_gs_base.IsValid(), "gs_base is not available")
+self.assertEqual(
+reg_gs_base.GetValueAsSigned(-1), 0, f"gs_base should be zero"
+)

[Lldb-commits] [PATCH] D155107: Add support for llvm::MCInstPrinter::setPrintBranchImmAsAddress

[Greg Clayton via Phabricator via lldb-commits]

clayborg added a comment.

In D155107#4510539 , @ted wrote:

> In D155107#4504667 , @jasonmolenda 
> wrote:
>
>> Isn't it better to print branches within a function as an offset, given that 
>> our disassembly format by default lists the offset of each instruction.  So 
>> instead of looking for a 6-digit long hex address, you're looking for a 
>> decimal offset in the output?  I'm not sure if you disagree with my opinion, 
>> or if you have an example where it is clearer to have an address that I'm 
>> not seeing.
>
> llvm-objdump calls setPrintImmAsAddress(true). I think we should do the same. 
> I also think that it looks better. @clayborg here is some riscv32 disassembly 
> with and without the change:
>
> with:
>
>   (lldb) dis -n factorial
>   factrv32`factorial:
>   0x1047c <+0>:  addi   sp, sp, -32
>   0x1047e <+2>:  sw ra, 28(sp)
>   0x10480 <+4>:  sw s0, 24(sp)
>   0x10482 <+6>:  addi   s0, sp, 32
>   0x10484 <+8>:  sw a0, -16(s0)
>   0x10488 <+12>: lw a0, -16(s0)
>   0x1048c <+16>: li a1, 1
>   0x1048e <+18>: beqa0, a1, 0x10496
>   0x10492 <+22>: j  0x104a4
>   0x10496 <+26>: j  0x1049a
>   0x1049a <+30>: li a0, 1
>   0x1049c <+32>: sw a0, -12(s0)
>   0x104a0 <+36>: j  0x104c2
>   0x104a4 <+40>: lw a0, -16(s0)
>   0x104a8 <+44>: sw a0, -20(s0)
>   0x104ac <+48>: addi   a0, a0, -1
>   0x104ae <+50>: jal0x1047c
>   0x104b0 <+52>: mv a1, a0
>   0x104b2 <+54>: lw a0, -20(s0)
>   0x104b6 <+58>: mula0, a0, a1
>   0x104ba <+62>: sw a0, -12(s0)
>   0x104be <+66>: j  0x104c2
>   0x104c2 <+70>: lw a0, -12(s0)
>   0x104c6 <+74>: lw ra, 28(sp)
>   0x104c8 <+76>: lw s0, 24(sp)
>   0x104ca <+78>: addi   sp, sp, 32
>   0x104cc <+80>: ret
>
> without:
>
>   factrv32`factorial:
>   0x1047c <+0>:  addi   sp, sp, -32
>   0x1047e <+2>:  sw ra, 28(sp)
>   0x10480 <+4>:  sw s0, 24(sp)
>   0x10482 <+6>:  addi   s0, sp, 32
>   0x10484 <+8>:  sw a0, -16(s0)
>   0x10488 <+12>: lw a0, -16(s0)
>   0x1048c <+16>: li a1, 1
>   0x1048e <+18>: beqa0, a1, 8
>   0x10492 <+22>: j  18
>   0x10496 <+26>: j  4
>   0x1049a <+30>: li a0, 1
>   0x1049c <+32>: sw a0, -12(s0)
>   0x104a0 <+36>: j  34
>   0x104a4 <+40>: lw a0, -16(s0)
>   0x104a8 <+44>: sw a0, -20(s0)
>   0x104ac <+48>: addi   a0, a0, -1
>   0x104ae <+50>: jal-50
>   0x104b0 <+52>: mv a1, a0
>   0x104b2 <+54>: lw a0, -20(s0)
>   0x104b6 <+58>: mula0, a0, a1
>   0x104ba <+62>: sw a0, -12(s0)
>   0x104be <+66>: j  4
>   0x104c2 <+70>: lw a0, -12(s0)
>   0x104c6 <+74>: lw ra, 28(sp)
>   0x104c8 <+76>: lw s0, 24(sp)
>   0x104ca <+78>: addi   sp, sp, 32
>   0x104cc <+80>: ret

Definitely looks nicer with the full addresses IMHO.

> Addresses 0x10492 and 0x10496 are jumps. It's much easier, IMO, to see what 
> the targets are with the change than without.
>
> Ultimately, I'd like to see more info for branch/jump targets, like x64 has, 
> but that's another patch:
>
>   0x40053f <+31>: jmp0x400563  ; <+67> at factorial.c:10

We already do this kind of comment when we know that there is an address that 
is being referred to in the disassembly. Maybe the "I know this opcode has an 
address" is not working for RISCV? I am not sure if there is a disassembler 
issue for RISCV only or what is going on. Check out the current arm64 
disassembly of a simple. main function from the current lldb:

  (lldb) disassemble --name main
  a.out`main:
  a.out[0x12e98] <+0>:   subsp, sp, #0x70
  a.out[0x12e9c] <+4>:   stpx29, x30, [sp, #0x60]
  a.out[0x12ea0] <+8>:   addx29, sp, #0x60
  a.out[0x12ea4] <+12>:  stur   wzr, [x29, #-0xc]
  a.out[0x12ea8] <+16>:  stur   w0, [x29, #-0x10]
  a.out[0x12eac] <+20>:  stur   x1, [x29, #-0x18]
  a.out[0x12eb0] <+24>:  stur   wzr, [x29, #-0x1c]
  a.out[0x12eb4] <+28>:  movw8, #0x2  ; =2 
  a.out[0x12eb8] <+32>:  strw8, [sp, #0x4]
  a.out[0x12ebc] <+36>:  stur   w8, [x29, #-0x4]
  a.out[0x12ec0] <+40>:  movw8, #0x3  ; =3 
  a.out[0x12ec4] <+44>:  stur   w8, [x29, #-0x8]
  a.out[0x12ec8] <+48>:  ldur   w8, [x29, #-0x4]
  a.out[0x12ecc] <+52>:  ldur   w9, [x29, #-0x8]
  a.out[0x12ed0] <+56>:  mulw8, w8, w9
  a.out[0x12ed4] <+60>:  stur   w8, [x29, #-0x20]
  a.out[0x12ed8] <+64>:  subx2, x29, #0x2c
  a.out[0x12edc] <+68>:  movw8, #0x1  ; =1 
  a.out[0x12ee0] <+72>:  stur   w8, [x29, #-0x2c]
  a.out[0x12ee4] <+76>:  subx0, x29, #0x28
  a.out[0x12ee8] <+80>:  adrp   x1, 0
  a.out[0x12eec] <+84>:  add

[Lldb-commits] [PATCH] D155117: Platform qemu-user: Build path to qemu automatically if not specified

[Ted Woodward via Phabricator via lldb-commits]

ted added a comment.

As for the GetSupportedArchitectures case, downstream I left in the code that 
checks the property, but changed the return {}; to

  return {ArchSpec(llvm::Triple("riscv32-unknown-linux")),
  ArchSpec(llvm::Triple("riscv64-unknown-linux"))};

I don't think we want that upstream, but downstream the only time we're 
targetting riscv linux is when using qemu.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D155117

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D155248: [lldb-vscode] Creating a new flag for adjusting the behavior of evaluation repl expressions to allow users to more easily invoke lldb commands.

[John Harrison via Phabricator via lldb-commits]

ashgti updated this revision to Diff 541728.
ashgti added a comment.

Updating patch.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D155248

Files:
  lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/vscode.py
  lldb/test/API/tools/lldb-vscode/completions/TestVSCode_completions.py
  lldb/test/API/tools/lldb-vscode/console/TestVSCode_console.py
  lldb/test/API/tools/lldb-vscode/module/TestVSCode_module.py
  lldb/test/API/tools/lldb-vscode/startDebugging/TestVSCode_startDebugging.py
  lldb/tools/lldb-vscode/Options.td
  lldb/tools/lldb-vscode/README.md
  lldb/tools/lldb-vscode/VSCode.cpp
  lldb/tools/lldb-vscode/VSCode.h
  lldb/tools/lldb-vscode/lldb-vscode.cpp

Index: lldb/tools/lldb-vscode/lldb-vscode.cpp
===
--- lldb/tools/lldb-vscode/lldb-vscode.cpp
+++ lldb/tools/lldb-vscode/lldb-vscode.cpp
@@ -1065,50 +1065,62 @@
   FillResponse(request, response);
   llvm::json::Object body;
   auto arguments = request.getObject("arguments");
-  std::string text = std::string(GetString(arguments, "text"));
+
+  // If we have a frame, try to set the context for variable completions.
+  lldb::SBFrame frame = g_vsc.GetLLDBFrame(*arguments);
+  if (frame.IsValid()) {
+frame.GetThread().GetProcess().SetSelectedThread(frame.GetThread());
+frame.GetThread().SetSelectedFrame(frame.GetFrameID());
+  }
+
+  std::string text = GetString(arguments, "text").str();
   auto original_column = GetSigned(arguments, "column", text.size());
-  auto actual_column = original_column - 1;
+  auto original_line = GetSigned(arguments, "line", 1);
+  auto offset = original_column - 1;
+  if (original_line > 1) {
+llvm::SmallVector<::llvm::StringRef, 2> lines;
+llvm::StringRef(text).split(lines, '\n');
+for (int i = 0; i < original_line - 1; i++) {
+  offset += lines[i].size();
+}
+  }
   llvm::json::Array targets;
-  // NOTE: the 'line' argument is not needed, as multiline expressions
-  // work well already
-  // TODO: support frameID. Currently
-  // g_vsc.debugger.GetCommandInterpreter().HandleCompletionWithDescriptions
-  // is frame-unaware.
-
-  if (!text.empty() && text[0] == '`') {
-text = text.substr(1);
-actual_column--;
-  } else {
+
+  if (g_vsc.DetectExpressionContext(frame, text) ==
+  ExpressionContext::Variable) {
 char command[] = "expression -- ";
 text = command + text;
-actual_column += strlen(command);
+offset += strlen(command);
   }
   lldb::SBStringList matches;
   lldb::SBStringList descriptions;
-  g_vsc.debugger.GetCommandInterpreter().HandleCompletionWithDescriptions(
-  text.c_str(), actual_column, 0, -1, matches, descriptions);
-  size_t count = std::min((uint32_t)100, matches.GetSize());
-  targets.reserve(count);
-  for (size_t i = 0; i < count; i++) {
-std::string match = matches.GetStringAtIndex(i);
-std::string description = descriptions.GetStringAtIndex(i);
-
-llvm::json::Object item;
-
-llvm::StringRef match_ref = match;
-for (llvm::StringRef commit_point : {".", "->"}) {
-  if (match_ref.contains(commit_point)) {
-match_ref = match_ref.rsplit(commit_point).second;
+
+  if (g_vsc.debugger.GetCommandInterpreter().HandleCompletionWithDescriptions(
+  text.c_str(), offset, 0, 100, matches, descriptions)) {
+// The first element is the common substring after the cursor position for
+// all the matches. The rest of the elements are the matches so ignore the
+// first result.
+targets.reserve(matches.GetSize() - 1);
+for (size_t i = 1; i < matches.GetSize(); i++) {
+  std::string match = matches.GetStringAtIndex(i);
+  std::string description = descriptions.GetStringAtIndex(i);
+
+  llvm::json::Object item;
+  llvm::StringRef match_ref = match;
+  for (llvm::StringRef commit_point : {".", "->"}) {
+if (match_ref.contains(commit_point)) {
+  match_ref = match_ref.rsplit(commit_point).second;
+}
   }
-}
-EmplaceSafeString(item, "text", match_ref);
+  EmplaceSafeString(item, "text", match_ref);
 
-if (description.empty())
-  EmplaceSafeString(item, "label", match);
-else
-  EmplaceSafeString(item, "label", match + " -- " + description);
+  if (description.empty())
+EmplaceSafeString(item, "label", match);
+  else
+EmplaceSafeString(item, "label", match + " -- " + description);
 
-targets.emplace_back(std::move(item));
+  targets.emplace_back(std::move(item));
+}
   }
 
   body.try_emplace("targets", std::move(targets));
@@ -1223,12 +1235,17 @@
   llvm::json::Object body;
   auto arguments = request.getObject("arguments");
   lldb::SBFrame frame = g_vsc.GetLLDBFrame(*arguments);
-  const auto expression = GetString(arguments, "expression");
+  std::string expression = GetString(arguments, "expression").str();
   llvm::StringRef context = 

[Lldb-commits] [PATCH] D155107: Add support for llvm::MCInstPrinter::setPrintBranchImmAsAddress

[Ted Woodward via Phabricator via lldb-commits]

ted added a comment.

In D155107#4511967 , @clayborg wrote:

> Looks like other disassemblers already show full addresses for the branches 
> and calls (at least arm64 does from my output above), so not sure why RISCV 
> would require this setting, but x86_64 and arm64 wouldn't because it is 
> already working that way??

llvm/lib/Target/RISCV/MCTargetDesc/RISCVInstPrinter.cpp has this code, 
specifically using PrintBranchImmAsAddress to gate this behavior:

  void RISCVInstPrinter::printBranchOperand(const MCInst *MI, uint64_t Address,
unsigned OpNo,
const MCSubtargetInfo &STI,
raw_ostream &O) {
const MCOperand &MO = MI->getOperand(OpNo);
if (!MO.isImm())
  return printOperand(MI, OpNo, STI, O);
  
if (PrintBranchImmAsAddress) {
  uint64_t Target = Address + MO.getImm();
  if (!STI.hasFeature(RISCV::Feature64Bit))
Target &= 0x;
  O << formatHex(Target);
} else {
  O << MO.getImm();
}
  }


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D155107

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D155653: [lldb][NFCI] Add some missing SB class forward declarations

[Alex Langford via Phabricator via lldb-commits]

bulbazord created this revision.
bulbazord added reviewers: JDevlieghere, mib, jingham.
Herald added a project: All.
bulbazord requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

I noticed these were missing.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D155653

Files:
  lldb/include/lldb/API/SBDefines.h


Index: lldb/include/lldb/API/SBDefines.h
===
--- lldb/include/lldb/API/SBDefines.h
+++ lldb/include/lldb/API/SBDefines.h
@@ -43,6 +43,7 @@
 namespace lldb {
 
 class LLDB_API SBAddress;
+class LLDB_API SBAttachInfo;
 class LLDB_API SBBlock;
 class LLDB_API SBBreakpoint;
 class LLDB_API SBBreakpointLocation;
@@ -84,10 +85,12 @@
 class LLDB_API SBModuleSpec;
 class LLDB_API SBModuleSpecList;
 class LLDB_API SBPlatform;
+class LLDB_API SBPlatformShellCommand;
 class LLDB_API SBProcess;
 class LLDB_API SBProcessInfo;
 class LLDB_API SBQueue;
 class LLDB_API SBQueueItem;
+class LLDB_API SBReproducer;
 class LLDB_API SBSection;
 class LLDB_API SBSourceManager;
 class LLDB_API SBStream;


Index: lldb/include/lldb/API/SBDefines.h
===
--- lldb/include/lldb/API/SBDefines.h
+++ lldb/include/lldb/API/SBDefines.h
@@ -43,6 +43,7 @@
 namespace lldb {
 
 class LLDB_API SBAddress;
+class LLDB_API SBAttachInfo;
 class LLDB_API SBBlock;
 class LLDB_API SBBreakpoint;
 class LLDB_API SBBreakpointLocation;
@@ -84,10 +85,12 @@
 class LLDB_API SBModuleSpec;
 class LLDB_API SBModuleSpecList;
 class LLDB_API SBPlatform;
+class LLDB_API SBPlatformShellCommand;
 class LLDB_API SBProcess;
 class LLDB_API SBProcessInfo;
 class LLDB_API SBQueue;
 class LLDB_API SBQueueItem;
+class LLDB_API SBReproducer;
 class LLDB_API SBSection;
 class LLDB_API SBSourceManager;
 class LLDB_API SBStream;
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D155653: [lldb][NFCI] Add some missing SB class forward declarations

[Med Ismail Bennani via Phabricator via lldb-commits]

mib accepted this revision.
mib added a comment.
This revision is now accepted and ready to land.

LGTM!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D155653

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits