Re: PATCH: 3/6 [2nd try]: Add AVX support (i386 changes)

["H.J. Lu" <hjl.tools@gmail.com>]

On Sat, Mar 27, 2010 at 9:09 AM, Mark Kettenis <mark.kettenis@xs4all.nl> wrote:
>> >> >> +    perror_with_name (_("Couldn't read extended state status"));
>> >> >> +
>> >> >> +  i387_supply_xsave (regcache, -1, xstateregs);
>> >> >> +  return 1;
>> >> >> +}
>> >> >> +
>> >> >> +/* Store all valid registers in GDB's register array covered by the
>> >> >> +   PTRACE_SETREGSET request into the process/thread specified by TID.
>> >> >> +   Return non-zero if successful, zero otherwise.  */
>> >> >> +
>> >> >> +static int
>> >> >> +store_xstateregs (const struct regcache *regcache, int tid, int regno)
>> >> >> +{
>> >> >> +  unsigned long long xstateregs[xstate_size_n_of_int64];
>> >> >
>> >> > I think it is better to use I386_XSTATE_MAX_SIZE here.
>> >>
>> >> That is how the kernel interface works.  Whatever value
>> >> I386_XSTATE_MAX_SIZE is today won't be the same tomorrow. We will
>> >> increase it in the coming years. But the same gdb binary will work
>> >> fine since kernel will only copy number of bytes specified in
>> >> iov.iov_len, which is all gdb cares/needs.
>> >
>> > Yes, you'll need to raise I386_XSTATE_MAX_SIZE whenever the kernel
>> > gains support for different/larger xstates.  But I don't see a problem
>> > with that, since you'll have to make changes to GDB to support those
>> > variants anyway.  That reminds me:
>>
>> I will remove I386_XSTATE_MAX_SIZE since it isn't needed by kernel.
>
> Huh?  You're missing the point here.  GDB is supposed to be written in
> C90, which doesn't support variable-length arrays.  So you need a
> compile-time constant to size the xstateregs array.  And
> I386_XSTATE_MAX_SIZE fits the bill there perfectly.
>

I will make the change.

Thanks.

-- 
H.J.