AOT: follow-up to #972: crashes, wrong answers, bun test --aot, build time

Source/JavaScriptCore/aot/AOT.cmake

@@ -343,6 +343,7 @@ set(JavaScriptCore_AOT_TEST_SOURCES
     aot/tests/AOTObjectSlotExitTests.cpp
     aot/tests/AOTOperationScopeTests.cpp
     aot/tests/AOTOptionWordTests.cpp
+    aot/tests/AOTOriginalBesideMockTests.cpp
     aot/tests/AOTParsedOptionalMemberTests.cpp
     aot/tests/AOTPassTestCommands.cpp
     aot/tests/AOTPipelineTestCommands.cpp

Source/JavaScriptCore/aot/analysis/AOTBlockCycles.cpp

@@ -292,6 +292,30 @@ BlockCycles::BlockCycles(CFG& cfg, const Function* function)
         ForestBuilder(cfg, function).findOutermostCycles(m_outermost, m_membersOfOutermostCycles, m_firstMemberOfOutermostCycle);
 }
 
+bool BlockCycles::isInCycleAvoiding(BlockID block, BlockID avoidedBlock, uint64_t& numberOfBlocksVisited) const
+{
+    ++numberOfBlocksVisited;
+    std::optional<unsigned> cycle = outermostCycleOf(block);
+    if (!cycle || block == avoidedBlock)
+        return false;
+    if (outermostCycleOf(avoidedBlock) != cycle)
+        return true;
+    BitVector isVisited(m_cfg.numNodes());
+    isVisited.set(avoidedBlock.index());
+    Vector<unsigned, 16> worklist { block.index() };
+    while (!worklist.isEmpty()) {
+        for (BlockID successor : m_cfg.successors(BlockID(worklist.takeLast()))) {
+            if (successor == block)
+                return true;
+            if (m_outermost[successor.index()] != *cycle || isVisited.set(successor.index()))
+                continue;
+            ++numberOfBlocksVisited;
+            worklist.append(successor.index());
+        }
+    }
+    return false;
+}
+
 void BlockCycles::computeForest() const
 {
     m_forest.emplace(m_cfg, ForestBuilder(m_cfg, m_functionWithFrequencies).run(m_outermost.span(), m_membersOfOutermostCycles.span(), m_firstMemberOfOutermostCycle.span()));

Source/JavaScriptCore/aot/analysis/AOTBlockCycles.h

@@ -33,6 +33,7 @@ class BlockCycles {
     }
 
     bool areInSameCycle(BlockID a, BlockID b) const { return isInCycle(a) && outermostCycleOf(a) == outermostCycleOf(b); }
+    bool isInCycleAvoiding(BlockID, BlockID avoidedBlock, uint64_t& numberOfBlocksVisited) const;
 
     std::optional<unsigned> outermostCycleOf(BlockID block) const
     {

Source/JavaScriptCore/aot/analysis/AOTEscapeAnalysis.h

@@ -302,6 +302,9 @@ class EscapeAnalysis {
             listUses();
         return { entriesOf(object, m_uses, m_firstUse), m_sharedUses.span().data() };
     }
+    // What goes by an entry of usesOf() alone can be kept under the entry's index, for all the objects that share it.
+    uint32_t numberOfSharedUses() const { return m_sharedUses.size(); }
+    uint32_t indexOfSharedUse(const ObjectUse& entry) const { return &entry - m_sharedUses.span().data(); }
     std::span<const ObjectID> contentsOf(ObjectID object) const LIFETIME_BOUND { return entriesOf(object, m_contents, m_firstContent); }
     std::span<const ObjectID> containersOf(ObjectID object) const LIFETIME_BOUND { return entriesOf(object, m_containers, m_firstContainer); }
     bool isInLocalHeapWords(ObjectID object) const { return m_objectsInLocalHeapWords.contains(object); }

Source/JavaScriptCore/aot/analysis/AOTFunctionAnalyses.cpp

@@ -495,6 +495,8 @@ FunctionAnalyses::~FunctionAnalyses()
         statistics.count(StatisticID::EffectsComputed, numberOfComputedAnswers);
     if (uint64_t numberOfKeptAnswers = m_effects.numberOfKeptAnswers())
         statistics.count(StatisticID::EffectsKept, numberOfKeptAnswers);
+    if (m_numberOfBlocksReadToBuildControlFlowGraphs)
+        statistics.count(StatisticID::BlocksReadToBuildControlFlowGraphs, m_numberOfBlocksReadToBuildControlFlowGraphs);
 }
 
 void FunctionAnalyses::reportQuestionBeforeRepair() const
@@ -719,6 +721,7 @@ CFG& FunctionAnalyses::cfg()
     dropResultsOfFormerGraph();
     if (!m_controlFlow.cfg) {
         m_controlFlow.cfg.emplace(m_function);
+        m_numberOfBlocksReadToBuildControlFlowGraphs += m_function.blocks.size();
         m_controlFlow.isReadByUseLists = false;
         m_controlFlow.numberOfBlocks = m_function.blocks.size();
         didBuild(FunctionAnalysis::ControlFlow);

Source/JavaScriptCore/aot/analysis/AOTFunctionAnalyses.h

@@ -282,6 +282,7 @@ class FunctionAnalyses {
     std::optional<EffectSummaries> m_unknownSummaries;
     const EffectContext m_effectContext;
     EffectsCache m_effects;
+    uint64_t m_numberOfBlocksReadToBuildControlFlowGraphs { 0 };
     ControlFlowResults m_controlFlow;
     ValueResults m_values;
     const bool m_countsBuilds;

Source/JavaScriptCore/aot/analysis/AOTGenericCopy.cpp

@@ -711,6 +711,19 @@ TypeID GenericCopy::typeAssumedInTypedCode(ValueID value, TypeID type, const Bit
     return type;
 }
 
+bool GenericCopy::isTestOfBranchInOtherBlockThatPassesProofs(ValueID value) const
+{
+    const UseLists& useLists = m_analyses.useLists();
+    return std::ranges::any_of(useLists.usesOf(value), [&](const Use& use) {
+        if (use.kind != Use::Kind::TerminatorOperand || use.block == useLists.blockOf(value))
+            return false;
+        const Terminator& terminator = m_function.at(use.block).terminator;
+        return terminator.isConditionalBranch() && std::ranges::any_of(terminator.targets, [](const Target& target) {
+            return !target.proofs.isEmpty();
+        });
+    });
+}
+
 auto GenericCopy::planRejoin(const TestedRejoin& rejoin, const RejoinTypes& rejoinTypes, const BitVector& isCopy) -> std::optional<RejoinPlan>
 {
     TypeArena& types = m_context.types;
@@ -767,6 +780,8 @@ auto GenericCopy::planRejoin(const TestedRejoin& rejoin, const RejoinTypes& rejo
         Type passing = given.castTo(testable, types);
         if (passing.isNever() || !passing.isSubsetOf(wanted, program))
             return false;
+        if (!parameterIndex && isTestOfBranchInOtherBlockThatPassesProofs(inTypedCode))
+            return false;
         plan.tests.append(RejoinTest { parameterIndex, inTypedCode, types.intern(testable) });
         return true;
     };

Source/JavaScriptCore/aot/analysis/AOTGenericCopy.h

@@ -138,6 +138,7 @@ class GenericCopy {
     bool mergesAllocationThatSinksWithItsCopyAt(BlockID) const;
     bool mergesGeneratorWithItsCopyAt(BlockID) const;
     bool canLeaveCopyAt(BlockID) const;
+    bool isTestOfBranchInOtherBlockThatPassesProofs(ValueID) const;
     bool handsOverTooMuchAtExits() const;
     bool tryPlan(Extent, uint64_t maxBlocks, const RegionCloner::LivenessSource&);
     void recordTestedRejoins();

Source/JavaScriptCore/aot/analysis/AOTGuardAvailability.cpp

@@ -292,7 +292,7 @@ class KillQuery {
         if (!isConfined(subject)) {
             bool wasThereAtEntry = namesOnlyObjectsOfKind(ObjectKind::EntryParameter, subject);
             return mayWriteBeyondOperands(written) || std::ranges::any_of(operandsWrittenThrough(written), [&](ValueID operand) {
-                return escape.mayAlias(operand, subject) && !(wasThereAtEntry && namesOnlyObjectsOfKind(ObjectKind::Allocation, operand));
+                return escape.mayAlias(operand, subject) && !(wasThereAtEntry && namesOnlyObjectsOfKind(ObjectKind::Allocation, operand)) && !wasThereBeforeWhatIsNamedBy(operand, subject);
             });
         }
 
@@ -302,6 +302,25 @@ class KillQuery {
         });
     }
 
+    // What was computed once, ahead of every allocation that the operand names, is none of those objects. Only what judges finished code is told: told to the passes it moves code.
+    bool wasThereBeforeWhatIsNamedBy(ValueID operand, ValueID subject)
+    {
+        if (!m_analyses.context().onlyJudges() || !namesOnlyObjectsOfKind(ObjectKind::Allocation, operand))
+            return false;
+        ValueLocation computed = m_analyses.useLists().locationOf(m_analyses.identityOf(subject));
+        if (computed.placement != ValuePlacement::InBlock || m_analyses.isInCycle(computed.block))
+            return false;
+        const EscapeAnalysis& escape = m_analyses.escape();
+        for (ObjectID object : escape.pointsTo(operand).objects) {
+            ValueLocation allocation = m_analyses.useLists().locationOf(escape.at(object).definition);
+            if (allocation.placement != ValuePlacement::InBlock)
+                return false;
+            if (allocation.block == computed.block ? allocation.index <= computed.index : !m_analyses.dominators().dominates(computed.block, allocation.block))
+                return false;
+        }
+        return true;
+    }
+
     bool mayBeMadeHere(ValueID operand)
     {
         return !m_analyses.context().onlyJudges() || !m_writer || m_analyses.escape().pointsTo(operand).objects.isEmpty() || !namesOnlyWhatCodeOutsideReaches(m_analyses, operand);
@@ -377,9 +396,29 @@ class KillQuery {
                 return use.kind == Use::Kind::Operand && use.user == m_writer;
             return use.kind == Use::Kind::TerminatorOperand && use.block == m_point.block;
         };
-        return std::ranges::any_of(m_analyses.escape().usesOf(object), [&](const ObjectUse& objectUse) {
-            return objectUse.isEscape && isUseByWriter(objectUse.use) && (!m_writer || mayBeMadeHere(m_function.at(m_writer).operand(objectUse.use.operandIndex)));
-        });
+        // The uses of an object stand in the order of the function, so those at one place stand together.
+        const UseLists& useLists = m_analyses.useLists();
+        const CFG& cfg = m_analyses.cfg();
+        auto placeOf = [&](ProgramPoint point) {
+            return std::pair { cfg.reversePostOrderIndex(point.block), point.index };
+        };
+        auto placeOfWriter = placeOf(m_writer ? useLists.pointOf(m_writer) : pointBeforeTerminatorOf(m_function, m_point.block));
+        ObjectUses uses = m_analyses.escape().usesOf(object);
+        AOT_THRESHOLD(size_t, fewestUsesSearched, 16);
+        size_t first = 0;
+        for (size_t end = uses.size() < fewestUsesSearched ? 0 : uses.size(); first < end;) {
+            size_t middle = first + (end - first) / 2;
+            if (placeOf(useLists.pointOf(uses[middle].use)) < placeOfWriter)
+                first = middle + 1;
+            else
+                end = middle;
+        }
+        bool escapes = false;
+        for (size_t index = first; !escapes && index < uses.size() && (uses.size() < fewestUsesSearched || placeOf(useLists.pointOf(uses[index].use)) == placeOfWriter); ++index) {
+            const ObjectUse& objectUse = uses[index];
+            escapes = objectUse.isEscape && isUseByWriter(objectUse.use) && (!m_writer || mayBeMadeHere(m_function.at(m_writer).operand(objectUse.use.operandIndex)));
+        }
+        return escapes;
     }
 
     std::span<const WrittenOperand> writtenOperands()

Source/JavaScriptCore/aot/analysis/AOTGuardFacts.cpp

@@ -861,7 +861,10 @@ static ValueID valueStoredDirectlyBehind(FunctionAnalyses& analyses, ValueID all
         bool storesIntoObject = (value.opcode == Opcode::PutSlot || value.opcode == Opcode::PutSlotOrAdd) && value.objectOperand() == allocation;
         if (storesIntoObject && value.key() == key)
             return value.valueOperand();
-        if (!storesIntoObject && !analyses.hasNoEffects(values[index]))
+        // A closure that is made takes the object along and looks at nothing: nobody calls it before the stores have run. The end of the birth takes a mark off the object and leaves its slots alone.
+        bool endsBirth = (value.opcode == Opcode::FinishBirth || value.opcode == Opcode::TryFinishBirth) && value.objectOperand() == allocation;
+        bool leavesSlotsAlone = (value.opcode == Opcode::NewClosure || endsBirth) && !analyses.context().build.isSabotaged(Sabotage::ClosuresMadeBehindAllocationsHaveEffects);
+        if (!storesIntoObject && !leavesSlotsAlone && !analyses.hasNoEffects(values[index]))
             return { };
     }
     return { };

Source/JavaScriptCore/aot/analysis/AOTIRBuilder.cpp

@@ -277,7 +277,7 @@ Terminator IRBuilder::makeTerminator(TerminatorKind kind, std::initializer_list<
 ValueID IRBuilder::addValue(Value&& value)
 {
     m_function.add(WTF::move(value));
-    return ValueID(m_function.values.size() - 1);
+    return m_function.idOfValueAt(m_function.values.size() - 1);
 }
 
 TypeID partInside(const Program& program, TypeArena& types, TypeID narrower, TypeID type)

Source/JavaScriptCore/aot/analysis/AOTIntegerRelations.cpp

@@ -45,11 +45,9 @@ static bool isInScope(FunctionAnalyses& analyses, BlockID scope, BlockID at)
     return scope == at || analyses.dominators().terminatorDominates(scope, at);
 }
 
-static std::optional<ValueID> findValueOnEdge(const Function& function, const auto& assumptions, Edge edge, uint32_t parameterIndex)
+static std::optional<ValueID> findValueOnEdge(const Function& function, const BitVector& isScopeOfAssumption, Edge edge, uint32_t parameterIndex)
 {
-    bool mayLeaveBeforeAssumptionHolds = edge.isHandlerEdge() && std::ranges::any_of(assumptions, [&](const auto& assumption) {
-        return assumption.scope == edge.from;
-    });
+    bool mayLeaveBeforeAssumptionHolds = edge.isHandlerEdge() && isScopeOfAssumption.get(edge.from.index());
     if (mayLeaveBeforeAssumptionHolds)
         return std::nullopt;
 
@@ -96,6 +94,12 @@ IntegerRelations::IntegerRelations(FunctionAnalyses& analyses)
 {
 }
 
+IntegerRelations::~IntegerRelations()
+{
+    if (m_assumptionsRead)
+        m_analyses.context().build.statistics().count(StatisticID::AssumptionsReadForIntegerRelations, m_assumptionsRead);
+}
+
 IntegerRelations::Answer IntegerRelations::isAtMost(ValueID left, ValueID right, int64_t offset, BlockID at)
 {
     m_steps = 0;
@@ -154,16 +158,26 @@ void IntegerRelations::assumeRange(ValueID id, Range range, BlockID scope)
     std::optional<int64_t> maximum = exactValueOf(CheckedInt64(range.maximum) - term.offset);
     if (!minimum || !maximum)
         return;
-    m_assumptions.append({ term.base, { *minimum, *maximum }, scope });
+    size_t numberOfEntries = m_lastAssumptionAbout.size();
+    if (numberOfEntries <= term.base.index()) {
+        m_lastAssumptionAbout.grow(std::max<size_t>(m_function.values.size(), term.base.index() + 1));
+        std::ranges::fill(m_lastAssumptionAbout.mutableSpan().subspan(numberOfEntries), 0);
+    }
+    uint32_t earlier = std::exchange(m_lastAssumptionAbout[term.base.index()], static_cast<uint32_t>(m_assumptions.size() + 1));
+    m_assumptions.append({ term.base, { *minimum, *maximum }, scope, earlier });
+    m_isScopeOfAssumption.set(scope.index());
 }
 
 std::optional<Range> IntegerRelations::assumedRangeOf(ValueID base, BlockID at) const
 {
     std::optional<Range> range;
-    for (const Assumption& assumption : m_assumptions) {
-        if (assumption.value != base || !isInScope(m_analyses, assumption.scope, at))
-            continue;
-        range = range ? range->intersectWith(assumption.range) : assumption.range;
+    if (!base || base.index() >= m_lastAssumptionAbout.size())
+        return range;
+    for (uint32_t number = m_lastAssumptionAbout[base.index()]; number; number = m_assumptions[number - 1].earlierAboutSameValue) {
+        const Assumption& assumption = m_assumptions[number - 1];
+        ++m_assumptionsRead;
+        if (isInScope(m_analyses, assumption.scope, at))
+            range = range ? range->intersectWith(assumption.range) : assumption.range;
     }
     return range;
 }
@@ -489,7 +503,7 @@ bool IntegerRelations::proveByInduction(ValueID left, ValueID right, int64_t off
 
     goals.append({ left, right, offset });
     bool holdsOnEveryEdge = std::ranges::all_of(cfg.incomingEdges(location.block), [&](Edge edge) {
-        std::optional<ValueID> incoming = findValueOnEdge(m_function, m_assumptions, edge, location.index);
+        std::optional<ValueID> incoming = findValueOnEdge(m_function, m_isScopeOfAssumption, edge, location.index);
         if (!incoming)
             return false;
         Term argument = normalize(*incoming);
@@ -593,7 +607,7 @@ bool IntegerRelations::proveParameterAtLeast(ValueID parameter, int64_t minimum,
 
     goals.append({ parameter, minimum });
     bool holdsOnEveryEdge = std::ranges::all_of(cfg.incomingEdges(location.block), [&](Edge edge) {
-        std::optional<ValueID> incoming = findValueOnEdge(m_function, m_assumptions, edge, location.index);
+        std::optional<ValueID> incoming = findValueOnEdge(m_function, m_isScopeOfAssumption, edge, location.index);
         return incoming && proveValueAtLeast(*incoming, minimum, edge.from, goals);
     });
     goals.removeLast();
@@ -632,7 +646,7 @@ bool IntegerRelations::proveWhole(ValueID id, BlockID at, AssumedWholeParameters
 
     assumed.append(id);
     bool receivesOnlyWholeNumbers = std::ranges::all_of(incomingEdges, [&](Edge edge) {
-        std::optional<ValueID> incoming = findValueOnEdge(m_function, m_assumptions, edge, location.index);
+        std::optional<ValueID> incoming = findValueOnEdge(m_function, m_isScopeOfAssumption, edge, location.index);
         return incoming && proveWhole(*incoming, edge.from, assumed);
     });
     assumed.removeLast();

Source/JavaScriptCore/aot/analysis/AOTIntegerRelations.h

@@ -12,6 +12,7 @@
 #include "AOTVector.h"
 #include <optional>
 #include <utility>
+#include <wtf/BitVector.h>
 #include <wtf/HashMap.h>
 #include <wtf/Noncopyable.h>
 
@@ -21,6 +22,7 @@ class IntegerRelations {
     WTF_MAKE_NONCOPYABLE(IntegerRelations);
 public:
     explicit IntegerRelations(FunctionAnalyses&);
+    ~IntegerRelations();
 
     enum class Answer : uint8_t { Unknown, Proven };
 
@@ -48,6 +50,8 @@ class IntegerRelations {
         ValueID value;
         Range range;
         BlockID scope;
+        // The one made before it about the same value, counted from one. 0: there is none.
+        uint32_t earlierAboutSameValue { 0 };
     };
 
     struct LowerBoundGoal {
@@ -96,6 +100,11 @@ class IntegerRelations {
     const Program& m_program;
     const Function& m_function;
     Vector<Assumption, 2> m_assumptions;
+    // One entry a value: the last assumption about it, counted from one. A function that reads a length in a thousand places has a thousand assumptions,
+    // and one question asks for those about a few values many times.
+    Vector<uint32_t> m_lastAssumptionAbout;
+    BitVector m_isScopeOfAssumption;
+    mutable uint64_t m_assumptionsRead { 0 };
     Vector<uint32_t> m_nextBlockBehindComparison;
     struct ComparedBases {
         ValueID left;

Source/JavaScriptCore/aot/analysis/AOTObservableOperations.cpp

@@ -124,6 +124,14 @@ void didReplaceBody(ReasonsForOperationsThatWent& reasons)
     reasons.didReplaceBody();
 }
 
+// The error goes on as it would without the handler.
+static bool onlyThrowsAgain(const Function& function, BlockID handler)
+{
+    const Block& block = function.at(handler);
+    return !block.handler && block.values.size() == 1 && function.at(block.values.first()).opcode == Opcode::CatchException
+        && block.terminator.kind == TerminatorKind::Rethrow && block.terminator.exceptionOperand() == block.values.first();
+}
+
 Vector<OperationThatWent> findOperationsThatWentWithoutReason(const EffectContext& context, const Function& before, const Function& after, ReasonsForOperationsThatWent& reasons)
 {
     Vector<OperationThatWent> result;
@@ -213,7 +221,7 @@ Vector<OperationThatWent> findOperationsThatWentWithoutReason(const EffectContex
             const Value& value = before.at(id);
             ValueID now = findCurrent(id);
             bool isListed = !!now;
-            if (isListed && block.handler && !hasHandlerAfter.quickGet(now.index()) && effectsOf(context, after, after.at(now)).normal.mayThrow && !reasons.hasReasonFor(id))
+            if (isListed && block.handler && !hasHandlerAfter.quickGet(now.index()) && effectsOf(context, after, after.at(now)).normal.mayThrow && !reasons.hasReasonFor(id) && !onlyThrowsAgain(before, block.handler->block))
                 result.append({ id, blockID, value.opcode, OperationThatWent::How::LostItsHandler, OperationThatWent::Under::TypesAsTheyStand });
             // The pass that rewrote an operation into another one is responsible for it: it may know operand values that the types do not show.
             // The replacement may have no effect and be removed by a later run, which does not report it, so tell the outer listener now.