Thanks! On Fri, Feb 15, 2019 at 04:22 Pavel Labath <pa...@labath.sk> wrote:
> Ah, it's our struct XSAVE friend again. Yeah, I think I have a pretty > good idea of what this is doing, and I believe we don't need any special > alignment there, so I just went ahead and removed it. Assuming this > doesn't cause any issues, you should be able to revert this patch. > > On 13/02/2019 18:46, Jonas Devlieghere wrote: > > Hi Pavel, > > > > I think the offending struct is XSAVE in RegisterContext_x86.h. I don't > > really know this code, would you mind having a look? It looks like you > > touched the alignment in the past and seems to know what this this is > > doing :-) > > > > tree f8fc1af1c62b728260b6c950649c55f7aba513f6 > > parent 6857692687979d934e188d117eadc93f48d52a0c > > author Pavel Labath <pa...@labath.sk <mailto:pa...@labath.sk>> Wed Sep > > 12 08:50:08 2018 +0000 > > committer Pavel Labath <pa...@labath.sk <mailto:pa...@labath.sk>> Wed > > Sep 12 08:50:08 2018 +0000 > > > > Reduce alignment on struct XSAVE, fixing a gcc warning > > > > The warning is about heap-allocating a struct with bigger alignment > > requirements than the standard heap allocator provides. > > > > AFAICT, all uses of the XSAVE struct are already heap-allocated, so this > > high alignment does not actually have any effect and removing it should > > be NFC. > > > > I have also done some digging in the commit history. This alignment > > requirement was since the XSAVE struct was introduced in r180572 when > > adding AVX register support for linux. It does not mention the alignment > > specifically, so I am guessing this was just put there because the > > corresponging XSAVE cpu instruction requires its buffer to be 64-byte > > aligned. However, LLDB will not be normally reading this struct via the > > XSAVE instruction directly. Instead we will ask the kernel to copy the > > buffer saved when suspeding the inferior. This should not require such > > strict alignment (in fact, linux kernel will happily do this for any > > alignment). > > > > llvm-svn: 342029 > > > > > > > > > > > > On Mon, Feb 11, 2019 at 10:55 PM Pavel Labath <pa...@labath.sk > > <mailto:pa...@labath.sk>> wrote: > > > > On 12/02/2019 01:30, Jonas Devlieghere via lldb-commits wrote: > > > Author: jdevlieghere > > > Date: Mon Feb 11 16:30:21 2019 > > > New Revision: 353778 > > > > > > URL: http://llvm.org/viewvc/llvm-project?rev=353778&view=rev > > > Log: > > > Define _ENABLE_EXTENDED_ALIGNED_STORAGE on Windows. > > > > > > Apparently there are multiple places where MSVC complains about > > > instantiations with extended aligment. I think it's better to > define > > > `_ENABLE_EXTENDED_ALIGNED_STORAGE` as suggested by the error > message. > > > > > > I don't have access to a Windows machine so this is all > speculative. > > > > > > > I think it would be worth investigating where is this large alignment > > coming from (my guess: something inside the RegisterContextDarwin > > structs). We've had a case in the past where we were heap-allocating > a > > struct with a 64-byte alignment. MSVC would give a warning there, but > > gcc&clang would silently under-align the result. > > > > In that case, the large alignment was actually unneeded, so I fixed > > both > > issues by just lowering it. I suspect something similar may be > possible > > here. > > > > pl > > > > -- Sent from my iPhone
_______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
