[backend] Fix source position annotations

If two call instructions were generated right after each other, the
source position table could get populated with two entries for the same
PC (triggered by the follow-up CL: https://crrev.com/c/2697359).
This CL fixes that by slightly changing the carry-over of source
positions from nodes to instructions.

The call node which has a source position attached generates two
instructions:
      18: gap () ([rax|R|tp] = v16(-); [rbx|R|t] = v17(-);)
          [rax|R|t] = ArchCallWasmFunction [immediate:4] #-1 [rax|R|tp] [rbx|R|t] [immediate:5]
      19: gap () ()
          ArchJmp [immediate:6]

Those are then reversed, and the source position is attached to the first
one (the ArchJmp). After reversing it again later, the source position
will be set to the pc *after* the call instruction, which in the example
happened to be just another call instruction which already had a source
position, resulting in this code:

[...]
0x388ee467d426    66  e875feffff     call 0x388ee467d2a0     ;; wasm stub: WasmThrow
0x388ee467d42b    6b  e850feffff     call 0x388ee467d280     ;; wasm stub: WasmStackGuard
[...]
Source positions:
 pc offset  position
        6b         5
        6b         0

By attaching the source position to the *last* instruction (after
reversing), we ensure that it will be generated for an instruction
*before* the call, or the call itself if this is the first instruction
emitted for that node.

R=jgruber@chromium.org

Bug: v8:11490, v8:11496
Change-Id: Ie95c87d0d9daea56ca14a811abcd02ac07a4cf84
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2697358
Commit-Queue: Clemens Backes <clemensb@chromium.org>
Reviewed-by: Jakob Gruber <jgruber@chromium.org>
Cr-Commit-Position: refs/heads/master@{#72951}

src/codegen/source-position-table.cc

@@ -74,7 +74,11 @@ void EncodeInt(ZoneVector<byte>* bytes, T value) {
 // Encode a PositionTableEntry.
 void EncodeEntry(ZoneVector<byte>* bytes, const PositionTableEntry& entry) {
   // We only accept ascending code offsets.
-  DCHECK_GE(entry.code_offset, 0);
+  DCHECK_LE(0, entry.code_offset);
+  // All but the first entry must be *strictly* ascending (no two entries for
+  // the same position).
+  // TODO(11496): This DCHECK fails tests.
+  // DCHECK_IMPLIES(!bytes->empty(), entry.code_offset > 0);
   // Since code_offset is not negative, we use sign to encode is_statement.
   EncodeInt(bytes,
             entry.is_statement ? entry.code_offset : -entry.code_offset - 1);

src/compiler/backend/instruction-selector.cc

@@ -1164,8 +1164,7 @@ void InstructionSelector::VisitBlock(BasicBlock* block) {
     if (!source_positions_) return true;
     SourcePosition source_position = source_positions_->GetSourcePosition(node);
     if (source_position.IsKnown() && IsSourcePositionUsed(node)) {
-      sequence()->SetSourcePosition(instructions_[instruction_start],
-                                    source_position);
+      sequence()->SetSourcePosition(instructions_.back(), source_position);
     }
     return true;
   };
@@ -1173,8 +1172,9 @@ void InstructionSelector::VisitBlock(BasicBlock* block) {
   // Generate code for the block control "top down", but schedule the code
   // "bottom up".
   VisitControl(block);
-  if (!FinishEmittedInstructions(block->control_input(), current_block_end))
+  if (!FinishEmittedInstructions(block->control_input(), current_block_end)) {
     return;
+  }
 
   // Visit code in reverse control flow order, because architecture-specific
   // matching may cover more than one node at a time.

test/unittests/codegen/source-position-table-unittest.cc

@@ -61,7 +61,7 @@ TEST_F(SourcePositionTableTest, EncodeExpression) {
   CHECK(!builder()->ToSourcePositionTable(isolate()).is_null());
 }
 
-TEST_F(SourcePositionTableTest, EncodeAscending) {
+TEST_F(SourcePositionTableTest, EncodeAscendingPositive) {
   int code_offset = 0;
   int source_position = 0;
   for (size_t i = 0; i < arraysize(offsets); i++) {
@@ -74,7 +74,13 @@ TEST_F(SourcePositionTableTest, EncodeAscending) {
     }
   }
 
-  // Also test negative offsets for source positions:
+  CHECK(!builder()->ToSourcePositionTable(isolate()).is_null());
+}
+
+TEST_F(SourcePositionTableTest, EncodeAscendingNegative) {
+  int code_offset = 0;
+  // Start with a big source position, then decrement it.
+  int source_position = 1 << 26;
   for (size_t i = 0; i < arraysize(offsets); i++) {
     code_offset += offsets[i];
     source_position -= offsets[i];