MIPS: Replace JR/JALR with JIC/JIALC for r6

src/mips/assembler-mips.cc

Index: src/mips/assembler-mips.cc
diff --git a/src/mips/assembler-mips.cc b/src/mips/assembler-mips.cc
index 99b1bcd0c5916f77bca8adf32ea92bb8e77973bb..296635dedc6b9a347d71df3ce5a51c073969f810 100644
--- a/src/mips/assembler-mips.cc
+++ b/src/mips/assembler-mips.cc
@@ -541,6 +541,13 @@ bool Assembler::IsBnec(Instr instr) {
 }
 
 
+bool Assembler::IsJicOrJialc(Instr instr) {
+  uint32_t opcode = GetOpcodeField(instr);
+  uint32_t rs = GetRsField(instr);
+  return (opcode == POP66 || opcode == POP76) && rs == 0;
+}
+
+
 bool Assembler::IsJump(Instr instr) {
   uint32_t opcode   = GetOpcodeField(instr);
   uint32_t rt_field = GetRtField(instr);
@@ -553,6 +560,18 @@ bool Assembler::IsJump(Instr instr) {
 }
 
 
+int16_t Assembler::GetJicOrJialcOffset(Instr instr) {
+  DCHECK(IsJicOrJialc(instr));
+  return instr & kImm16Mask;
+}
+
+
+int16_t Assembler::GetLuiOffset(Instr instr) {
+  DCHECK(IsLui(instr));
+  return instr & kImm16Mask;
+}
+

[balazs.kilvady, 2016/01/12 11:30:40]

I would add 1 inline function to the header with this function body. Without
DCHECK() or modify Instruction::InstructionType to work for Instr type and check
if the instruction is a kImmediateType.

[miran.karic, 2016/02/18 08:38:04]

Acknowledged. There already exists a function GetImmediate16 in
assembler-mips.cc, maybe I can just use that one?

+
 bool Assembler::IsJ(Instr instr) {
   uint32_t opcode = GetOpcodeField(instr);
   // Checks if the instruction is a jump.
@@ -704,6 +723,58 @@ static inline int32_t AddBranchOffset(int pos, Instr instr) {
 }
 
 
+uint32_t Assembler::createTargetAddress(Instr instr_lui, Instr instr_jic) {

[paul.l..., 2016/01/11 22:34:57]

nit: Start with capital C: CreateTargetAddress, or change to all
lower_case_with_underscore. Here and two places below.

[miran.karic, 2016/02/18 08:38:05]

Done.

+  DCHECK(IsLui(instr_lui) && IsJicOrJialc(instr_jic));
+  int16_t jic_offset = GetJicOrJialcOffset(instr_jic);
+  int16_t lui_offset = GetLuiOffset(instr_lui);
+
+
+  if (jic_offset < 0) {
+    lui_offset += kImm16Mask;
+  }
+  uint32_t lui_offsetU = ((uint32_t)lui_offset) << kLuiShift;

[paul.l..., 2016/01/11 22:34:57]

nit: probably should not end in 'U', suggest '_u'

[miran.karic, 2016/02/18 08:38:04]

Done.

+  uint32_t jic_offsetU = ((uint32_t)jic_offset) & kImm16Mask;
+
+  return lui_offsetU | jic_offsetU;
+}
+
+
+// Idea is to use just lui and jic instructions. We will insert lower

[paul.l..., 2016/01/11 22:34:57]

nit: remove 'Idea is to ', and start with 'Use just ...'
Also, please correct the word-wrapping of this comment block. There should not
be single words on a line, except possibly the last line.

[miran.karic, 2016/02/18 08:38:04]

Done.

+// part of the target address in jic offset part. Because jic
+// sign-extends
+// offset and then add it with register, before that addition,
+// difference
+// between uppert part of the target address and upper part of the
+// sign-extended
+// offset (0xffff or 0x0000), will be inserted in jic register with lui
+// instruction.
+
+
+void Assembler::unpackTargetAddress(uint32_t address, int16_t& lui_offset,
+                                    int16_t& jic_offset) {
+  lui_offset = (address & kHiMask) >> kLuiShift;
+  jic_offset = address & kLoMask;
+
+  if (jic_offset < 0) {
+    lui_offset -= kImm16Mask;
+  }
+}
+
+
+void Assembler::unpackTargetAddressUnsigned(uint32_t address,
+                                            uint32_t& lui_offset,
+                                            uint32_t& jic_offset) {
+  int16_t lui_offset16 = (address & kHiMask) >> kLuiShift;
+  int16_t jic_offset16 = address & kLoMask;
+
+  if (jic_offset16 < 0) {
+    lui_offset16 -= kImm16Mask;
+  }
+  lui_offset = ((uint32_t)lui_offset16) & kImm16Mask;
+  jic_offset = ((uint32_t)jic_offset16) & kImm16Mask;
+}
+
+
 int Assembler::target_at(int pos, bool is_internal) {
   Instr instr = instr_at(pos);
   if (is_internal) {
@@ -726,15 +797,20 @@ int Assembler::target_at(int pos, bool is_internal) {
      }
   }
   // Check we have a branch or jump instruction.
-  DCHECK(IsBranch(instr) || IsLui(instr));
+  DCHECK(IsBranch(instr) || IsLui(instr) || IsJicOrJialc(instr));

[balazs.kilvady, 2016/01/12 11:30:40]

It looks like In the bellow code block that only instr2 can be JicOrJialc so I
think instr cannot be (only a lui) so this DCHECK should not be modified, I
think.

[miran.karic, 2016/02/18 08:38:04]

Acknowledged. At first I thought you are referring to the DCHECK several lines
below, since I see the comment below the line, but now I see what you mean. I
will fix this.

   if (IsBranch(instr)) {
     return AddBranchOffset(pos, instr);
   } else {
-    Instr instr_lui = instr_at(pos + 0 * Assembler::kInstrSize);
-    Instr instr_ori = instr_at(pos + 1 * Assembler::kInstrSize);
-    DCHECK(IsOri(instr_ori));
-    int32_t imm = (instr_lui & static_cast<int32_t>(kImm16Mask)) << kLuiShift;
-    imm |= (instr_ori & static_cast<int32_t>(kImm16Mask));
+    Instr instr1 = instr_at(pos + 0 * Assembler::kInstrSize);
+    Instr instr2 = instr_at(pos + 1 * Assembler::kInstrSize);
+    DCHECK(IsOri(instr2) || IsJicOrJialc(instr2));
+    int32_t imm;
+    if (IsJicOrJialc(instr2)) {
+      imm = createTargetAddress(instr1, instr2);
+    } else {
+      imm = (instr1 & static_cast<int32_t>(kImm16Mask)) << kLuiShift;
+      imm |= (instr2 & static_cast<int32_t>(kImm16Mask));
+    }
 
     if (imm == kEndOfJumpChain) {
       // EndOfChain sentinel is returned directly, not relative to pc or pos.
@@ -782,24 +858,34 @@ void Assembler::target_at_put(int32_t pos, int32_t target_pos,
     return;
   }
 
-  DCHECK(IsBranch(instr) || IsLui(instr));
+  DCHECK(IsBranch(instr) || IsLui(instr) || IsJicOrJialc(instr));

[balazs.kilvady, 2016/01/12 11:30:40]

Like at the previous DCHECK() comment.

[miran.karic, 2016/02/18 08:38:04]

Done.

   if (IsBranch(instr)) {
     instr = SetBranchOffset(pos, target_pos, instr);
     instr_at_put(pos, instr);
   } else {
-    Instr instr_lui = instr_at(pos + 0 * Assembler::kInstrSize);
-    Instr instr_ori = instr_at(pos + 1 * Assembler::kInstrSize);
-    DCHECK(IsOri(instr_ori));
+    Instr instr1 = instr_at(pos + 0 * Assembler::kInstrSize);
+    Instr instr2 = instr_at(pos + 1 * Assembler::kInstrSize);
+    DCHECK(IsOri(instr2) || IsJicOrJialc(instr2));
     uint32_t imm = reinterpret_cast<uint32_t>(buffer_) + target_pos;
     DCHECK((imm & 3) == 0);
-
-    instr_lui &= ~kImm16Mask;
-    instr_ori &= ~kImm16Mask;
-
-    instr_at_put(pos + 0 * Assembler::kInstrSize,
-                 instr_lui | ((imm & kHiMask) >> kLuiShift));
-    instr_at_put(pos + 1 * Assembler::kInstrSize,
-                 instr_ori | (imm & kImm16Mask));
+    DCHECK(IsLui(instr1) && (IsJicOrJialc(instr2) || IsOri(instr2)));
+    if (IsJicOrJialc(instr2)) {

[paul.l..., 2016/01/11 22:34:57]

Just for readability, can this if (IsJicOrJialc(instr2)) { } be combined with
the one just below the instr2 &= ~kImm16Mask; ? It doesn't look like there are
dependencies there.

[miran.karic, 2016/02/18 08:38:04]

Done.

+      uint32_t lui_offsetU, jic_offsetU;

[paul.l..., 2016/01/11 22:34:57]

nit: as before, don't use capital U at end like this. _u

[miran.karic, 2016/02/18 08:38:04]

Done.

+      unpackTargetAddressUnsigned(imm, lui_offsetU, jic_offsetU);
+    }
+    instr1 &= ~kImm16Mask;
+    instr2 &= ~kImm16Mask;
+    if (IsJicOrJialc(instr2)) {
+      uint32_t lui_offsetU, jic_offsetU;
+      unpackTargetAddressUnsigned(imm, lui_offsetU, jic_offsetU);
+      instr_at_put(pos + 0 * Assembler::kInstrSize, instr1 | lui_offsetU);
+      instr_at_put(pos + 1 * Assembler::kInstrSize, instr2 | jic_offsetU);
+    } else {
+      instr_at_put(pos + 0 * Assembler::kInstrSize,
+                   instr1 | ((imm & kHiMask) >> kLuiShift));
+      instr_at_put(pos + 1 * Assembler::kInstrSize,
+                   instr2 | (imm & kImm16Mask));
+    }
   }
 }
 
@@ -2790,24 +2876,37 @@ int Assembler::RelocateInternalReference(RelocInfo::Mode rmode, byte* pc,
   } else {
     DCHECK(RelocInfo::IsInternalReferenceEncoded(rmode));
     if (IsLui(instr)) {
-      Instr instr_lui = instr_at(pc + 0 * Assembler::kInstrSize);
-      Instr instr_ori = instr_at(pc + 1 * Assembler::kInstrSize);
-      DCHECK(IsOri(instr_ori));
-      int32_t imm = (instr_lui & static_cast<int32_t>(kImm16Mask)) << kLuiShift;
-      imm |= (instr_ori & static_cast<int32_t>(kImm16Mask));
+      Instr instr1 = instr_at(pc + 0 * Assembler::kInstrSize);
+      Instr instr2 = instr_at(pc + 1 * Assembler::kInstrSize);
+      DCHECK(IsOri(instr2) || IsJicOrJialc(instr2));
+      int32_t imm;
+      if (IsJicOrJialc(instr2)) {
+        imm = createTargetAddress(instr1, instr2);
+      } else {
+        imm = (instr1 & static_cast<int32_t>(kImm16Mask)) << kLuiShift;
+        imm |= (instr2 & static_cast<int32_t>(kImm16Mask));
+      }
+
       if (imm == kEndOfJumpChain) {
         return 0;  // Number of instructions patched.
       }
       imm += pc_delta;
       DCHECK((imm & 3) == 0);
 
-      instr_lui &= ~kImm16Mask;
-      instr_ori &= ~kImm16Mask;
+      instr1 &= ~kImm16Mask;
+      instr2 &= ~kImm16Mask;
 
-      instr_at_put(pc + 0 * Assembler::kInstrSize,
-                   instr_lui | ((imm >> kLuiShift) & kImm16Mask));
-      instr_at_put(pc + 1 * Assembler::kInstrSize,
-                   instr_ori | (imm & kImm16Mask));
+      if (IsJicOrJialc(instr2)) {
+        uint32_t lui_offsetU, jic_offsetU;
+        Assembler::unpackTargetAddressUnsigned(imm, lui_offsetU, jic_offsetU);
+        instr_at_put(pc + 0 * Assembler::kInstrSize, instr1 | lui_offsetU);
+        instr_at_put(pc + 1 * Assembler::kInstrSize, instr2 | jic_offsetU);
+      } else {
+        instr_at_put(pc + 0 * Assembler::kInstrSize,
+                     instr1 | ((imm >> kLuiShift) & kImm16Mask));
+        instr_at_put(pc + 1 * Assembler::kInstrSize,
+                     instr2 | (imm & kImm16Mask));
+      }
       return 2;  // Number of instructions patched.
     } else {
       UNREACHABLE();
@@ -2976,19 +3075,40 @@ void Assembler::CheckTrampolinePool() {
       }
 
       int pool_start = pc_offset();
-      for (int i = 0; i < unbound_labels_count_; i++) {
-        uint32_t imm32;
-        imm32 = jump_address(&after_pool);
-        { BlockGrowBufferScope block_buf_growth(this);
-          // Buffer growth (and relocation) must be blocked for internal
-          // references until associated instructions are emitted and available
-          // to be patched.
-          RecordRelocInfo(RelocInfo::INTERNAL_REFERENCE_ENCODED);
-          lui(at, (imm32 & kHiMask) >> kLuiShift);
-          ori(at, at, (imm32 & kImm16Mask));
+      if (IsMipsArchVariant(kMips32r6)) {
+        for (int i = 0; i < unbound_labels_count_; i++) {
+          uint32_t imm32;
+          imm32 = jump_address(&after_pool);
+          uint32_t lui_offset, jic_offset;
+          unpackTargetAddressUnsigned(imm32, lui_offset, jic_offset);
+          {
+            BlockGrowBufferScope block_buf_growth(this);
+            // Buffer growth (and relocation) must be blocked for internal
+            // references until associated instructions are emitted and
+            // available to be patched.
+            RecordRelocInfo(RelocInfo::INTERNAL_REFERENCE_ENCODED);
+            lui(at, lui_offset);
+            jic(at, jic_offset);
+          }
+          CheckBuffer();
+        }
+      } else {
+        for (int i = 0; i < unbound_labels_count_; i++) {
+          uint32_t imm32;
+          imm32 = jump_address(&after_pool);
+          {
+            BlockGrowBufferScope block_buf_growth(this);
+            // Buffer growth (and relocation) must be blocked for internal
+            // references until associated instructions are emitted and
+            // available to be patched.
+            RecordRelocInfo(RelocInfo::INTERNAL_REFERENCE_ENCODED);
+            lui(at, (imm32 & kHiMask) >> kLuiShift);
+            ori(at, at, (imm32 & kImm16Mask));
+          }
+          CheckBuffer();
+          jr(at);
+          nop();
         }
-        jr(at);
-        nop();
       }
       bind(&after_pool);
       trampoline_ = Trampoline(pool_start, unbound_labels_count_);
@@ -3012,10 +3132,10 @@ Address Assembler::target_address_at(Address pc) {
   Instr instr1 = instr_at(pc);
   Instr instr2 = instr_at(pc + kInstrSize);
   // Interpret 2 instructions generated by li: lui/ori
-  if ((GetOpcodeField(instr1) == LUI) && (GetOpcodeField(instr2) == ORI)) {
+  if (IsLui(instr1) && IsOri(instr2)) {
     // Assemble the 32 bit value.
-    return reinterpret_cast<Address>(
-        (GetImmediate16(instr1) << 16) | GetImmediate16(instr2));
+    return reinterpret_cast<Address>((GetImmediate16(instr1) << kLuiShift) |
+                                     GetImmediate16(instr2));
   }
 
   // We should never get here, force a bad address if we do.
@@ -3036,6 +3156,8 @@ void Assembler::QuietNaN(HeapObject* object) {
 // On Mips, a target address is stored in a lui/ori instruction pair, each
 // of which load 16 bits of the 32-bit address to a register.
 // Patching the address must replace both instr, and flush the i-cache.
+// On r6, target address is stored in a lui/jic pair, and both instr have to be
+// patched.
 //
 // There is an optimization below, which emits a nop when the address
 // fits in just 16 bits. This is unlikely to help, and should be benchmarked,
@@ -3051,15 +3173,27 @@ void Assembler::set_target_address_at(Isolate* isolate, Address pc,
 #ifdef DEBUG
   // Check we have the result from a li macro-instruction, using instr pair.
   Instr instr1 = instr_at(pc);
-  CHECK((GetOpcodeField(instr1) == LUI && GetOpcodeField(instr2) == ORI));
+  CHECK(IsLui(instr1) && (IsOri(instr2) || IsJicOrJialc(instr2)));
 #endif
 
-  // Must use 2 instructions to insure patchable code => just use lui and ori.
-  // lui rt, upper-16.
-  // ori rt rt, lower-16.
-  *p = LUI | rt_code | ((itarget & kHiMask) >> kLuiShift);
-  *(p + 1) = ORI | rt_code | (rt_code << 5) | (itarget & kImm16Mask);
+  if (IsJicOrJialc(instr2)) {
+    // Must use 2 instructions to insure patchable code => use lui and jic
+    uint32_t lui_offset, jic_offset;
+    Assembler::unpackTargetAddressUnsigned(itarget, lui_offset, jic_offset);
+
+    *p &= ~kImm16Mask;
+    *(p + 1) &= ~kImm16Mask;
 
+    *p |= lui_offset;
+    *(p + 1) |= jic_offset;
+
+  } else {
+    // Must use 2 instructions to insure patchable code => just use lui and ori.
+    // lui rt, upper-16.
+    // ori rt rt, lower-16.
+    *p = LUI | rt_code | ((itarget & kHiMask) >> kLuiShift);
+    *(p + 1) = ORI | rt_code | (rt_code << 5) | (itarget & kImm16Mask);
+  }
 
   if (icache_flush_mode != SKIP_ICACHE_FLUSH) {
     Assembler::FlushICache(isolate, pc, 2 * sizeof(int32_t));