#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)
Change History (8)
by , 21 years ago
| Attachment: | be-unaligned.diff added |
|---|
comment:1 by , 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 , 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.
comment:3 by , 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 , 21 years ago
| Owner: | set to |
|---|---|
| Status: | new → closed |
comment:6 by , 5 years ago
| Component: | → Engine: SCUMM |
|---|---|
| Game: | → The Dig |
Patch against a June 13 CVS snapshot