ayushsahay1837 wrote:

@omjavaid, I apologize for the delayed response. Thanks for looking into this!

> **1)** Description and commit message may require a rewrite. I had to read 
> till the last paragraph to figure out that VL is fixed for process lifetime. 
> Also streaming mode is an SME thing, mixing it in here makes it confusing 
> whether dynamic switching between SVE and SME and dynamic register size 
> change is supported or not.

Microsoft hasn't released a Windows SDK with streaming SVE support yet; so, 
streaming-mode register access is still unsupported. However, once streaming 
SVE is available, a thread may move between streaming and non-streaming modes, 
which could change the vector length even if it only alternates between the two 
possible values: the non-streaming and streaming SVE vector lengths. As a 
result, the register context layout may need to change when streaming SVE 
support is introduced. Nevertheless, I agree that this could create unnecessary 
confusion. So, streaming SVE will be handled separately when the support 
becomes available.

> **2)** ConfigureRegisterContext runs on every stop via DoStop -> 
> InvalidateAllRegisters and does a full GetThreadContext with XSTATE for every 
> thread. Can we do something about it? Also the comment says "in case there is 
> any change in the configuration" which contradicts the description, VL cant 
> change.

The latest revision no longer suggests dynamic vector length switching. It 
separates SVE layout configuration from stop-time value invalidation. Moreover, 
SVE layout configuration no longer caches thread contexts. The non-streaming 
SVE vector length is cached in _m_sve_vl_ after the SVE header has been read 
and parsed for the first time, and _vg_ is served from that cache if available. 
Splitting GPR/FPR-only caching from SVE caching is left as a follow-up.

> **4)** We need a way to test this without hardware, something like 
> https://lldb.llvm.org/resources/qemu-testing.html but for windows.
> 
> **5)** Current test only runs at VL=128 where Z access just falls back to FPR 
> read/write so the new interleaving code never actually runs. Can you do a run 
> on a QEMU windows arm64 guest with SVE VL > 128 and share results? Best if 
> you give add test instructions using QEMU and may be if I find time can try 
> it own my own.

Agreed, testing this with QEMU especially with VL > 128 so that the Z-register 
high-byte interleaving path is covered would be very valuable (the currently 
available hardware only exposes VL=128). Would it be alright if we address this 
in a follow-up change? I’ve documented the current limitation and updated the 
test to derive register sizes from vg rather than assuming that vg is 128 bits. 
That should allow the test to run correctly on a wider-VL configuration once 
one is available.

> **7)** Too many #if PF_ARM_SVE_INSTRUCTIONS_AVAILABLE, even inside function 
> signatures and if bodies, hard to follow the control flow. We had the same 
> problem on Linux and solved it with LinuxPTraceDefines_arm64sve.h.

I agree, the conditional structure is indeed noisy. However, a Windows 
equivalent of _LinuxPTraceDefines_arm64sve.h_ may be simpler to tackle after 
we’ve landed a minimum viable product. What do you think?

https://github.com/llvm/llvm-project/pull/205906
_______________________________________________
lldb-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to