[FFmpeg-devel] [PATCH] Make HAVE_FAST_UNALIGNED allow unaligned memory accesses

[Måns Rullgård]

Michael Niedermayer wrote:
> On Wed, Jul 16, 2008 at 01:46:56PM +0100, M?ns Rullg?rd wrote:
>>
>> Michael Niedermayer wrote:
>> > On Tue, Jul 15, 2008 at 08:16:21PM +0100, Mans Rullgard wrote:
>> >> If HAVE_FAST_UNALIGNED is defined, potentially unaligned data is
>> >> accessed through normal pointers.  Otherwise, compiler-specific
>> >> code is used to perform unaligned accesses, falling back to
>> >> byte-wise access if no compiler support is available.
>> >> ---
>> >>  libavutil/intreadwrite.h |  130
>> >> +++++++++++++++++++++++++++------------------
>> >>  1 files changed, 78 insertions(+), 52 deletions(-)
>> >
>> > [...]
>> >
>> >> @@ -37,7 +39,19 @@ struct unaligned_16 { uint16_t l; }
>> >> __attribute__((packed));
>> >>  #define AV_WN32(a, b) (((struct unaligned_32 *) (a))->l) = (b)
>> >>  #define AV_WN64(a, b) (((struct unaligned_64 *) (a))->l) = (b)
>> >>
>> >> +#elif defined(__DECC)
>> >> +
>> >> +#define AV_RN16(a) (*((const __unaligned uint16_t*)(a)))
>> >> +#define AV_RN32(a) (*((const __unaligned uint32_t*)(a)))
>> >> +#define AV_RN64(a) (*((const __unaligned uint64_t*)(a)))
>> >> +
>> >> +#define AV_WN16(a, b) *((__unaligned uint16_t*)(a)) = (b)
>> >> +#define AV_WN32(a, b) *((__unaligned uint32_t*)(a)) = (b)
>> >> +#define AV_WN64(a, b) *((__unaligned uint64_t*)(a)) = (b)
>> >
>> > This change is ok, as a seperate commit (note ive edit it, the removial
>> > of #else on its own would break the code).
>> >
>> > The rest of the patch has to be split in cosmetics that are free of
>> > functional changes and the functional changes before it is reviewable.
>>
>> Are you referring to the reordering of some macros?  That can probably
>> be done separately first.
>
> yes

OK, I'll see about that.

>> Anyway, I'm not asking for a detailed review right now.  I want to know
>> if this is the right thing at all.  The code currently in svn has the
>> effect of ignoring HAVE_FAST_ALIGNED when gcc is used, since gcc turns
>> accesses to "attribute packed" things into byte-reads much like the
>> non-fast_unaligned code does.  Somehow, I don't think this was the
>> original intent.
>
> Ok, now i understand what this patch was about :)
> yes iam fine with that if it doesnt break anything (segfaults on arches that
> require alignent or misaligned reads through some exception handler ...)

The intent is that HAVE_FAST_UNALIGNED will only be set if the hardware
supports unaligned access without taking a trap of any kind.  It is
currently only enabled for x86, so this change should be safe from that
point of view.

-- 
M?ns Rullg?rd
mans at mansr.com