Bug 1546407 - Wasm baseline compiler: fix inconsistent indexing in MachineStackTracker::{set,is}GCPointer. r=lhansen.
authorJulian Seward <jseward@acm.org>
Wed, 24 Apr 2019 06:59:50 +0200
changeset 529989 cfafd091d30107f974a22c06ef790bee7950c1cd
parent 529988 da523e237c3d922401d8d3e53fdabe14621d7eac
child 529990 df4c4083aff8d84ff17010249d08e7b6dc6f9beb
push id11265
push userffxbld-merge
push dateMon, 13 May 2019 10:53:39 +0000
treeherdermozilla-beta@77e0fe8dbdd3 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerslhansen
bugs1546407
milestone68.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1546407 - Wasm baseline compiler: fix inconsistent indexing in MachineStackTracker::{set,is}GCPointer. r=lhansen. MachineStackTracker::setGCPointer claims in a comment that its argument `offsetFromSP` is an index up from the lowest address denoted by the stack map. It implements that consistently with the comment. MachineStackTracker::isGCPointer makes the same claim in a comment, but actually implements the indexing in the opposite direction. This patch: * inverts the indexing direction of ::isGCPointer, making it consistent with the comments and with ::setGCPointer * changes the one-and-only use point of ::isGCPointer, in createStackMap(), accordingly. Rather than change the argument to ::isGCPointer, the iteration direction of the containing loop is inverted, since that fits better with the surrounding code. Differential Revision: https://phabricator.services.mozilla.com/D28594
js/src/wasm/WasmBaselineCompile.cpp
--- a/js/src/wasm/WasmBaselineCompile.cpp
+++ b/js/src/wasm/WasmBaselineCompile.cpp
@@ -2034,29 +2034,31 @@ class MachineStackTracker {
   // Notionally push |n| non-pointers on the stack.
   MOZ_MUST_USE bool pushNonGCPointers(size_t n) {
     return vec_.appendN(false, n);
   }
 
   // Mark the stack slot |offsetFromSP| up from the bottom as holding a
   // pointer.
   void setGCPointer(size_t offsetFromSP) {
-    // Offset 0 is the most recently pushed, offset 1 is the second most
-    // recently pushed item, etc.
+    // offsetFromSP == 0 denotes the most recently pushed item, == 1 the
+    // second most recently pushed item, etc.
     MOZ_ASSERT(offsetFromSP < vec_.length());
 
     size_t offsetFromTop = vec_.length() - 1 - offsetFromSP;
     numPtrs_ = numPtrs_ + 1 - (vec_[offsetFromTop] ? 1 : 0);
     vec_[offsetFromTop] = true;
   }
 
   // Query the pointerness of the slot |offsetFromSP| up from the bottom.
   bool isGCPointer(size_t offsetFromSP) {
     MOZ_ASSERT(offsetFromSP < vec_.length());
-    return vec_[offsetFromSP];
+
+    size_t offsetFromTop = vec_.length() - 1 - offsetFromSP;
+    return vec_[offsetFromTop];
   }
 
   // Return the number of words tracked by this MachineStackTracker.
   size_t length() { return vec_.length(); }
 
   // Return the number of pointer-typed words tracked by this
   // MachineStackTracker.
   size_t numPtrs() {
@@ -2353,17 +2355,17 @@ struct StackMapGenerator {
           stackMap->setBit(i);
         }
         i++;
       }
     }
     // Followed by the "main" part of the map.
     for (uint32_t i = 0; i < augmentedMstWords; i++) {
       if (augmentedMst.isGCPointer(i)) {
-        stackMap->setBit(numMappedWords - 1 - i);
+        stackMap->setBit(extraWords + i);
       }
     }
 
     stackMap->setExitStubWords(extraWords);
 
     // Record in the map, how far down from the highest address the Frame* is.
     // Take the opportunity to check that we haven't marked any part of the
     // Frame itself as a pointer.