> On 8 Jun 2018, at 15:37, Simon Marchi wrote: > > On 2018-06-08 10:13, Alan Hayward wrote: >> (Moved review to correct thread) >> Thanks for the reviews. >>> On 7 Jun 2018, at 21:18, Simon Marchi wrote: >>> Hi Alan, >>> Just some quick comments. >>> I get this when building on x86-64 with --enable-targets=all: >> Hmm.. I had lost that flag from my build script. I Re-added it, and >> reproduced the issues. >>> CXX aarch64-tdep.o >>> In file included from /home/emaisin/src/binutils-gdb/gdb/nat/aarch64-sve-linux-ptrace.h:29:0, >>> from /home/emaisin/src/binutils-gdb/gdb/aarch64-tdep.c:61: >>> /home/emaisin/src/binutils-gdb/gdb/nat/aarch64-linux-sigcontext.h:19:22: error: field ‘head’ has incomplete type ‘_aarch64_ctx’ >>> struct _aarch64_ctx head; >>> ^ >>> /home/emaisin/src/binutils-gdb/gdb/nat/aarch64-linux-sigcontext.h:19:9: note: forward declaration of ‘struct _aarch64_ctx’ >>> struct _aarch64_ctx head; >>> ^ >>> First, we should not include "nat/aarch64-sve-linux-ptrace.h" (a file that only makes >>> sense when building on AArch64) in aarch64-tdep.c, a file built on all architecture >>> when including the support for AArch64 debugging. It looks like aarch64-tdep.c >>> needs sve_vq_from_vl. Maybe that definition could be moved to arch/, which can be >>> included in aarch64-tdep.c. >> I had put it in there because I wanted to try and make it a complete block >> copied from Linux. The issue makes sense, so I’ve updated to restore >> sve_vq_from_vl/sve_vl_from_vq back to arch/aarch64.h and removed it from >> nat/aarch64-linux-sigcontext.h >>> Then, is the _aarch64_ctx structure guaranteed to be defined on older AArch64 kernels >>> or should we include it too? >> _aarch64_ctx is part of the standard aarch64 signal handling. A quick >> git blame gives >> me 2012 - which is roughly the age of aarch64. So, it should always be defined. >> Updated patch below. Checked it builds (with other sve patches) on: >> X86 all-targets >> Aarch64 Linux 4.10 (pre sve headers) ubuntu 16.04 >> Aarch64 Linux 4.15 (with sve headers) ubuntu 18.04 >> Are you ok with the new version? > > The code looks good to me, thanks. I am still unsure about the licensing side of it, let me ask the FSF people about it, I'll come back to you when it's done. I hope it won't take too long! > Ok, thanks for chasing that up. Happy to be cc’ed (or not) on any email. This patch (I think) only blocks 5/10 and 9/10 in the series. The rest should be ok to still go in (once reviewed). Alan. &j!z޶׏4b֫rnr