Replace the constant pool access ldr instructions with movw/movt, and remove ...

Replace the constant pool access ldr instructions with movw/movt, and remove the corresponding constant pool entries.
BUG=none
TEST=none

[zhangk, 2010-03-19 22:12:52]

In this upload, the constant pool related load instructions are replaced with
movw/movt, and the constant pool entries are removed. This is used to improve
the performance and to reduce the code size. Based on this revision, the
performance on SunSpider is improved by 1.8%. Two issues that have to be
mentioned:

1. The serialization/deserialization handshake mechanism is not updated for this
change. The serialization/deserialization tests fail.

2. The ARMv7 support was checked in before, so it is not added to this issue.

[Erik Corry, 2010-03-22 11:50:09]

As mentioned in email our measurements indicate that inline cache calls are
updated very frequently in the startup phase of a new web page and therefore
they probably have to stay as constant pool loads.  A constant pool load can be
updated without flushing the instruction cache.  Unless your own measurements on
page-cycler-like loads contradict this we have to make sure that IC calls are
still done with the constant pool.  Given this, it's an open question whether or
not this is still worth it.  If it is worth it then here are some comments on
the code so far.

If it is determined that it is worthwhile then note that Android builds are
typically done without the debugger, so we don't have to fix the debugger
support before landing this, as long as no movw or movt instructions are
generated if there is a debugger.  We don't want to have ugly ifdefs any more
places than we already have, but if the ARMv7 availability check and the
debugger presence check are rolled into one test then that shouldn't be a
problem.

http://codereview.chromium.org/1128009/diff/1/5
File src/arm/assembler-arm-inl.h (right):

http://codereview.chromium.org/1128009/diff/1/5#newcode89
src/arm/assembler-arm-inl.h:89: Instr instr = Memory::int32_at(pc);
You should assert that these two instructions actually are movw and movt
instructions.  See kLdrPCMask and lLdrPCPattern for examples on how to do this.

http://codereview.chromium.org/1128009/diff/1/5#newcode91
src/arm/assembler-arm-inl.h:91: Instr instr_movt = Memory::int32_at(pc + 4);
Instead of 4 you should write kInstrSize.  We frown on unnamed constants in the
V8code.

