Bug 1918970 part 10 - Define sort order for ranges with same start position. r=jseward

`std::sort` is not a stable sorting function. To avoid relying on implementation-defined
ordering, extend the comparator function to compare the `to` position and the bundle id
when ranges start at the same position.

Bundles now store an 'id' in non-debug builds too. Each compilation has its own
counter that starts at 0 so that we don't need atomic operations to determine the next
id.

Differential Revision: https://phabricator.services.mozilla.com/D222610
This commit is contained in:
Jan de Mooij 2024-09-19 07:22:54 +00:00
parent 81a75abe02
commit e9ea2d8aa9
2 changed files with 61 additions and 50 deletions

View File

@ -427,7 +427,7 @@
//
// LB2(parent=none v3 8-21 { 16_v3:A } ## v3 24-25 { 25_v3:F:xmm0 })
//
// LB merely indicates "LiveBundle", and the 2 is the debugId_ value (see
// LB merely indicates "LiveBundle", and the 2 is the id_ value (see
// below). This bundle has two LiveRanges
//
// v3 8-21 { 16_v3:A }
@ -515,9 +515,10 @@
// * else (it has a null spillParent_ field) it is a root node, and so other
// LiveBundles may point at it.
//
// When compiled with JS_JITSPEW, LiveBundle has a 32-bit `debugId_` field.
// This is used only for debug printing, and makes it easier to see
// parent-child relationships induced by the `spillParent_` pointers.
// LiveBundle has a 32-bit `id_` field. This is used for debug printing, and
// makes it easier to see parent-child relationships induced by the
// `spillParent_` pointers. It's also used for sorting live ranges in
// VirtualRegister::sortRanges.
//
// The "life cycle" of LiveBundles is described in Section 2 below.
//
@ -1123,10 +1124,23 @@ void VirtualRegister::sortRanges() {
}
// Sort ranges by start position in descending order.
auto compareRange = [](LiveRange* a, LiveRange* b) -> bool {
return a->from() > b->from();
//
// It would be correct to only compare the start position, but std::sort is
// not a stable sort and we don't want the order of ranges with the same start
// position to depend on the sort implementation. Use the end position and the
// bundle's id to ensure ranges are always sorted the same way.
auto compareRanges = [](LiveRange* a, LiveRange* b) -> bool {
if (a->from() != b->from()) {
return a->from() > b->from();
}
if (a->to() != b->to()) {
return a->to() > b->to();
}
// Overlapping live ranges must belong to different bundles.
MOZ_ASSERT_IF(a != b, a->bundle()->id() != b->bundle()->id());
return a->bundle()->id() > b->bundle()->id();
};
std::sort(ranges_.begin(), ranges_.end(), compareRange);
std::sort(ranges_.begin(), ranges_.end(), compareRanges);
rangesSorted_ = true;
}
@ -2188,7 +2202,8 @@ bool BacktrackingAllocator::tryMergeReusedRegister(VirtualRegister& def,
// The new range goes in a separate bundle, where it will be spilled during
// allocation.
LiveBundle* secondBundle = LiveBundle::FallibleNew(alloc(), nullptr, nullptr);
LiveBundle* secondBundle =
LiveBundle::FallibleNew(alloc(), nullptr, nullptr, getNextBundleId());
if (!secondBundle) {
return false;
}
@ -2207,7 +2222,8 @@ bool BacktrackingAllocator::mergeAndQueueRegisters() {
continue;
}
LiveBundle* bundle = LiveBundle::FallibleNew(alloc(), nullptr, nullptr);
LiveBundle* bundle =
LiveBundle::FallibleNew(alloc(), nullptr, nullptr, getNextBundleId());
if (!bundle) {
return false;
}
@ -2606,7 +2622,8 @@ bool BacktrackingAllocator::splitAt(LiveBundle* bundle,
bool spillBundleIsNew = false;
LiveBundle* spillBundle = bundle->spillParent();
if (!spillBundle) {
spillBundle = LiveBundle::FallibleNew(alloc(), bundle->spillSet(), nullptr);
spillBundle = LiveBundle::FallibleNew(alloc(), bundle->spillSet(), nullptr,
getNextBundleId());
if (!spillBundle) {
return false;
}
@ -2636,8 +2653,8 @@ bool BacktrackingAllocator::splitAt(LiveBundle* bundle,
LiveBundleVector newBundles;
// The bundle which ranges are currently being added to.
LiveBundle* activeBundle =
LiveBundle::FallibleNew(alloc(), bundle->spillSet(), spillBundle);
LiveBundle* activeBundle = LiveBundle::FallibleNew(
alloc(), bundle->spillSet(), spillBundle, getNextBundleId());
if (!activeBundle || !newBundles.append(activeBundle)) {
return false;
}
@ -2651,8 +2668,8 @@ bool BacktrackingAllocator::splitAt(LiveBundle* bundle,
LiveRange* range = *iter;
if (UseNewBundle(splitPositions, range->from(), &activeSplitPosition)) {
activeBundle =
LiveBundle::FallibleNew(alloc(), bundle->spillSet(), spillBundle);
activeBundle = LiveBundle::FallibleNew(alloc(), bundle->spillSet(),
spillBundle, getNextBundleId());
if (!activeBundle || !newBundles.append(activeBundle)) {
return false;
}
@ -2691,8 +2708,8 @@ bool BacktrackingAllocator::splitAt(LiveBundle* bundle,
activeRange->usesBegin()->pos != use->pos ||
activeRange->usesBegin()->usePolicy() == LUse::FIXED ||
use->usePolicy() == LUse::FIXED)) {
activeBundle =
LiveBundle::FallibleNew(alloc(), bundle->spillSet(), spillBundle);
activeBundle = LiveBundle::FallibleNew(
alloc(), bundle->spillSet(), spillBundle, getNextBundleId());
if (!activeBundle || !newBundles.append(activeBundle)) {
return false;
}
@ -2883,8 +2900,8 @@ bool BacktrackingAllocator::trySplitAcrossHotcode(LiveBundle* bundle,
return splitAt(bundle, splitPositions);
}
LiveBundle* hotBundle = LiveBundle::FallibleNew(alloc(), bundle->spillSet(),
bundle->spillParent());
LiveBundle* hotBundle = LiveBundle::FallibleNew(
alloc(), bundle->spillSet(), bundle->spillParent(), getNextBundleId());
if (!hotBundle) {
return false;
}
@ -2893,8 +2910,8 @@ bool BacktrackingAllocator::trySplitAcrossHotcode(LiveBundle* bundle,
LiveBundle* coldBundle = nullptr;
if (testbed) {
coldBundle = LiveBundle::FallibleNew(alloc(), bundle->spillSet(),
bundle->spillParent());
coldBundle = LiveBundle::FallibleNew(
alloc(), bundle->spillSet(), bundle->spillParent(), getNextBundleId());
if (!coldBundle) {
return false;
}
@ -2923,8 +2940,9 @@ bool BacktrackingAllocator::trySplitAcrossHotcode(LiveBundle* bundle,
}
} else {
if (!preBundle) {
preBundle = LiveBundle::FallibleNew(alloc(), bundle->spillSet(),
bundle->spillParent());
preBundle =
LiveBundle::FallibleNew(alloc(), bundle->spillSet(),
bundle->spillParent(), getNextBundleId());
if (!preBundle) {
return false;
}
@ -2944,8 +2962,9 @@ bool BacktrackingAllocator::trySplitAcrossHotcode(LiveBundle* bundle,
}
} else {
if (!postBundle) {
postBundle = LiveBundle::FallibleNew(alloc(), bundle->spillSet(),
bundle->spillParent());
postBundle =
LiveBundle::FallibleNew(alloc(), bundle->spillSet(),
bundle->spillParent(), getNextBundleId());
if (!postBundle) {
return false;
}
@ -4490,12 +4509,12 @@ UniqueChars LiveRange::toString() const {
UniqueChars LiveBundle::toString() const {
AutoEnterOOMUnsafeRegion oomUnsafe;
UniqueChars buf = JS_smprintf("LB%u(", debugId());
UniqueChars buf = JS_smprintf("LB%u(", id());
if (buf) {
if (spillParent()) {
buf = JS_sprintf_append(std::move(buf), "parent=LB%u",
spillParent()->debugId());
buf =
JS_sprintf_append(std::move(buf), "parent=LB%u", spillParent()->id());
} else {
buf = JS_sprintf_append(std::move(buf), "parent=none");
}

View File

@ -385,14 +385,6 @@ class SpillSet : public TempObject {
void setAllocation(LAllocation alloc);
};
#ifdef JS_JITSPEW
// See comment on LiveBundle::debugId_ just below. This needs to be atomic
// because TSan automation runs on debug builds will otherwise (correctly)
// report a race.
static mozilla::Atomic<uint32_t> LiveBundle_debugIdCounter =
mozilla::Atomic<uint32_t>{0};
#endif
// A set of live ranges which are all pairwise disjoint. The register allocator
// attempts to find allocations for an entire bundle, and if it fails the
// bundle will be broken into smaller ones which are allocated independently.
@ -412,24 +404,19 @@ class LiveBundle : public TempObject {
// will not be split.
LiveBundle* spillParent_;
#ifdef JS_JITSPEW
// This is used only for debug-printing bundles. It gives them an
// This is used for debug-printing bundles. It gives them an
// identifiable identity in the debug output, which they otherwise wouldn't
// have.
uint32_t debugId_;
#endif
// have. It's also used for sorting VirtualRegister's live ranges; see the
// comment in VirtualRegister::sortRanges.
const uint32_t id_;
LiveBundle(SpillSet* spill, LiveBundle* spillParent)
: spill_(spill), spillParent_(spillParent) {
#ifdef JS_JITSPEW
debugId_ = LiveBundle_debugIdCounter++;
#endif
}
LiveBundle(SpillSet* spill, LiveBundle* spillParent, uint32_t id)
: spill_(spill), spillParent_(spillParent), id_(id) {}
public:
static LiveBundle* FallibleNew(TempAllocator& alloc, SpillSet* spill,
LiveBundle* spillParent) {
return new (alloc.fallible()) LiveBundle(spill, spillParent);
LiveBundle* spillParent, uint32_t id) {
return new (alloc.fallible()) LiveBundle(spill, spillParent, id);
}
using RangeIterator = InlineForwardListIterator<LiveRange>;
@ -467,9 +454,9 @@ class LiveBundle : public TempObject {
LiveBundle* spillParent() const { return spillParent_; }
#ifdef JS_JITSPEW
uint32_t debugId() const { return debugId_; }
uint32_t id() const { return id_; }
#ifdef JS_JITSPEW
// Return a string describing this bundle.
UniqueChars toString() const;
#endif
@ -734,6 +721,9 @@ class BacktrackingAllocator : protected RegisterAllocator {
Vector<LiveBundle*, 4, BackgroundSystemAllocPolicy> spilledBundles;
// The bundle id that will be used for the next LiveBundle that's allocated.
uint32_t nextBundleId_ = 0;
using LiveBundleVector = Vector<LiveBundle*, 4, BackgroundSystemAllocPolicy>;
// Misc accessors
@ -746,6 +736,8 @@ class BacktrackingAllocator : protected RegisterAllocator {
return vregs[alloc->toUse()->virtualRegister()];
}
uint32_t getNextBundleId() { return nextBundleId_++; }
// Helpers for creating and adding MoveGroups
[[nodiscard]] bool addMove(LMoveGroup* moves, LiveRange* from, LiveRange* to,
LDefinition::Type type) {