#1175 closed defect (wontfix)
SCUMM_NEED_ALIGNMENT detection bogus
| Reported by: | SF/mellum | Owned by: | fingolfin |
|---|---|---|---|
| Priority: | normal | Component: | Ports |
| Version: | Keywords: | ||
| Cc: | Game: |
Description
SCUMM_NEED_ALIGNMENT tries to detect whether unaligned accesses "work". Unfortunately, this is bogus; many systems, like Alpha, don't support unaligned accesses, but catch them with a trap, fix them in kernel space and continue. Therefore, it will appear they work; only they slow down the program by a factor of 100. The only reasonable method is to hardcode architectures where it is known that unaligned accesses are OK and fast. Anyway, I doubt it is of much use for code like this:
#if defined(SCUMM_NEED_ALIGNMENT) memcpy(dst, src, 8); #else ((uint32 *)dst)[0] = ((const uint32 *)src)[0]; ((uint32 *)dst)[1] = ((const uint32 *)src)[1]; #endif
If gcc doesn't generate optimal code for memcpy(dst, src, 8) for your platform, you should rather file a bug report against gcc than kludge around it.
Ticket imported from: #791741. Ticket imported from: bugs/1175.
Change History (4)
comment:1 by , 21 years ago
comment:2 by , 21 years ago
| Owner: | set to |
|---|---|
| Resolution: | → wontfix |
| Status: | new → closed |
comment:3 by , 21 years ago
We will not change the alignment detection to be arch based, that would be a very bad idea, as bbrox explained. However, we could add a special case for the Alpha; it's up to you or another Alpha user to submit an according patch via the patch tracker.
As for just using memcpy directly: Not everybody is using gcc, or a compiler which optimizes away memcpy for static len params. I am not even sure whether GCC really really will generate equivalent code for memcpy & our "hand optimized" copy code. But even if it does, it doesn't matter at all: GCC is not the only compiler we care about.
comment:4 by , 5 years ago
| Component: | --Unset-- → Ports |
|---|
Well, usually, a configure script should not check for specific platforms but to see if the platform supports a given feature or not (otherwise, each time someone tries on a platform we have no knowledge of, we would need to change the configure script). So, basically, the configure test is sound as ScummVM will work on this platform as unaligned memory accesses are supported (emulated, true, but supported).
This check works on the platform we had access to (Linux/ARM, Solaris/SPARC, MIPS, ...). So the best would be for you to send a patch adding an Alpha-only override. Wonder why, though, the system has this trap mechanism whereas most other archs do not have it.
Now, as for the 'memcpy' thingy, the thing is that the '#else' part was first and when someone had a crash on a platform like ARM, he added the 'memcpy' code and let the other code stay in place. I agree though that it could be merged :-)