[Webkit-unassigned] [Bug 31060] Use ARMv7 specific encoding for immediate constants on ARMv7 target

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=31060


Gavin Barraclough <barraclough at apple.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #42381|review?                     |review-
               Flag|                            |




--- Comment #2 from Gavin Barraclough <barraclough at apple.com>  2009-11-04 01:45:33 PDT ---
(From update of attachment 42381)
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.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.