Opened 16 years ago

Closed 16 years ago

Last modified 8 months ago

#8247 closed patch

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

Reported by: eriktorbjorn Owned by: fingolfin
Priority: normal Component: Engine: SCUMM
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 16 years ago.
Patch against a June 13 CVS snapshot
be-unaligned-full.diff (29.4 KB) - added by eriktorbjorn 16 years ago.
Patch against a June 14 CVS snapshot

Download all attachments as: .zip

Change History (8)

Changed 16 years ago by eriktorbjorn

Attachment: be-unaligned.diff added

Patch against a June 13 CVS snapshot

comment:1 Changed 16 years ago by eriktorbjorn

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

comment:2 Changed 16 years ago by fingolfin

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.

Changed 16 years ago by eriktorbjorn

Attachment: be-unaligned-full.diff added

Patch against a June 14 CVS snapshot

comment:3 Changed 16 years ago by eriktorbjorn

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 Changed 16 years ago by fingolfin

Owner: set to fingolfin
Status: newclosed

comment:5 Changed 16 years ago by fingolfin

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

comment:6 Changed 8 months ago by digitall

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