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