http://codereview.chromium.org/1128009/diff/1/5#newcode92
src/arm/assembler-arm-inl.h:92: if ((instr_movt & 0xff00000) == 0x3400000) {
This makes no sense.  If we don't know for sure that it's a movw/movt pair then
why are we getting the address?

http://codereview.chromium.org/1128009/diff/1/5#newcode103
src/arm/assembler-arm-inl.h:103: if ((instr & 0x0f7f0000) == 0x051f0000) {
These should have names like kLdrPCMask and kLdrPCPattern.

http://codereview.chromium.org/1128009/diff/1/5#newcode268
src/arm/assembler-arm-inl.h:268: if ((instr & 0x0f7f0000) == 0x051f0000)
Please use {} in multiline if statements.

http://codereview.chromium.org/1128009/diff/1/5#newcode286
src/arm/assembler-arm-inl.h:286: // movw movt
Please use sentences or at least noun phrases with capitalization and full
stops.

http://codereview.chromium.org/1128009/diff/1/5#newcode287
src/arm/assembler-arm-inl.h:287: uint32_t *new_pc = (unsigned int*)pc;
We don't allow C-style casts in Google C++ code.
Also, the asterisk goes next to the type, not next to the variable in
declarations.

http://codereview.chromium.org/1128009/diff/1/5#newcode291
src/arm/assembler-arm-inl.h:291: if ((*(new_pc+1) & 0xff00000) == 0x3400000) {
Please assert that the previous instruction is movt instead of checking that it
is.  If there is no movt there then we can't patch.  We can't use a single movw
instruction if we need to be able to patch with an arbitrary value.

http://codereview.chromium.org/1128009/diff/1/5#newcode298
src/arm/assembler-arm-inl.h:298: // Intuitively, we would think it is necessary
to flush the instruction cache
This comment is important.  Your change means it is now necessary to flush the
icache.

http://codereview.chromium.org/1128009/diff/1/6
File src/arm/assembler-arm.cc (right):

http://codereview.chromium.org/1128009/diff/1/6#newcode69
src/arm/assembler-arm.cc:69: 
This looks like an accidental edit.

http://codereview.chromium.org/1128009/diff/1/6#newcode635
src/arm/assembler-arm.cc:635: Condition cond = static_cast<Condition>(instr &
CondMask);
You need to check here whether ARMv7 is available and only use it if so. if
(CpuFeatures::IsSupported(ARMv7)))

http://codereview.chromium.org/1128009/diff/1/6#newcode636
src/arm/assembler-arm.cc:636: if ((x.rmode_ != RelocInfo::EMBEDDED_OBJECT) &&
It's not clear to me why EMBEDDED_OBJECT is special here.

http://codereview.chromium.org/1128009/diff/1/6#newcode642
src/arm/assembler-arm.cc:642: if (x.imm32_ > 65535)
You need to lose the 'if'.  If we have MustUseIp then we can't know whether the
object will move to an address above 65535.

http://codereview.chromium.org/1128009/diff/1/6#newcode966
src/arm/assembler-arm.cc:966: emit(cond | (0x34)*B20 | ((src.imm32_ >> 16 >> 12)
& 0xf)*B16
Shifting twice doesn't make things clearer and putting a hex constant in ()
doesn't add anything either.

http://codereview.chromium.org/1128009/diff/1/6#newcode967
src/arm/assembler-arm.cc:967: | dst.code()*B12 | (((src.imm32_ >> 16)& 0xfff)));
Space before &.

Using double brackets like this: ((expression)) doesn't make things easier to
read.

http://codereview.chromium.org/1128009/diff/1/6#newcode1714
src/arm/assembler-arm.cc:1714: bool withconstpool) {
Arguments should be named_like_this.

http://codereview.chromium.org/1128009/diff/1/6#newcode1822
src/arm/assembler-arm.cc:1822: if (((instr & (7*B25 | P | U | B | W | 15*B16 |
Off12Mask)) !=
This if needs at least a comment to explain what it is doing.

http://codereview.chromium.org/1128009/diff/1/9
File src/arm/disasm-arm.cc (right):

http://codereview.chromium.org/1128009/diff/1/9#newcode382
src/arm/disasm-arm.cc:382: ", #%d", imm);
I think it would be more readable if the movt instruction printed out #xxxx0000.
 What do the ARM disassemblers print for this?

http://codereview.chromium.org/1128009/diff/1/9#newcode664
src/arm/disasm-arm.cc:664:
//------------------------------------------------------------
This looks like an accidental edit.

http://codereview.chromium.org/1128009/diff/1/9#newcode703
src/arm/disasm-arm.cc:703: Format(instr, "movw'cond 'mw");  // movw instruction
The comment doesn't seem to add anything here.

http://codereview.chromium.org/1128009/diff/1/9#newcode729
src/arm/disasm-arm.cc:729: Format(instr, "movt'cond 'mt");  // movt instruction
Or here.

http://codereview.chromium.org/1128009/diff/1/8
File src/arm/simulator-arm.cc (right):

http://codereview.chromium.org/1128009/diff/1/8#newcode1531
src/arm/simulator-arm.cc:1531: // Format(instr, "movw'cond 'rd, 'rn, 'imm");
Is there really an rn in this instruction?

http://codereview.chromium.org/1128009/diff/1/8#newcode1532
src/arm/simulator-arm.cc:1532: alu_out = (instr->Immed4Field() << 12) |
instr->Offset12Field();
This seems to be done a lot of places.  Perhaps instead of adding an Immed4Field
method we should add an ImmedMovtMovwField method that gets all 16 bits.

http://codereview.chromium.org/1128009/diff/1/8#newcode1574
src/arm/simulator-arm.cc:1574: // Format(instr, "movt'cond 'rd, 'rn, 'imm");
rn?

http://codereview.chromium.org/1128009/diff/1/8#newcode2039
src/arm/simulator-arm.cc:2039: int offset_8 = 0x000000FF & instr->Bits(7, 0);
Surely the bits 7 to 0 are already in the range 0 - 255, so anding with
0x000000ff won't make any difference?

http://codereview.chromium.org/1128009/diff/1/8#newcode2094
src/arm/simulator-arm.cc:2094: /*if((unsigned int) instr == 0x579241ac)
Accidental edit?

http://codereview.chromium.org/1128009/diff/1/3
File src/objects.cc (right):

http://codereview.chromium.org/1128009/diff/1/3#newcode5012
src/objects.cc:5012: Instr instr = Memory::int32_at(it.rinfo()->pc());
We can't have ARM-specific code in this file since it is used for all
architectures.  Some refactoring will be necessary.

http://codereview.chromium.org/1128009/diff/1/3#newcode5021
src/objects.cc:5021: if ((instr_movt & 0xff00000) == 0x3400000) {
Same comment as above on the movt not being optional.

http://codereview.chromium.org/1128009/diff/1/3#newcode5022
src/objects.cc:5022: unsigned int offset_movt = ((instr_movt & 0xf0000) >> 4) |
Here is another place where my proposed new method could be used.

http://codereview.chromium.org/1128009/diff/1/3#newcode5027
src/objects.cc:5027: Code** p = reinterpret_cast<Code **>(p1);
I wonder why we have to do this in two steps?

[zhangk, 2010-04-05 23:42:15]

Hi Erik,

Thank you for the reviews. We merged the changes with the newer bleeding edge
(svn 4311) to 
1. take care of the function calls generated in macro-assembler-arm.cc;
2. use the existing ARMv7 feature;
3. re-evalute the benefit.

Most of the comments and concerns are addressed. The regression test (except the
serialization tests) passes. We see about 2.6% improvement on SunSpider and
v8-suite. In the current changes, I think we don't need CPU::FlushICache in
set_target_address_at. 
1. set_target_address_at is called by set_target_object and set_target_address. 
2. set_target_object is called in objects.cc followed with
"CPU::FlushICache(instruction_start(), instruction_size())".
3. calls to set_target_address
3.1 the function call to set_target_address in objects.cc is also followed by a
FlushICache.
3.2 the function call to set_target_address in ic-inh.h is to update the target
of a function call. In macro-assembler-arm.cc, we remain the use of "ldr + blx";
no movw/movt is generated for this case. Thus, set_target_address will update
the const pool, not the ICache.
3.3 set_target_address is called in debug.cc, mark-compact.cc, and liveedit.cc.
Currently, we're not quite clear about what those function calls do and how to
triger those function calls. If you have any insights on this, please let us
know.

-Kun

http://codereview.chromium.org/1128009/diff/1/5
File src/arm/assembler-arm-inl.h (right):

http://codereview.chromium.org/1128009/diff/1/5#newcode89
src/arm/assembler-arm-inl.h:89: Instr instr = Memory::int32_at(pc);
On 2010/03/22 11:50:09, Erik Corry wrote:

Done.

http://codereview.chromium.org/1128009/diff/1/5#newcode91
src/arm/assembler-arm-inl.h:91: Instr instr_movt = Memory::int32_at(pc + 4);
On 2010/03/22 11:50:09, Erik Corry wrote:

Done.

http://codereview.chromium.org/1128009/diff/1/5#newcode92
src/arm/assembler-arm-inl.h:92: if ((instr_movt & 0xff00000) == 0x3400000) {
On 2010/03/22 11:50:09, Erik Corry wrote:

Done.

http://codereview.chromium.org/1128009/diff/1/5#newcode103
src/arm/assembler-arm-inl.h:103: if ((instr & 0x0f7f0000) == 0x051f0000) {
On 2010/03/22 11:50:09, Erik Corry wrote:

Done.

http://codereview.chromium.org/1128009/diff/1/5#newcode268
src/arm/assembler-arm-inl.h:268: if ((instr & 0x0f7f0000) == 0x051f0000)
On 2010/03/22 11:50:09, Erik Corry wrote:

Done.

http://codereview.chromium.org/1128009/diff/1/5#newcode286
src/arm/assembler-arm-inl.h:286: // movw movt
On 2010/03/22 11:50:09, Erik Corry wrote:

Done.

http://codereview.chromium.org/1128009/diff/1/5#newcode287
src/arm/assembler-arm-inl.h:287: uint32_t *new_pc = (unsigned int*)pc;
On 2010/03/22 11:50:09, Erik Corry wrote:

Done.

http://codereview.chromium.org/1128009/diff/1/5#newcode291
src/arm/assembler-arm-inl.h:291: if ((*(new_pc+1) & 0xff00000) == 0x3400000) {
On 2010/03/22 11:50:09, Erik Corry wrote:

Done.

http://codereview.chromium.org/1128009/diff/1/6
File src/arm/assembler-arm.cc (right):

http://codereview.chromium.org/1128009/diff/1/6#newcode69
src/arm/assembler-arm.cc:69: 
On 2010/03/22 11:50:09, Erik Corry wrote:

Done.

http://codereview.chromium.org/1128009/diff/1/6#newcode635
src/arm/assembler-arm.cc:635: Condition cond = static_cast<Condition>(instr &
CondMask);
On 2010/03/22 11:50:09, Erik Corry wrote:

Done.

http://codereview.chromium.org/1128009/diff/1/6#newcode636
src/arm/assembler-arm.cc:636: if ((x.rmode_ != RelocInfo::EMBEDDED_OBJECT) &&
On 2010/03/22 11:50:09, Erik Corry wrote:

EMBEDDED_OBJECT causes failures. The reason is unknown, so it's disabled now.

http://codereview.chromium.org/1128009/diff/1/6#newcode642
src/arm/assembler-arm.cc:642: if (x.imm32_ > 65535)
On 2010/03/22 11:50:09, Erik Corry wrote:

Done.

http://codereview.chromium.org/1128009/diff/1/6#newcode966
src/arm/assembler-arm.cc:966: emit(cond | (0x34)*B20 | ((src.imm32_ >> 16 >> 12)
& 0xf)*B16
On 2010/03/22 11:50:09, Erik Corry wrote:

Done.

http://codereview.chromium.org/1128009/diff/1/6#newcode967
src/arm/assembler-arm.cc:967: | dst.code()*B12 | (((src.imm32_ >> 16)& 0xfff)));
On 2010/03/22 11:50:09, Erik Corry wrote:

Done.

http://codereview.chromium.org/1128009/diff/1/6#newcode1714
src/arm/assembler-arm.cc:1714: bool withconstpool) {
On 2010/03/22 11:50:09, Erik Corry wrote:

Done.

http://codereview.chromium.org/1128009/diff/1/6#newcode1822
src/arm/assembler-arm.cc:1822: if (((instr & (7*B25 | P | U | B | W | 15*B16 |
Off12Mask)) !=
On 2010/03/22 11:50:09, Erik Corry wrote:

Done.

http://codereview.chromium.org/1128009/diff/1/9
File src/arm/disasm-arm.cc (right):

http://codereview.chromium.org/1128009/diff/1/9#newcode664
src/arm/disasm-arm.cc:664:
//------------------------------------------------------------
On 2010/03/22 11:50:09, Erik Corry wrote:

Done.

http://codereview.chromium.org/1128009/diff/1/9#newcode703
src/arm/disasm-arm.cc:703: Format(instr, "movw'cond 'mw");  // movw instruction
On 2010/03/22 11:50:09, Erik Corry wrote:

Done.

http://codereview.chromium.org/1128009/diff/1/9#newcode729
src/arm/disasm-arm.cc:729: Format(instr, "movt'cond 'mt");  // movt instruction
On 2010/03/22 11:50:09, Erik Corry wrote:

Done.

http://codereview.chromium.org/1128009/diff/1/8
File src/arm/simulator-arm.cc (right):

http://codereview.chromium.org/1128009/diff/1/8#newcode1531
src/arm/simulator-arm.cc:1531: // Format(instr, "movw'cond 'rd, 'rn, 'imm");
On 2010/03/22 11:50:09, Erik Corry wrote:

Done.

http://codereview.chromium.org/1128009/diff/1/8#newcode1532
src/arm/simulator-arm.cc:1532: alu_out = (instr->Immed4Field() << 12) |
instr->Offset12Field();
On 2010/03/22 11:50:09, Erik Corry wrote:

Done.

http://codereview.chromium.org/1128009/diff/1/8#newcode1574
src/arm/simulator-arm.cc:1574: // Format(instr, "movt'cond 'rd, 'rn, 'imm");
On 2010/03/22 11:50:09, Erik Corry wrote:

Done.

http://codereview.chromium.org/1128009/diff/1/8#newcode2039
src/arm/simulator-arm.cc:2039: int offset_8 = 0x000000FF & instr->Bits(7, 0);
On 2010/03/22 11:50:09, Erik Corry wrote:

Done.

http://codereview.chromium.org/1128009/diff/1/8#newcode2094
src/arm/simulator-arm.cc:2094: /*if((unsigned int) instr == 0x579241ac)
On 2010/03/22 11:50:09, Erik Corry wrote:

Done.

http://codereview.chromium.org/1128009/diff/1/3
File src/objects.cc (right):

http://codereview.chromium.org/1128009/diff/1/3#newcode5012
src/objects.cc:5012: Instr instr = Memory::int32_at(it.rinfo()->pc());
On 2010/03/22 11:50:09, Erik Corry wrote:

Done.

http://codereview.chromium.org/1128009/diff/1/3#newcode5021
src/objects.cc:5021: if ((instr_movt & 0xff00000) == 0x3400000) {
On 2010/03/22 11:50:09, Erik Corry wrote:

Done.

http://codereview.chromium.org/1128009/diff/1/3#newcode5022
src/objects.cc:5022: unsigned int offset_movt = ((instr_movt & 0xf0000) >> 4) |
On 2010/03/22 11:50:09, Erik Corry wrote:

Done.

http://codereview.chromium.org/1128009/diff/1/3#newcode5027
src/objects.cc:5027: Code** p = reinterpret_cast<Code **>(p1);
On 2010/03/22 11:50:09, Erik Corry wrote:

Done.

[zhangk, 2010-04-20 23:42:44]

Hi Erik,

Thank you for clarifying the facts. I merged your simulator I-cache flush code.
It works fine. I uploaded a new patch set. The major changes are

1. Change the set_target_address API to explicitly enalbe/disable I-cache flush.
The default parameter is to enforce I-cache flush.

2. Disable I-cache flush only for the function calls in objects.cc. For the
other places like mark-compact.cc ic-inl.h, liveedit.cc, debug.cc, I-cache flush
is always taken.

Thanks,
-Kun

[Erik Corry, 2010-04-26 11:03:49]

On 2010/04/20 23:42:44, zhangk wrote:
> Hi Erik,
> 
> Thank you for clarifying the facts. I merged your simulator I-cache flush
code.
> It works fine. I uploaded a new patch set. The major changes are
> 
> 1. Change the set_target_address API to explicitly enalbe/disable I-cache
flush.
> The default parameter is to enforce I-cache flush.
> 
> 2. Disable I-cache flush only for the function calls in objects.cc. For the
> other places like mark-compact.cc ic-inl.h, liveedit.cc, debug.cc, I-cache
flush
> is always taken.

This is good news.  Do all tests pass now?

[zhangk, 2010-04-26 21:27:55]

All tests pass except the serialization/deserialization tests.

Thanks,
-Kun

[zhangk, 2010-04-26 23:56:27]

Hi Erik,

The serialization/de-serialization handshake protocol has to be changed with
constant pool optimization. I'm wondering if we should add a new flag to turn
off this optimization for the purpose of serialization/de-serialization.

Thanks,
-Kun

[Erik Corry, 2010-05-03 10:06:27]

The change needs to be brought up to date so that it is relative to the bleeding
edge.  Due to limitations in the codereview tool this probably means creating a
new issue on codereview.chromium.org.

http://codereview.chromium.org/1128009/diff/46001/47003
File src/arm/assembler-arm-inl.h (right):

http://codereview.chromium.org/1128009/diff/46001/47003#newcode97
src/arm/assembler-arm-inl.h:97: uint32_t offset = ((instr & 0xf0000) >> 4) |
(instr & 0xfff);
The name 'offset' seems wrong here.  It's the immediate from the movw-movt
instruction.  The word 'offset' makes it sound like it is relative to something.

http://codereview.chromium.org/1128009/diff/46001/47003#newcode110
src/arm/assembler-arm-inl.h:110: return
reinterpret_cast<Object**>(get_movw_movt_address(pc_));
This seems wrong.  This function is expected to return a Handle, which is
implemented internally as a pointer to a pointer to an object.  This line takes
a pointer to and object and casts it to add the extra indirection.  Incidentally
this line makes use of the implicit Handle constructor that takes an Object**. 
I have made that constructor explicit since it violated our style guide.

I think the correct change is to have this function return an Object* instead of
a Handle<Object>.  It seems like the only place we use this (objects.cc) we
immediately get the Object* out of the Handle anyway.

http://codereview.chromium.org/1128009/diff/46001/47003#newcode123
src/arm/assembler-arm-inl.h:123: Assembler::set_target_address_at(pc_,
reinterpret_cast<Address>(target),
If all arguments don't fit on one line it is neater to give them one line each
so that it visually looks like 3 arguments and not two.

http://codereview.chromium.org/1128009/diff/46001/47003#newcode298
src/arm/assembler-arm-inl.h:298: void Assembler::set_target_address_at(Address
pc, Address target,
Please put arguments either on one line or on a line each.

http://codereview.chromium.org/1128009/diff/46001/47003#newcode307
src/arm/assembler-arm-inl.h:307: *new_pc = (*new_pc & 0xfff0f000) | ((new_addr &
0xf000) << 4) |
Please give this constant, 0xfff0ff000, a name and use it throughout.

Also, there is a function that returns ((x & 0xf000) << 4) | (x & 0xfff). 
Please use it throughout.

http://codereview.chromium.org/1128009/diff/46001/47003#newcode319
src/arm/assembler-arm-inl.h:319: if (need_icache_flush)
Please use braces on multiline if statements.  This isn't a Google style guide
requirement, but we try to stick to it on V8.

http://codereview.chromium.org/1128009/diff/46001/47003#newcode320
src/arm/assembler-arm-inl.h:320: CPU::FlushICache(pc, sizeof(target));
sizeof(target) looks wrong here.  With the loop above, the size to be flushed is
not a constant.

http://codereview.chromium.org/1128009/diff/46001/47004
File src/arm/assembler-arm.cc (right):

http://codereview.chromium.org/1128009/diff/46001/47004#newcode590
src/arm/assembler-arm.cc:590: movt(rd, x, cond);
In the case of the relocinfo being "None" we don't need to emit the movt unless
it is non-zero.  The RelocInfo class in assembler.h seems to be missing an
IsNone() method, but it is easy to add.

http://codereview.chromium.org/1128009/diff/46001/47004#newcode602
src/arm/assembler-arm.cc:602: ldr(ip, MemOperand(pc, 0), cond);
This code, that handles instructions other than mov, is missing in the movw_movt
branch of this instruction.  If you add it back then perhaps you don't need the
use_movw_movt argument any more?

http://codereview.chromium.org/1128009/diff/46001/47004#newcode1954
src/arm/assembler-arm.cc:1954: int block_for_prinfo =  num_prinfo_*kInstrSize;
There should be spaces around '*'.

http://codereview.chromium.org/1128009/diff/46001/47004#newcode1961
src/arm/assembler-arm.cc:1961: if (((instr & (7*B25 | P | U | B | W | 15*B16 |
Off12Mask)) !=
It must be possible to rewrite this with a named mask and a named value.

http://codereview.chromium.org/1128009/diff/46001/47004#newcode1991
src/arm/assembler-arm.cc:1991: if (((instr & (7*B25 | P | U | B | W | 15*B16 |
Off12Mask)) !=
And this.

http://codereview.chromium.org/1128009/diff/46001/47004#newcode1993
src/arm/assembler-arm.cc:1993: continue;
Use braces on multi-line ifs.

http://codereview.chromium.org/1128009/diff/46001/47002
File src/arm/assembler-arm.h (right):

http://codereview.chromium.org/1128009/diff/46001/47002#newcode715
src/arm/assembler-arm.h:715: SBit s = LeaveCC, Condition cond = al, bool
use_movw_movt = true);
Having bool arguments makes the code unreadable at the call site.  We prefer
enums like SBit.

http://codereview.chromium.org/1128009/diff/46001/47002#newcode1085
src/arm/assembler-arm.h:1085: bool with_const_pool = true);
Also here the bool argument makes code unreadble and should be replaced with an
enum.

http://codereview.chromium.org/1128009/diff/46001/47007
File src/arm/disasm-arm.cc (right):

http://codereview.chromium.org/1128009/diff/46001/47007#newcode386
src/arm/disasm-arm.cc:386: ", #%d0000", imm);
This line has wrong alignment.

http://codereview.chromium.org/1128009/diff/46001/47008
File src/arm/macro-assembler-arm.cc (right):

http://codereview.chromium.org/1128009/diff/46001/47008#newcode130
src/arm/macro-assembler-arm.cc:130: mov(ip, Operand(target, rmode), LeaveCC,
cond, false);
There should be a comment here to explain why we don't want to use movw and
movt.