Opened 21 years ago

Closed 21 years ago

Last modified 6 years ago

#8247 closed patch

ALL: READ...() vs READ...UNALIGNED()

Reported by: eriktorbjorn Owned by: fingolfin
Priority: normal Component: Engine: SCUMM
Version: Keywords:
Cc: Game: The Dig

Description

WooShell was having problems with unaligned reads in the NUT renderer when playing The Dig on a Sparc.

Looking at scummsys.h, I noticed that in the little-endian case, we automatically define the READ primitives to be alignment-safe, if SCUMM_NEED_ALIGNMENT is defined. The _UNALIGNED read primitives are just aliases for the normal read primitives, since they all work on big-endian values.

However, in the big-endian case READ_BE_...() are never alignment-safe, so it's the caller's responsibility to use the READ_BE_..._UNALIGNED() primitives instead, when necessary.

Is there any particular reason we do it this way? Couldn't we just get rid of the _UNALIGNED primitives completely? I realize that the alignment-safe primitives will be a bit slower of course, but I vaguely remember experimenting with this sort of thing on an assignment at school once. The speedup wasn't that great, really, and from what I recall that code did quite a lot of reading and writing...

This patch changes the big-endian case to be more like the little-endian case, but doesn't touch any other files since I wanted some feedback on this part first.

Ticket imported from: #754151. Ticket imported from: patches/352.

Attachments (2)

be-unaligned.diff (2.6 KB ) - added by eriktorbjorn 21 years ago.
Patch against a June 13 CVS snapshot
be-unaligned-full.diff (29.4 KB ) - added by eriktorbjorn 21 years ago.
Patch against a June 14 CVS snapshot

Download all attachments as: .zip

Change History (8)

by eriktorbjorn, 21 years ago

Attachment: be-unaligned.diff added

Patch against a June 13 CVS snapshot

comment:1 by eriktorbjorn, 21 years ago

By the way, I would have asked this on the development list instead, but email is down here at the moment.

comment:2 by fingolfin, 21 years ago

Personally I wouldn't object to making all of the READ_* calls always alignment safe. I doubt that this will cause any noticable speed impact on any of our targets anyway. Modern processor will read a full cacheline anyway, so you only get little to no overhead.

by eriktorbjorn, 21 years ago

Attachment: be-unaligned-full.diff added

Patch against a June 14 CVS snapshot

comment:3 by eriktorbjorn, 21 years ago

Here's a more complete patch, then. I've only had the time to give it a quick try, but it worked that far at least.

comment:4 by fingolfin, 21 years ago

Owner: set to fingolfin
Status: newclosed

comment:5 by fingolfin, 21 years ago

I added some more cleanup, and put this into CVS.

comment:6 by digitall, 6 years ago

Component: Engine: SCUMM
Game: The Dig
Note: See TracTickets for help on using tickets.