MIPS64: r6 compact branch optimization.

src/mips64/assembler-mips64-inl.h

Index: src/mips64/assembler-mips64-inl.h
diff --git a/src/mips64/assembler-mips64-inl.h b/src/mips64/assembler-mips64-inl.h
index a824ee7fc13c7d38a63a40dd079319c960498e9b..71078352871a1284f3b6e637ab685fe750205078 100644
--- a/src/mips64/assembler-mips64-inl.h
+++ b/src/mips64/assembler-mips64-inl.h
@@ -438,20 +438,40 @@ void Assembler::CheckTrampolinePoolQuick(int extra_instructions) {
 }
 
 
-void Assembler::emit(Instr x) {
+void Assembler::emit(Instr x, CompactBranchType is_compact_branch) {
   if (!is_buffer_growth_blocked()) {
     CheckBuffer();
   }
+  if (IsPrevInstrCompactBranch()) {
+    if (Instruction::IsForbiddenAfterBranchInstr(x)) {
+      // Nop instruction to preceed a CTI in forbidden slot:
+      Instr nop = SPECIAL | SLL;
+      *reinterpret_cast<Instr*>(pc_) = nop;
+      pc_ += kInstrSize;
+    }
+    ClearCompactBranchState();
+  }
   *reinterpret_cast<Instr*>(pc_) = x;
   pc_ += kInstrSize;
+  if (is_compact_branch == CompactBranchType::COMPACT_BRANCH) {
+    EmittedCompactBranchInstruction();
+  }
   CheckTrampolinePoolQuick();
 }
 
 
-void Assembler::emit(uint64_t x) {
+void Assembler::emit(uint64_t x, CompactBranchType is_compact_branch) {
   if (!is_buffer_growth_blocked()) {
     CheckBuffer();
   }
+  if (IsPrevInstrCompactBranch()) {
+    // Nop instruction to preceed a CTI in forbidden slot:

[Ilija.Pavlovic1, 2015/12/18 11:38:22]

Missing check IsForbiddenAfterBranchInstr(x)?

[balazs.kilvady, 2015/12/18 19:18:05]

Also when we emitting a 64-bit value that is not an instruction (we are emitting
64-bit addresses this way) I think, and that cannot be a
`IsForbiddenAfterBranchInstr(x)` but the upper 32 bits of a 64-bit value might
look like a branch istruction for the CPU. (For example a if the upper 32 bits
holds 8 that "looks like" a jr, as I remember. So the most significant 32 bits
of 'x' must be passed to IsForbiddenAfterBranchInstr() and also please take care
of little/big endian coding.

[ivica.bogosavljevic, 2015/12/22 10:22:39]

There is no need for IsForbiddenAfterBranchInstr(x) here because x is not an
instruction. We need to emit a NOP here unconditionally, and these are the
reasons: 
1) I believe that the instruction in the forbidden slot enters the decoding
phase in the CPU pipeline, this could create a condition of ReservedInstruction
exception even though the instruction itself doesn't get executed. (according to
the instruction manual this should not happen, but there will be many
architectures where this could actually happen)
2) The saving we get if we add this is not big. Normally there is one jump, and
a list of constants emitted one after another.
3) Additional complexity because we need to make a difference between big endian
and little endian encodings.

[balazs.kilvady, 2015/12/22 11:01:55]

I guess in this case the CPU could not know if the next 32 bits holds something
which won't be executed ever.
I think the endianess is #define-d so a
#ifdef LITTLEENDIAN
Instruction::IsForbiddenAfterBranchInstr(static_cast<Instr>(x));
#else
Instruction::IsForbiddenAfterBranchInstr(static_cast<Instr>(x >> 32));
#endif
is not a big deal. I would vote to add something like this. At least worth a
test if our assumptation is right. Otherwise we just put safety nop()s
everywhere.

[balazs.kilvady, 2016/01/04 13:38:39]

 I still miss  Instruction::IsForbiddenAfterBranchInstr(static_cast<Instr>([x |
x >> 32])); or at least a comment. I try to modify TEST(jump_tables4) in
test-macro-assembler-mip64.cc to test "corner cases".

+    Instr nop = SPECIAL | SLL;
+    *reinterpret_cast<Instr*>(pc_) = nop;
+    pc_ += kInstrSize;
+
+    ClearCompactBranchState();
+  }
   *reinterpret_cast<uint64_t*>(pc_) = x;
   pc_ += kInstrSize * 2;

[Ilija.Pavlovic1, 2015/12/18 11:38:22]

Missing check is_compact_branch == COMPACT_BRANCH ? (similar to lines 456 - 458)

[balazs.kilvady, 2015/12/18 19:18:05]

A 64-bit value cannot be a real instruction so it cannot be a compact branch, I
think.

[ivica.bogosavljevic, 2015/12/22 10:22:39]

Acknowledged.

[ivica.bogosavljevic, 2015/12/22 10:22:39]

Done.

   CheckTrampolinePoolQuick();