Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(146)

Issue 868803010: Revert of [arm] Assembler support for internal references. (Closed)

Created:
5 years, 10 months ago by Benedikt Meurer
Modified:
5 years, 10 months ago
CC:
v8-dev
Base URL:
https://chromium.googlesource.com/v8/v8.git@master
Target Ref:
refs/pending/heads/master
Project:
v8
Visibility:
Public.

Description

Revert of [arm] Assembler support for internal references. (patchset #1 id:1 of https://codereview.chromium.org/887073007/) Reason for revert: Android GN build broken. Original issue's description: > [arm] Assembler support for internal references. > > BUG=v8:3872 > LOG=n > R=verwaest@chromium.org > > Committed: https://chromium.googlesource.com/v8/v8/+/49cbe537e715c960fb9773af2e240133726f465b TBR=svenpanne@chromium.org,verwaest@chromium.org NOPRESUBMIT=true NOTREECHECKS=true NOTRY=true BUG=v8:3872

Patch Set 1 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+25 lines, -242 lines) Patch
M src/arm/assembler-arm.h View 1 chunk +0 lines, -1 line 0 comments Download
M src/arm/assembler-arm.cc View 5 chunks +25 lines, -74 lines 0 comments Download
M src/arm/constants-arm.h View 1 chunk +0 lines, -1 line 0 comments Download
M test/cctest/test-assembler-arm.cc View 1 chunk +0 lines, -166 lines 0 comments Download

Messages

Total messages: 4 (1 generated)
Benedikt Meurer
Created Revert of [arm] Assembler support for internal references.
5 years, 10 months ago (2015-02-06 07:35:52 UTC) #1
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/868803010/1
5 years, 10 months ago (2015-02-06 07:36:53 UTC) #2
commit-bot: I haz the power
5 years, 10 months ago (2015-02-06 07:37:00 UTC) #4
Failed to apply patch for src/arm/assembler-arm.cc:
While running git apply --index -3 -p1;
  error: patch failed: src/arm/assembler-arm.cc:3489
  error: repository lacks the necessary blob to fall back on 3-way merge.
  error: src/arm/assembler-arm.cc: patch does not apply

Patch:       src/arm/assembler-arm.cc
Index: src/arm/assembler-arm.cc
diff --git a/src/arm/assembler-arm.cc b/src/arm/assembler-arm.cc
index
baaf18af4f7a7178fdd498c9d62ecaee0289fc45..33b61a3480321fcf083ccd0cd7aa1ca5c5056c8f
100644
--- a/src/arm/assembler-arm.cc
+++ b/src/arm/assembler-arm.cc
@@ -228,8 +228,7 @@
 //
-----------------------------------------------------------------------------
 // Implementation of RelocInfo
 
-// static
-const int RelocInfo::kApplyMask = 1 << RelocInfo::INTERNAL_REFERENCE;
+const int RelocInfo::kApplyMask = 0;
 
 
 bool RelocInfo::IsCodedSpecially() {
@@ -797,20 +796,14 @@
     // Emitted link to a label, not part of a branch.
     return instr;
   }
-  if ((instr & 7 * B25) == 5 * B25) {
-    int imm26 = ((instr & kImm24Mask) << 8) >> 6;
-    // b, bl, or blx imm24
-    if ((Instruction::ConditionField(instr) == kSpecialCondition) &&
-        ((instr & B24) != 0)) {
-      // blx uses bit 24 to encode bit 2 of imm26
-      imm26 += 2;
-    }
-    return pos + kPcLoadDelta + imm26;
-  }
-  // Internal reference to the label.
-  DCHECK_EQ(15 * B28 | 7 * B25 | 1 * B0, instr & (15 * B28 | 7 * B25 | 1 *
B0));
-  int imm26 = (((instr >> 1) & kImm24Mask) << 8) >> 6;
-  return pos + imm26;
+  DCHECK((instr & 7*B25) == 5*B25);  // b, bl, or blx imm24
+  int imm26 = ((instr & kImm24Mask) << 8) >> 6;
+  if ((Instruction::ConditionField(instr) == kSpecialCondition) &&
+      ((instr & B24) != 0)) {
+    // blx uses bit 24 to encode bit 2 of imm26
+    imm26 += 2;
+  }
+  return pos + kPcLoadDelta + imm26;
 }
 
 
@@ -884,25 +877,19 @@
     }
     return;
   }
