<div dir="auto"><div>Wt., 13 lis 2018, 06:03: Matt Turner <<a href="mailto:mattst88@gmail.com">mattst88@gmail.com</a>> napisał(a):<br><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">On Mon, Nov 12, 2018 at 3:07 PM Eric Anholt <<a href="mailto:eric@anholt.net" target="_blank" rel="noreferrer">eric@anholt.net</a>> wrote:<br>
><br>
> Matt Turner <<a href="mailto:mattst88@gmail.com" target="_blank" rel="noreferrer">mattst88@gmail.com</a>> writes:<br>
><br>
> > Prior to this patch sizeof(linear_header) was 20 bytes in a<br>
> > non-debug build on 32-bit platforms. We do some pointer arithmetic to<br>
> > calculate the next available location with<br>
> ><br>
> >    ptr = (linear_size_chunk *)((char *)&latest[1] + latest->offset);<br>
> ><br>
> > in linear_alloc_child(). The &latest[1] adds 20 bytes, so an allocation<br>
> > would only be 4-byte aligned.<br>
> ><br>
> > On 32-bit SPARC a 'sttw' instruction (which stores a consecutive pair of<br>
> > 4-byte registers to memory) requires an 8-byte aligned address. Such an<br>
> > instruction is used to store to an 8-byte integer type, like intmax_t<br>
> > which is used in glcpp's expression_value_t struct.<br>
> ><br>
> > As a result of the 4-byte alignment returned by linear_alloc_child() we<br>
> > would generate a SIGBUS (unaligned exception) on SPARC.<br>
> ><br>
> > According to the GNU libc manual malloc() always returns memory that has<br>
> > at least an alignment of 8-bytes [1]. I think our allocator should do<br>
> > the same.<br>
> ><br>
> > So, simple fix with two parts:<br>
> ><br>
> >    (1) Increase SUBALLOC_ALIGNMENT to 8 unconditionally.<br>
> >    (2) Mark linear_header with an aligned attribute, which will cause<br>
> >        its sizeof to be rounded up to that alignment. (We already do<br>
> >        this for ralloc_header)<br>
> ><br>
> > With this done, all Mesa's unit tests now pass on SPARC.<br>
> ><br>
> > [1] <a href="https://www.gnu.org/software/libc/manual/html_node/Aligned-Memory-Blocks.html" rel="noreferrer noreferrer" target="_blank">https://www.gnu.org/software/libc/manual/html_node/Aligned-Memory-Blocks.html</a><br>
> ><br>
> > Fixes: 47e17586924f ("glcpp: use the linear allocator for most objects")<br>
> > Bug: <a href="https://bugs.gentoo.org/636326" rel="noreferrer noreferrer" target="_blank">https://bugs.gentoo.org/636326</a><br>
> > ---<br>
> >  src/util/ralloc.c | 14 ++++++++++++--<br>
> >  1 file changed, 12 insertions(+), 2 deletions(-)<br>
> ><br>
> > diff --git a/src/util/ralloc.c b/src/util/ralloc.c<br>
> > index 745b4cf1226..fc35661996d 100644<br>
> > --- a/src/util/ralloc.c<br>
> > +++ b/src/util/ralloc.c<br>
> > @@ -552,10 +552,18 @@ ralloc_vasprintf_rewrite_tail(char **str, size_t *start, const char *fmt,<br>
> >   */<br>
> ><br>
> >  #define MIN_LINEAR_BUFSIZE 2048<br>
> > -#define SUBALLOC_ALIGNMENT sizeof(uintptr_t)<br>
> > +#define SUBALLOC_ALIGNMENT 8<br>
> >  #define LMAGIC 0x87b9c7d3<br>
> ><br>
> > -struct linear_header {<br>
> > +struct<br>
> > +#ifdef _MSC_VER<br>
> > + __declspec(align(8))<br>
> > +#elif defined(__LP64__)<br>
> > + __attribute__((aligned(16)))<br>
> > +#else<br>
> > + __attribute__((aligned(8)))<br>
> > +#endif<br>
> > +   linear_header {<br>
> >  #ifndef NDEBUG<br>
> >     unsigned magic;   /* for debugging */<br>
> >  #endif<br>
> > @@ -647,6 +655,8 @@ linear_alloc_child(void *parent, unsigned size)<br>
> >     ptr = (linear_size_chunk *)((char*)&latest[1] + latest->offset);<br>
> >     ptr->size = size;<br>
> >     latest->offset += full_size;<br>
> > +<br>
> > +   assert((uintptr_t)&ptr[1] % SUBALLOC_ALIGNMENT == 0);<br>
> >     return &ptr[1];<br>
> >  }<br>
><br>
> These patches are:<br>
><br>
> Reviewed-by: Eric Anholt <<a href="mailto:eric@anholt.net" target="_blank" rel="noreferrer">eric@anholt.net</a>><br>
<br>
Thanks a bunch! I hope this is useful for you on arm as well.<br>
<br>
> However, shouldn't we also bump SUBALLOC_ALIGNMENT to 16 on LP64, too,<br>
> if that's what glibc is doing for malloc?<br>
<br>
16-byte alignment is necessary for SSE aligned vector load/store<br>
instructions. I suppose we're not getting any vectorized SSE<br>
load/store instructions to memory allocated by linear_alloc_* and<br>
that's why we haven't seen problems?<br></blockquote></div></div><div dir="auto"><br></div><div dir="auto">FWIW, at least clang on x86 assumes malloc/new return pointers aligned to 16 bytes, though it probably doesn't detect linear_alloc_* as such.</div><div dir="auto"><br></div><div dir="auto">Regards,</div><div dir="auto">Gustaw Smolarczyk</div><div dir="auto"><br></div><div dir="auto"><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
Seems reasonable to bump it to 16-bytes.<br>
_______________________________________________<br>
mesa-dev mailing list<br>
<a href="mailto:mesa-dev@lists.freedesktop.org" target="_blank" rel="noreferrer">mesa-dev@lists.freedesktop.org</a><br>
<a href="https://lists.freedesktop.org/mailman/listinfo/mesa-dev" rel="noreferrer noreferrer" target="_blank">https://lists.freedesktop.org/mailman/listinfo/mesa-dev</a><br>
</blockquote></div></div></div>