MIPS: r6 compact branch optimization.

src/ic/mips/ic-mips.cc

 // Copyright 2012 the V8 project authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #if V8_TARGET_ARCH_MIPS
 
 #include "src/codegen.h"
 #include "src/ic/ic.h"
 #include "src/ic/ic-compiler.h"
 #include "src/ic/stub-cache.h"
@@ skipping 863 matching lines @@
 
   if (FLAG_trace_ic) {
     PrintF("[  patching ic at %p, andi=%p, delta=%d\n", address,
            andi_instruction_address, delta);
   }
 
   Address patch_address =
       andi_instruction_address - delta * Instruction::kInstrSize;
   Instr instr_at_patch = Assembler::instr_at(patch_address);
   Instr branch_instr =
       Assembler::instr_at(patch_address + Instruction::kInstrSize);

[paul.l..., 2015/10/15 01:51:24]

Add ' USE(branch_instr);' here or somewhere. After the change, all usages are in
DCHECK, so release builds give unused var warning/error.

[balazs.kilvady, 2015/10/30 21:11:14]

Done.

   // This is patching a conditional "jump if not smi/jump if smi" site.
   // Enabling by changing from
   //   andi at, rx, 0
   //   Branch <target>, eq, at, Operand(zero_reg)
   // to:
   //   andi at, rx, #kSmiTagMask
   //   Branch <target>, ne, at, Operand(zero_reg)
   // and vice-versa to be disabled again.
   CodePatcher patcher(patch_address, 2);
   Register reg = Register::from_code(Assembler::GetRs(instr_at_patch));
   if (check == ENABLE_INLINED_SMI_CHECK) {
     DCHECK(Assembler::IsAndImmediate(instr_at_patch));
     DCHECK_EQ(0u, Assembler::GetImmediate16(instr_at_patch));
     patcher.masm()->andi(at, reg, kSmiTagMask);
   } else {
     DCHECK_EQ(check, DISABLE_INLINED_SMI_CHECK);
     DCHECK(Assembler::IsAndImmediate(instr_at_patch));
     patcher.masm()->andi(at, reg, 0);
   }
   DCHECK(Assembler::IsBranch(branch_instr));
-  if (Assembler::IsBeq(branch_instr)) {
-    patcher.ChangeBranchCondition(ne);
-  } else {
-    DCHECK(Assembler::IsBne(branch_instr));
-    patcher.ChangeBranchCondition(eq);
-  }
+  patcher.ChangeBranchCondition();

[paul.l..., 2015/10/15 01:51:24]

I kind of like the old way, where you check if (eq) set-to(ne), etc.

What you have now is a just a toggle. Could this get out of sync in any way?

(I do understand extra complexity in the implementation of
ChangeBranchCondition())

[balazs.kilvady, 2015/10/30 21:11:14]

Done.

 }
 }  // namespace internal
 }  // namespace v8
 
 #endif  // V8_TARGET_ARCH_MIPS