Project

General

Profile

Emulator Issues #7056

Our architecture defines are stupid

Added by Sonicadvance1 about 7 years ago.

Status:
Fixed
Priority:
High
Assignee:
% Done:

0%

Operating system:
Android
Issue type:
Bug
Milestone:
Regression:
No
Relates to usability:
No
Relates to performance:
No
Easy:
Yes
Relates to maintainability:
Yes
Regression start:
Fixed in:

Description

Our architecture defines are quickly becoming a problem for supporting new architectures.
Currently ARMv7 works due to an assumption that !x86_64 falls back to a 32bit codepath that compiles fine on ARMv7.
With the addition of AArch64, this assumption that !x86_64 code path should fallback to a 32bit code fails.

I'm proposing we change our defines in such a way that we handle 64bit and 32bit code paths independent of what the host architecture is. This would make it so AArch64 can compile some x86_64 headers since there won't be implicit fallback to "x86" code.

An example of how we are failing currently is https://github.com/dolphin-emu/dolphin/blob/master/Source/Core/Common/x64Emitter.h#L196
This is checking for x86_64 and falling back on to the 32bit code path. This causes issues with AArch64 of course because we are casting a 64bit pointer to u32 and losing precision causing build failure.

Even those this header isn't hitting running code on AArch64, it still needs to be included in some cases where a clean cut from architecture dependent code will require large re-architecting of our codebase to implicitly cut out any x86/x86_64 specific code.

A very good example of code that would need to be completely changed to cut out x86 specific code is the DSP JIT. This would require a extremely large amount of code change to cut it out from ARM builds. Not only including a DSPJITInterface just like our current JITInterface, but along with large struct array changes for instruction pointers and how the DSP calls code to be recompiled.

I'm not entirely sure how this should be changed, but we will need more of our own defines. Basically need two sets of defines, one for determining bitness, and one for arch.
Maybe something like
_ARCH_64
_ARCH_32
and
_M_ARM (includes AArch64 by checking for _ARCH_64)
_M_MIPS (includes longsoon/warrior by checking for _ARCH_64)
_M_X86 (includes x86_64 by checking for _ARCH_64)

History

#1 Updated by delroth about 7 years ago

Proposal sounds good to me. Would just suggest adding additional convenience #defines like _M_ARM_64, _M_MIPS_64, _M_X86_64.

#2 Updated by Sonicadvance1 about 7 years ago

Should we have 32bit /only/ helper defines as well?
aka _M_ARM is on both 32bit and 64bit, while _M_ARM_64 would be on 64bit only. Should we have _M_ARM_32 as well?

_M_ARM_32
_M_X86_32
etc etc

#3 Updated by Sonicadvance1 about 7 years ago

Going to do the _32 bit helpers as well because they are actually helpful in a bunch of situations.

#5 Updated by flacs about 7 years ago

  • Status changed from New to Fixed

Merged in 09212d4.

Also available in: Atom PDF