Bug 31060 – Use ARMv7 specific encoding for immediate constants on ARMv7 target

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

[Gabor Loki]

ARMv7 provides a simple way to store 16bit wide immediate constants to a register. This is useful when JavaScriptCore is tuned to an ARMv7 processor.

[Gabor Loki]

Created attachment 42381 [details]
Use ARMv7 specific encoding for immediate constants on ARMv7 target

[Gavin Barraclough]

Comment on attachment 42381 [details]
Use ARMv7 specific encoding for immediate constants on ARMv7 target

Having 0 as the "cannot encode" value for getImm16Op2 is probably not a good idea, since 0 is a common immediate, and could be encoded in one instruction.

I guess this is not actively a problem in this patch, since you don't call encodeComplexImm on zero – but to guard against this mistake creeping in you should probably ASSERT(imm != 0) in this function, or use a value other than 0 to mean 'cannot encode'.

We also prefer to avoid magic values in the code, so I'd suggest that you should define up a name for this (e.g. something like static const ARMWord INVALID_IMM16 0;), then return 0; should be return INVALID_IMM16;, if (tmp) should be if (tmp != INVALID_IMM16).

It would also probably be a good idea to add guards to movw_r & movt_r that op2 is valid.

r-, because I think some extra ASSERTs here are worthwhile, but the patch is otherwise all good.

[Gabor Loki]

Created attachment 42474 [details]
Use ARMv7 specific encoding for immediate constants on ARMv7 target (take 2)

[Zoltan Horvath]

Landed in 50553.
https://trac.webkit.org/changeset/50553