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

Mark Kettenis
2010-04-07T10:13:36Z

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain, Size: 1876 bytes --]

> Date: Fri, 2 Apr 2010 08:27:55 -0700
> From: "H.J. Lu" <hjl.tools@gmail.com>
>
> On Fri, Apr 2, 2010 at 7:41 AM, Mark Kettenis <mark.kettenis@xs4all.nl> wrote:
> >> Date: Fri, 2 Apr 2010 07:31:07 -0700
> >> From: "H.J. Lu" <hongjiu.lu@intel.com>
> >>
> >> On Sun, Mar 28, 2010 at 06:11:24PM -0700, H.J. Lu wrote:
> >> > Hi,
> >> >
> >> > Here are i386 changes to support AVX. OK to install?
> >> >
> >>
> >> Here is the updated i386 changes to support AVX. OK to install?
> >
> > Sorry, but I'm still unhappy with the way you modify the
> > i386_linux_regset_sections[] array at run time.  I think the best
> > thing to do is to have gcore *always* create a NT_X86_XSTATE note of
>
> Generate NT_X86_XSTATE note without kernel/processor
> NT_X86_XSTATE note support may require changes to
> existing FXSAVE code path. I will investigate it.
>
> BTW, I have a follow up patch to implement 32bit core
> registers without SSE registers to properly support older
> processors, like Pentium and Pentium Pro. Should
> "gcore" generate NT_PRXFPREG note?

Probably. It'll surely make the code simpler.

>
> > the maximum size supported by GDB.  That way you can remove a lot of
> > code (including the duplication of code in i387_collect_xsave).
> >
>
> XSAVE is different from FXSAVE in some subtle ways, although
> XSAVE memory layout is an extension to FXSAVE memory layout.
> XSAVE has used or initialized states for SSE and AVX registers.
> Most of the codes in i387_collect_xsave deal with used/initialized states.
>
> Please identify the duplication of code in i387_collect_xsave. I will take
> a look.

There is in if (gcore) { } else { } there, that has quite a bit of
duplicated code. I may be missing something, but the fact that
i387_collect_xsave() does different things whether it is generating a
core file or not seems to be undesirable and wrong to me.