-  if ((instr & 7 * B25) == 5 * B25) {
-    // b, bl, or blx imm24
-    int imm26 = target_pos - (pos + kPcLoadDelta);
-    if (Instruction::ConditionField(instr) == kSpecialCondition) {
-      // blx uses bit 24 to encode bit 2 of imm26
-      DCHECK((imm26 & 1) == 0);
-      instr = (instr & ~(B24 | kImm24Mask)) | ((imm26 & 2) >> 1) * B24;
-    } else {
-      DCHECK((imm26 & 3) == 0);
-      instr &= ~kImm24Mask;
-    }
-    int imm24 = imm26 >> 2;
-    DCHECK(is_int24(imm24));
-    instr_at_put(pos, instr | (imm24 & kImm24Mask));
-    return;
-  }
-  // Patch internal reference to label.
-  DCHECK_EQ(15 * B28 | 7 * B25 | 1 * B0, instr & (15 * B28 | 7 * B25 | 1 *
B0));
-  instr_at_put(pos, reinterpret_cast<Instr>(buffer_ + target_pos));
+  int imm26 = target_pos - (pos + kPcLoadDelta);
+  DCHECK((instr & 7*B25) == 5*B25);  // b, bl, or blx imm24
+  if (Instruction::ConditionField(instr) == kSpecialCondition) {
+    // blx uses bit 24 to encode bit 2 of imm26
+    DCHECK((imm26 & 1) == 0);
+    instr = (instr & ~(B24 | kImm24Mask)) | ((imm26 & 2) >> 1)*B24;
+  } else {
+    DCHECK((imm26 & 3) == 0);
+    instr &= ~kImm24Mask;
+  }
+  int imm24 = imm26 >> 2;
+  DCHECK(is_int24(imm24));
+  instr_at_put(pos, instr | (imm24 & kImm24Mask));
 }
 
 
@@ -3439,16 +3426,9 @@
   reloc_info_writer.Reposition(reloc_info_writer.pos() + rc_delta,
                                reloc_info_writer.last_pc() + pc_delta);
 
-  // Relocate internal references.
-  for (RelocIterator it(desc); !it.done(); it.next()) {
-    if (it.rinfo()->rmode() == RelocInfo::INTERNAL_REFERENCE) {
-      // Don't patch unbound internal references (bit 0 set); those are still
-      // hooked up in the Label chain and will be automatically patched once
-      // the label is bound.
-      int32_t* p = reinterpret_cast<int32_t*>(it.rinfo()->pc());
-      if ((*p & 1 * B0) == 0) *p += pc_delta;
-    }
-  }
+  // None of our relocation types are pc relative pointing outside the code
+  // buffer nor pc absolute pointing inside the code buffer, so there is no
need
+  // to relocate any emitted relocation entries.
 
   // Relocate pending relocation entries.
   for (int i = 0; i < num_pending_32_bit_reloc_info_; i++) {
@@ -3489,35 +3469,6 @@
   CheckBuffer();
   *reinterpret_cast<uint32_t*>(pc_) = data;
   pc_ += sizeof(uint32_t);
-}
-
-
-void Assembler::dd(Label* label) {
-  CheckBuffer();
-  RecordRelocInfo(RelocInfo::INTERNAL_REFERENCE);
-  if (label->is_bound()) {
-    uint32_t data = reinterpret_cast<uint32_t>(buffer_ + label->pos());
-    DCHECK_EQ(0u, data & 1 * B0);
-    *reinterpret_cast<uint32_t*>(pc_) = data;
-    pc_ += sizeof(uint32_t);
-  } else {
-    int target_pos;
-    if (label->is_linked()) {
-      // Point to previous instruction that uses the link.
-      target_pos = label->pos();
-    } else {
-      // First entry of the link chain points to itself.
-      target_pos = pc_offset();
-    }
-    label->link_to(pc_offset());
-    // Encode internal reference to unbound label. We set the least significant
-    // bit to distinguish unbound internal references in GrowBuffer() below.
-    int imm26 = target_pos - pc_offset();
-    DCHECK_EQ(0, imm26 & 3);
-    int imm24 = imm26 >> 2;
-    DCHECK(is_int24(imm24));
-    emit(15 * B28 | 7 * B25 | ((imm24 & kImm24Mask) << 1) | 1 * B0);
-  }
 }

Powered by Google App Engine
This is Rietveld 408576698