Bug 29122 – Use PLATFORM(ARM_THUMB2) instead of PLATFORM_ARM_ARCH(7)

• Status: RESOLVED
• Resolution: FIXED
• Component: JavaScriptCore

[Gabor Loki]

Currently we are using PLATFORM_ARM_ARCH(7) macro to separate ARM and Thumb-2 instruction set for JIT. If the target architecture of the compilation is ARMv7, the JIT will use Thumb2 instruction set anyway. This scenario is inaccurate. I think we should enable ARM instruction set for ARMv7 as well.

[Gabor Loki]

Created attachment 39345 [details]
Use PLATFORM(ARM_THUMB2) instead of PLATFORM_ARM_ARCH(7)

This patch replaces the usages of PLATFORM_ARM_ARCH(7) to PLATFORM(ARM_THUMB2) which is currently defines only when PLATFORM_ARM_ARCH(7) && PLATFORM(IPHONE) is true.

[Gavin Barraclough]

I'm not really comfortable with this patch, since it seems to me to give a misleading impression.  I think that a new programmer to JSC could easily be lead to assume that PLATFORM_ARM, PLATFORM_ARM_THUMB2, PLATFORM_X86, and PLATFORM_X86_64 are all independent and equivalent in behavior.  A programmer could very easily make the mistake of assuming that code wrapped in PLATFORM(ARM) would not be compiled on a PLATFORM(ARM_THUMB2) build.  This is not the case – both platforms are ARM platforms, and both define that macro.

I think that to aid clarity in the code and make it more explicit what is going on we should be going the other way – making all ifdefs that check for either ARM or ARMv7 (but not both) use either:

    PLATFORM(ARM) && PLATFORM_ARM_ARCH(7)
or
    PLATFORM(ARM) && !PLATFORM_ARM_ARCH(7)

Such that we are very explicit, rather than relying on the ordering of ifdefs, which may be fragile.

[Gabor Loki]

(In reply to comment #2)

Well then, how do you compile JIT with ARM instruction set for ARMv7 target?

[Gavin Barraclough]

This seems an odd requirement, since ARMv7 processors are not guaranteed to support the ARM instruction set – and not all do.  Use of thumb2 on ARMv7 processors is, in ARM's terminology, 'mandated'.

[Gabor Loki]

Only ARMv7M does not contain ARM instruction set. ARMv7A and R support it. Anyway I think the current way of using macros blocks the ARM instruction set support for ARMv7A&R at least. For example, we are not able to compile generic ARM JIT with -mcpu=cortex-a8 compiler option.

[Zoltan Herczeg]

(In reply to comment #4)
> This seems an odd requirement, since ARMv7 processors are not guaranteed to
> support the ARM instruction set – and not all do.  Use of thumb2 on ARMv7
> processors is, in ARM's terminology, 'mandated'.

Although not all support ARM instruction set, I feel we must let the users to dicide which instruction set they prefer. Right now there is no way to use ARM instruction set, since the defines not allow it. Of course the default can be thumb2, just we should give them the opportunity to switch to something else.

[Gavin Barraclough]

Let me think about this.  One solution that addresses my concern stated above may be to remove all direct use of PLATFORM_ARM from the code, and instead use  PLATFORM_ARM_TRADITIONAL and PLATFORM_ARM_THUMB2.  That solves any confusion re exclusivity.  If this is the right king of approch, may not be right to use PLATFORM here, may be a case for USE.  On vacation today, gimme the weekend to thknk about this.

G.

[Gabor Loki]

Created attachment 39595 [details]
Defines two pseudo-platforms for ARM and Thumb-2 instruction set

This patch introduce WTF_PLATFORM_ARM_TRADITIONAL and WTF_PLATFORM_ARM_THUMB2 pseudo-platforms according to Gavin suggestion on irc.

The PLATFORM_ARM_ARCH(7) is replaced with PLATFORM(ARM_THUMB2) and the PLATFORM(ARM) && !PLATFORM_ARM_ARCH(7) is replaced with PLATFORM(ARM_TRADITIONAL). The common code is still under PLATFORM(ARM) #if checks.

By default the WTF_PLATFORM_ARM_THUMB2 is set when 'thumb2' or '__thumb2__' compiler directives are set. Otherwise if the target ARM architecture is v4 or above, WTF_PLATFORM_ARM_TRADITIONAL is set.

This gives an opportunity to use JIT with ARM instruction set on ARMv7 as well.

[Gabor Loki]

Created attachment 39596 [details]
Defines two pseudo-platforms for ARM and Thumb-2 instruction set (v2)

I fixed a typo and updated after some landed patches.

[Gavin Barraclough]

Comment on attachment 39596 [details]
Defines two pseudo-platforms for ARM and Thumb-2 instruction set (v2)

All looks great, will test the iPhone build & land this in the morning.

[Gavin Barraclough]

Sending        JavaScriptCore/ChangeLog
Sending        JavaScriptCore/assembler/ARMAssembler.cpp
Sending        JavaScriptCore/assembler/ARMAssembler.h
Sending        JavaScriptCore/assembler/ARMv7Assembler.h
Sending        JavaScriptCore/assembler/MacroAssembler.h
Sending        JavaScriptCore/assembler/MacroAssemblerARM.cpp
Sending        JavaScriptCore/assembler/MacroAssemblerARM.h
Sending        JavaScriptCore/assembler/MacroAssemblerCodeRef.h
Sending        JavaScriptCore/jit/ExecutableAllocator.h
Sending        JavaScriptCore/jit/JIT.h
Sending        JavaScriptCore/jit/JITInlineMethods.h
Sending        JavaScriptCore/jit/JITOpcodes.cpp
Sending        JavaScriptCore/jit/JITStubs.cpp
Sending        JavaScriptCore/jit/JITStubs.h
Sending        JavaScriptCore/wtf/Platform.h
Sending        JavaScriptCore/yarr/RegexJIT.cpp
Transmitting file data ................
Committed revision 48525.