[PATCH] x86: Simplify AMX XSAVEC calculation logic.

[H.J. Lu]

On Wed, Apr 2, 2025 at 7:10 PM H.J. Lu <hjl.tools@gmail.com> wrote:
>
> On Wed, Apr 2, 2025 at 4:46 PM Sunil Pandey <skpgkp2@gmail.com> wrote:
> >
> >
> >
> > On Wed, Apr 2, 2025 at 3:24 PM H.J. Lu <hjl.tools@gmail.com> wrote:
> >>
> >> On Tue, Apr 1, 2025 at 7:07 AM Sunil Pandey <skpgkp2@gmail.com> wrote:
> >> >
> >> >
> >> >
> >> > On Mon, Mar 31, 2025 at 9:25 PM Sunil Pandey <skpgkp2@gmail.com> wrote:
> >> >>
> >> >>
> >> >>
> >> >> On Mon, Mar 31, 2025 at 6:13 AM Florian Weimer <fweimer@redhat.com> wrote:
> >> >>>
> >> >>> * Sunil K. Pandey:
> >> >>>
> >> >>> > +#ifdef __x86_64__
> >> >>> > +                   if (amx_flag)
> >> >>> > +                     {
> >> >>> > +                       unsigned int amx_size
> >> >>> > +                         = xstate_comp_offsets[18] + xstate_comp_sizes[18];
> >> >>> > +                       _dl_x86_features_tlsdesc_state_size
> >> >>> > +                         = ALIGN_UP (amx_size + TLSDESC_CALL_REGISTER_SAVE_AREA,
> >> >>> > +                                     64);
> >> >>> > +                     }
> >> >>> > +#endif
> >> >>>
> >> >>> I diskike the magic 18 (as the AMX bit number) appearing here.
> >> >>>
> >> >>> The logic for including and excluding states seems reversed to me.
> >> >>> Shouldn't we save and restore everything that we don't know anything
> >> >>> about, especially for the TLSDESC case?
> >> >>>
> >> >>> Thanks,
> >> >>> Florian
> >> >>
> >> >>
> >> >> Thank you so much. Will submit v2.
> >> >
> >> >
> >> > v2 patch attached.
> >>
> >> Please add more descriptions in the commit log.
> >>
> >>  #ifdef __x86_64__
> >> -       unsigned int amx_size
> >> - = (xstate_amx_comp_offsets[31]
> >> -    + xstate_amx_comp_sizes[31]);
> >> -       amx_size
> >> - = ALIGN_UP ((amx_size
> >> -      + TLSDESC_CALL_REGISTER_SAVE_AREA),
> >> -     64);
> >> -       /* Set TLSDESC state size to the compact AMX
> >> - state size for XSAVEC.  */
> >> -       _dl_x86_features_tlsdesc_state_size = amx_size;
> >> +       _dl_x86_features_tlsdesc_state_size
> >> + = ALIGN_UP (size + TLSDESC_CALL_REGISTER_SAVE_AREA, 64);
> >> +       /* Subtract AMX size from total size.  */
> >>
> >> Subtract size from the start of X86_XSTATE_TILECFG_ID space to the end of
> >> X86_XSTATE_TILEDATA_ID space.
> >>
> >> +       size -= ((xstate_comp_offsets[18]
> >> + + xstate_comp_sizes[18])
> >>
> >> xstate_comp_offsets[X86_XSTATE_TILEDATA_ID + 1]?
> >>
> >> +        - (xstate_comp_offsets[16]
> >> +   + xstate_comp_sizes[16]));
> >>
> >> xstate_comp_offsets[X86_XSTATE_TILECFG_ID]?
> >
> >
> >
> >  X86_XSTATE_TILEDATA_ID and X86_XSTATE_TILECFG_ID must start from cache aligned location.
> > Depending on which previous state gets enabled, size calculation may not be correct.
> >
> > How about
> >
> >  size -= (xstate_comp_offsets[X86_XSTATE_TILEDATA_ID + 1]
> >               - (xstate_comp_offsets[X86_XSTATE_TILECFG_ID - 1]
> >                  + xstate_comp_sizes[X86_XSTATE_TILECFG_ID - 1]));
> >
>
> How about
>
> size  = ALIGN_UP (size + TLSDESC_CALL_REGISTER_SAVE_AREA,  64);
> size -= xstate_comp_offsets[X86_XSTATE_TILEDATA_ID + 1] -
> xstate_comp_offsets[X86_XSTATE_TILECFG_ID];
>
> xstate_comp_offsets[X86_XSTATE_TILECFG_ID] ==
> xstate_comp_offsets[X86_XSTATE_TILEDATA_ID + 1]
> if there is no AMX.  Otherwise, they are multiples of 64.
>
>
> --
> H.J.

Please try this.


-- 
H.J.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-x86-Optimize-xstate-size-calculation.patch
Type: text/x-patch
Size: 5230 bytes
Desc: not available
URL: <https://sourceware.org/pipermail/libc-alpha/attachments/20250403/27f5d4f7/attachment-0001.bin>