From 06d41edf2a73f27b784d44067c7f1f27be3ff913 Mon Sep 17 00:00:00 2001 From: Brian Hackett Date: Sat, 1 Dec 2012 16:32:26 -0700 Subject: [PATCH 01/12] Factor computation of live ranges out of LSRA, bug 817213. r=jandem --- js/src/Makefile.in | 1 + js/src/ion/C1Spewer.cpp | 4 +- js/src/ion/JSONSpewer.cpp | 2 +- js/src/ion/LinearScan.cpp | 935 ++---------------------------- js/src/ion/LinearScan.h | 381 +----------- js/src/ion/LiveRangeAllocator.cpp | 742 ++++++++++++++++++++++++ js/src/ion/LiveRangeAllocator.h | 519 +++++++++++++++++ 7 files changed, 1331 insertions(+), 1253 deletions(-) create mode 100644 js/src/ion/LiveRangeAllocator.cpp create mode 100644 js/src/ion/LiveRangeAllocator.h diff --git a/js/src/Makefile.in b/js/src/Makefile.in index d42d34796c28..38862fa82806 100644 --- a/js/src/Makefile.in +++ b/js/src/Makefile.in @@ -292,6 +292,7 @@ CPPSRCS += MIR.cpp \ LICM.cpp \ LinearScan.cpp \ LIR.cpp \ + LiveRangeAllocator.cpp \ Lowering.cpp \ Lowering-shared.cpp \ MCallOptimize.cpp \ diff --git a/js/src/ion/C1Spewer.cpp b/js/src/ion/C1Spewer.cpp index a19ed28e6039..6a40c82b390e 100644 --- a/js/src/ion/C1Spewer.cpp +++ b/js/src/ion/C1Spewer.cpp @@ -117,9 +117,9 @@ C1Spewer::spewIntervals(FILE *fp, LinearScanAllocator *regalloc, LInstruction *i for (size_t i = 0; i < vreg->numIntervals(); i++) { LiveInterval *live = vreg->getInterval(i); if (live->numRanges()) { - fprintf(fp, "%d object \"", (i == 0) ? vreg->reg() : int32(nextId++)); + fprintf(fp, "%d object \"", (i == 0) ? vreg->id() : int32(nextId++)); LAllocation::PrintAllocation(fp, live->getAllocation()); - fprintf(fp, "\" %d -1", vreg->reg()); + fprintf(fp, "\" %d -1", vreg->id()); for (size_t j = 0; j < live->numRanges(); j++) { fprintf(fp, " [%d, %d[", live->getRange(j)->from.pos(), live->getRange(j)->to.pos()); diff --git a/js/src/ion/JSONSpewer.cpp b/js/src/ion/JSONSpewer.cpp index eddde32a7b1e..fe202f9208eb 100644 --- a/js/src/ion/JSONSpewer.cpp +++ b/js/src/ion/JSONSpewer.cpp @@ -396,7 +396,7 @@ JSONSpewer::spewIntervals(LinearScanAllocator *regalloc) VirtualRegister *vreg = ®alloc->vregs[ins->getDef(k)->virtualRegister()]; beginObject(); - integerProperty("vreg", vreg->reg()); + integerProperty("vreg", vreg->id()); beginListProperty("intervals"); for (size_t i = 0; i < vreg->numIntervals(); i++) { diff --git a/js/src/ion/LinearScan.cpp b/js/src/ion/LinearScan.cpp index f2e42535b63c..6c00e6557960 100644 --- a/js/src/ion/LinearScan.cpp +++ b/js/src/ion/LinearScan.cpp @@ -17,782 +17,6 @@ using namespace js::ion; using mozilla::DebugOnly; -static bool -UseCompatibleWith(const LUse *use, LAllocation alloc) -{ - switch (use->policy()) { - case LUse::ANY: - case LUse::KEEPALIVE: - return alloc.isRegister() || alloc.isMemory(); - case LUse::REGISTER: - return alloc.isRegister(); - case LUse::FIXED: - // Fixed uses are handled using fixed intervals. The - // UsePosition is only used as hint. - return alloc.isRegister(); - default: - JS_NOT_REACHED("Unknown use policy"); - } - return false; -} - -#ifdef DEBUG -static bool -DefinitionCompatibleWith(LInstruction *ins, const LDefinition *def, LAllocation alloc) -{ - if (ins->isPhi()) { - if (def->type() == LDefinition::DOUBLE) - return alloc.isFloatReg() || alloc.kind() == LAllocation::DOUBLE_SLOT; - return alloc.isGeneralReg() || alloc.kind() == LAllocation::STACK_SLOT; - } - - switch (def->policy()) { - case LDefinition::DEFAULT: - if (!alloc.isRegister()) - return false; - return alloc.isFloatReg() == (def->type() == LDefinition::DOUBLE); - case LDefinition::PRESET: - return alloc == *def->output(); - case LDefinition::MUST_REUSE_INPUT: - if (!alloc.isRegister() || !ins->numOperands()) - return false; - return alloc == *ins->getOperand(def->getReusedInput()); - case LDefinition::PASSTHROUGH: - return true; - default: - JS_NOT_REACHED("Unknown definition policy"); - } - return false; -} -#endif - -int -Requirement::priority() const -{ - switch (kind_) { - case Requirement::FIXED: - return 0; - - case Requirement::REGISTER: - return 1; - - case Requirement::NONE: - return 2; - - default: - JS_NOT_REACHED("Unknown requirement kind."); - return -1; - } -} - -bool -LiveInterval::addRangeAtHead(CodePosition from, CodePosition to) -{ - JS_ASSERT(from < to); - - Range newRange(from, to); - - if (ranges_.empty()) - return ranges_.append(newRange); - - Range &first = ranges_.back(); - if (to < first.from) - return ranges_.append(newRange); - - if (to == first.from) { - first.from = from; - return true; - } - - JS_ASSERT(from < first.to); - JS_ASSERT(to > first.from); - if (from < first.from) - first.from = from; - if (to > first.to) - first.to = to; - - return true; -} - -bool -LiveInterval::addRange(CodePosition from, CodePosition to) -{ - JS_ASSERT(from < to); - - Range newRange(from, to); - - Range *i; - // Find the location to insert the new range - for (i = ranges_.end() - 1; i >= ranges_.begin(); i--) { - if (newRange.from <= i->to) { - if (i->from < newRange.from) - newRange.from = i->from; - break; - } - } - // Perform coalescing on overlapping ranges - for (; i >= ranges_.begin(); i--) { - if (newRange.to < i->from) - break; - if (newRange.to < i->to) - newRange.to = i->to; - ranges_.erase(i); - } - - return ranges_.insert(i + 1, newRange); -} - -void -LiveInterval::setFrom(CodePosition from) -{ - while (!ranges_.empty()) { - if (ranges_.back().to < from) { - ranges_.erase(&ranges_.back()); - } else { - if (from == ranges_.back().to) - ranges_.erase(&ranges_.back()); - else - ranges_.back().from = from; - break; - } - } -} - -bool -LiveInterval::covers(CodePosition pos) -{ - if (pos < start() || pos >= end()) - return false; - - // Loop over the ranges in ascending order. - size_t i = lastProcessedRangeIfValid(pos); - for (; i < ranges_.length(); i--) { - if (pos < ranges_[i].from) - return false; - setLastProcessedRange(i, pos); - if (pos < ranges_[i].to) - return true; - } - return false; -} - -CodePosition -LiveInterval::nextCoveredAfter(CodePosition pos) -{ - for (size_t i = 0; i < ranges_.length(); i++) { - if (ranges_[i].to <= pos) { - if (i) - return ranges_[i-1].from; - break; - } - if (ranges_[i].from <= pos) - return pos; - } - return CodePosition::MIN; -} - -CodePosition -LiveInterval::intersect(LiveInterval *other) -{ - if (start() > other->start()) - return other->intersect(this); - - // Loop over the ranges in ascending order. As an optimization, - // try to start at the last processed range. - size_t i = lastProcessedRangeIfValid(other->start()); - size_t j = other->ranges_.length() - 1; - if (i >= ranges_.length() || j >= other->ranges_.length()) - return CodePosition::MIN; - - while (true) { - const Range &r1 = ranges_[i]; - const Range &r2 = other->ranges_[j]; - - if (r1.from <= r2.from) { - if (r1.from <= other->start()) - setLastProcessedRange(i, other->start()); - if (r2.from < r1.to) - return r2.from; - if (i == 0 || ranges_[i-1].from > other->end()) - break; - i--; - } else { - if (r1.from < r2.to) - return r1.from; - if (j == 0 || other->ranges_[j-1].from > end()) - break; - j--; - } - } - - return CodePosition::MIN; -} - -/* - * This function takes the callee interval and moves all ranges following or - * including provided position to the target interval. Additionally, if a - * range in the callee interval spans the given position, it is split and the - * latter half is placed in the target interval. - * - * This function should only be called if it is known that the interval should - * actually be split (and, presumably, a move inserted). As such, it is an - * error for the caller to request a split that moves all intervals into the - * target. Doing so will trip the assertion at the bottom of the function. - */ -bool -LiveInterval::splitFrom(CodePosition pos, LiveInterval *after) -{ - JS_ASSERT(pos >= start() && pos < end()); - JS_ASSERT(after->ranges_.empty()); - - // Move all intervals over to the target - size_t bufferLength = ranges_.length(); - Range *buffer = ranges_.extractRawBuffer(); - if (!buffer) - return false; - after->ranges_.replaceRawBuffer(buffer, bufferLength); - - // Move intervals back as required - for (Range *i = &after->ranges_.back(); i >= after->ranges_.begin(); i--) { - if (pos >= i->to) - continue; - - if (pos > i->from) { - // Split the range - Range split(i->from, pos); - i->from = pos; - if (!ranges_.append(split)) - return false; - } - if (!ranges_.append(i + 1, after->ranges_.end())) - return false; - after->ranges_.shrinkBy(after->ranges_.end() - i - 1); - break; - } - - // Split the linked list of use positions - UsePosition *prev = NULL; - for (UsePositionIterator usePos(usesBegin()); usePos != usesEnd(); usePos++) { - if (usePos->pos > pos) - break; - prev = *usePos; - } - - uses_.splitAfter(prev, &after->uses_); - return true; -} - -LiveInterval * -VirtualRegister::intervalFor(CodePosition pos) -{ - for (LiveInterval **i = intervals_.begin(); i != intervals_.end(); i++) { - if ((*i)->covers(pos)) - return *i; - if (pos < (*i)->end()) - break; - } - return NULL; -} - -LiveInterval * -VirtualRegister::getFirstInterval() -{ - JS_ASSERT(!intervals_.empty()); - return intervals_[0]; -} - -void -LiveInterval::addUse(UsePosition *use) -{ - // Insert use positions in ascending order. Note that instructions - // are visited in reverse order, so in most cases the loop terminates - // at the first iteration and the use position will be added to the - // front of the list. - UsePosition *prev = NULL; - for (UsePositionIterator current(usesBegin()); current != usesEnd(); current++) { - if (current->pos >= use->pos) - break; - prev = *current; - } - - if (prev) - uses_.insertAfter(prev, use); - else - uses_.pushFront(use); -} - -UsePosition * -LiveInterval::nextUseAfter(CodePosition after) -{ - for (UsePositionIterator usePos(usesBegin()); usePos != usesEnd(); usePos++) { - if (usePos->pos >= after) { - LUse::Policy policy = usePos->use->policy(); - JS_ASSERT(policy != LUse::RECOVERED_INPUT); - if (policy != LUse::KEEPALIVE) - return *usePos; - } - } - return NULL; -} - -/* - * This function locates the first "real" use of this interval that follows - * the given code position. Non-"real" uses are currently just snapshots, - * which keep virtual registers alive but do not result in the - * generation of code that use them. - */ -CodePosition -LiveInterval::nextUsePosAfter(CodePosition after) -{ - UsePosition *min = nextUseAfter(after); - return min ? min->pos : CodePosition::MAX; -} - -/* - * This function finds the position of the first use of this interval - * that is incompatible with the provideded allocation. For example, - * a use with a REGISTER policy would be incompatible with a stack slot - * allocation. - */ -CodePosition -LiveInterval::firstIncompatibleUse(LAllocation alloc) -{ - for (UsePositionIterator usePos(usesBegin()); usePos != usesEnd(); usePos++) { - if (!UseCompatibleWith(usePos->use, alloc)) - return usePos->pos; - } - return CodePosition::MAX; -} - -/* - * This function pre-allocates and initializes as much global state as possible - * to avoid littering the algorithms with memory management cruft. - */ -bool -LinearScanAllocator::createDataStructures() -{ - if (!RegisterAllocator::init()) - return false; - - liveIn = lir->mir()->allocate(graph.numBlockIds()); - if (!liveIn) - return false; - - // Initialize fixed intervals. - for (size_t i = 0; i < AnyRegister::Total; i++) { - AnyRegister reg = AnyRegister::FromCode(i); - LiveInterval *interval = new LiveInterval(NULL, 0); - interval->setAllocation(LAllocation(reg)); - fixedIntervals[i] = interval; - } - - fixedIntervalsUnion = new LiveInterval(NULL, 0); - - if (!vregs.init(lir->mir(), graph.numVirtualRegisters())) - return false; - - // Build virtual register objects - for (size_t i = 0; i < graph.numBlocks(); i++) { - if (mir->shouldCancel("LSRA create data structures (main loop)")) - return false; - - LBlock *block = graph.getBlock(i); - for (LInstructionIterator ins = block->begin(); ins != block->end(); ins++) { - for (size_t j = 0; j < ins->numDefs(); j++) { - LDefinition *def = ins->getDef(j); - if (def->policy() != LDefinition::PASSTHROUGH) { - uint32 reg = def->virtualRegister(); - if (!vregs[reg].init(reg, block, *ins, def, /* isTemp */ false)) - return false; - } - } - - for (size_t j = 0; j < ins->numTemps(); j++) { - LDefinition *def = ins->getTemp(j); - if (def->isBogusTemp()) - continue; - if (!vregs[def].init(def->virtualRegister(), block, *ins, def, /* isTemp */ true)) - return false; - } - } - for (size_t j = 0; j < block->numPhis(); j++) { - LPhi *phi = block->getPhi(j); - LDefinition *def = phi->getDef(0); - if (!vregs[def].init(phi->id(), block, phi, def, /* isTemp */ false)) - return false; - } - } - - return true; -} - -static inline AnyRegister -GetFixedRegister(LDefinition *def, LUse *use) -{ - return def->type() == LDefinition::DOUBLE - ? AnyRegister(FloatRegister::FromCode(use->registerCode())) - : AnyRegister(Register::FromCode(use->registerCode())); -} - -static inline bool -IsNunbox(VirtualRegister *vreg) -{ -#ifdef JS_NUNBOX32 - return (vreg->type() == LDefinition::TYPE || - vreg->type() == LDefinition::PAYLOAD); -#else - return false; -#endif -} - -static inline bool -IsTraceable(VirtualRegister *reg) -{ - if (reg->type() == LDefinition::OBJECT) - return true; -#ifdef JS_PUNBOX64 - if (reg->type() == LDefinition::BOX) - return true; -#endif - return false; -} - -#ifdef DEBUG -static inline bool -NextInstructionHasFixedUses(LBlock *block, LInstruction *ins) -{ - LInstructionIterator iter(block->begin(ins)); - iter++; - for (LInstruction::InputIterator alloc(**iter); alloc.more(); alloc.next()) { - if (alloc->isUse() && alloc->toUse()->isFixedRegister()) - return true; - } - return false; -} - -// Returns true iff ins has a def/temp reusing the input allocation. -static bool -IsInputReused(LInstruction *ins, LUse *use) -{ - for (size_t i = 0; i < ins->numDefs(); i++) { - if (ins->getDef(i)->policy() == LDefinition::MUST_REUSE_INPUT && - ins->getOperand(ins->getDef(i)->getReusedInput())->toUse() == use) - { - return true; - } - } - - for (size_t i = 0; i < ins->numTemps(); i++) { - if (ins->getTemp(i)->policy() == LDefinition::MUST_REUSE_INPUT && - ins->getOperand(ins->getTemp(i)->getReusedInput())->toUse() == use) - { - return true; - } - } - - return false; -} -#endif - -/* - * This function builds up liveness intervals for all virtual registers - * defined in the function. Additionally, it populates the liveIn array with - * information about which registers are live at the beginning of a block, to - * aid resolution and reification in a later phase. - * - * The algorithm is based on the one published in: - * - * Wimmer, Christian, and Michael Franz. "Linear Scan Register Allocation on - * SSA Form." Proceedings of the International Symposium on Code Generation - * and Optimization. Toronto, Ontario, Canada, ACM. 2010. 170-79. PDF. - * - * The algorithm operates on blocks ordered such that dominators of a block - * are before the block itself, and such that all blocks of a loop are - * contiguous. It proceeds backwards over the instructions in this order, - * marking registers live at their uses, ending their live intervals at - * definitions, and recording which registers are live at the top of every - * block. To deal with loop backedges, variables live at the beginning of - * a loop gain an interval covering the entire loop. - */ -bool -LinearScanAllocator::buildLivenessInfo() -{ - Vector loopWorkList; - BitSet *loopDone = BitSet::New(graph.numBlockIds()); - if (!loopDone) - return false; - - for (size_t i = graph.numBlocks(); i > 0; i--) { - if (mir->shouldCancel("LSRA Build Liveness Info (main loop)")) - return false; - - LBlock *block = graph.getBlock(i - 1); - MBasicBlock *mblock = block->mir(); - - BitSet *live = BitSet::New(graph.numVirtualRegisters()); - if (!live) - return false; - liveIn[mblock->id()] = live; - - // Propagate liveIn from our successors to us - for (size_t i = 0; i < mblock->lastIns()->numSuccessors(); i++) { - MBasicBlock *successor = mblock->lastIns()->getSuccessor(i); - // Skip backedges, as we fix them up at the loop header. - if (mblock->id() < successor->id()) - live->insertAll(liveIn[successor->id()]); - } - - // Add successor phis - if (mblock->successorWithPhis()) { - LBlock *phiSuccessor = mblock->successorWithPhis()->lir(); - for (unsigned int j = 0; j < phiSuccessor->numPhis(); j++) { - LPhi *phi = phiSuccessor->getPhi(j); - LAllocation *use = phi->getOperand(mblock->positionInPhiSuccessor()); - uint32 reg = use->toUse()->virtualRegister(); - live->insert(reg); - } - } - - // Variables are assumed alive for the entire block, a define shortens - // the interval to the point of definition. - for (BitSet::Iterator liveRegId(*live); liveRegId; liveRegId++) { - if (!vregs[*liveRegId].getInterval(0)->addRangeAtHead(inputOf(block->firstId()), - outputOf(block->lastId()).next())) - { - return false; - } - } - - // Shorten the front end of live intervals for live variables to their - // point of definition, if found. - for (LInstructionReverseIterator ins = block->rbegin(); ins != block->rend(); ins++) { - // Calls may clobber registers, so force a spill and reload around the callsite. - if (ins->isCall()) { - for (AnyRegisterIterator iter(allRegisters_); iter.more(); iter++) { - if (!addFixedRangeAtHead(*iter, inputOf(*ins), outputOf(*ins))) - return false; - } - } - - for (size_t i = 0; i < ins->numDefs(); i++) { - if (ins->getDef(i)->policy() != LDefinition::PASSTHROUGH) { - LDefinition *def = ins->getDef(i); - - CodePosition from; - if (def->policy() == LDefinition::PRESET && def->output()->isRegister()) { - // The fixed range covers the current instruction so the - // interval for the virtual register starts at the next - // instruction. If the next instruction has a fixed use, - // this can lead to unnecessary register moves. To avoid - // special handling for this, assert the next instruction - // has no fixed uses. defineFixed guarantees this by inserting - // an LNop. - JS_ASSERT(!NextInstructionHasFixedUses(block, *ins)); - AnyRegister reg = def->output()->toRegister(); - if (!addFixedRangeAtHead(reg, inputOf(*ins), outputOf(*ins).next())) - return false; - from = outputOf(*ins).next(); - } else { - from = inputOf(*ins); - } - - if (def->policy() == LDefinition::MUST_REUSE_INPUT) { - // MUST_REUSE_INPUT is implemented by allocating an output - // register and moving the input to it. Register hints are - // used to avoid unnecessary moves. We give the input an - // LUse::ANY policy to avoid allocating a register for the - // input. - LUse *inputUse = ins->getOperand(def->getReusedInput())->toUse(); - JS_ASSERT(inputUse->policy() == LUse::REGISTER); - JS_ASSERT(inputUse->usedAtStart()); - *inputUse = LUse(inputUse->virtualRegister(), LUse::ANY, /* usedAtStart = */ true); - } - - LiveInterval *interval = vregs[def].getInterval(0); - interval->setFrom(from); - - // Ensure that if there aren't any uses, there's at least - // some interval for the output to go into. - if (interval->numRanges() == 0) { - if (!interval->addRangeAtHead(from, from.next())) - return false; - } - live->remove(def->virtualRegister()); - } - } - - for (size_t i = 0; i < ins->numTemps(); i++) { - LDefinition *temp = ins->getTemp(i); - if (temp->isBogusTemp()) - continue; - if (ins->isCall()) { - JS_ASSERT(temp->isPreset()); - continue; - } - - if (temp->policy() == LDefinition::PRESET) { - AnyRegister reg = temp->output()->toRegister(); - if (!addFixedRangeAtHead(reg, inputOf(*ins), outputOf(*ins))) - return false; - } else { - if (!vregs[temp].getInterval(0)->addRangeAtHead(inputOf(*ins), outputOf(*ins))) - return false; - } - } - - DebugOnly hasUseRegister = false; - DebugOnly hasUseRegisterAtStart = false; - - for (LInstruction::InputIterator alloc(**ins); alloc.more(); alloc.next()) { - if (alloc->isUse()) { - LUse *use = alloc->toUse(); - - // The first instruction, LLabel, has no uses. - JS_ASSERT(inputOf(*ins) > outputOf(block->firstId())); - - // Call uses should always be at-start or fixed, since the fixed intervals - // use all registers. - JS_ASSERT_IF(ins->isCall() && !alloc.isSnapshotInput(), - use->isFixedRegister() || use->usedAtStart()); - -#ifdef DEBUG - // Don't allow at-start call uses if there are temps of the same kind, - // so that we don't assign the same register. - if (ins->isCall() && use->usedAtStart()) { - for (size_t i = 0; i < ins->numTemps(); i++) - JS_ASSERT(vregs[ins->getTemp(i)].isDouble() != vregs[use].isDouble()); - } - - // If there are both useRegisterAtStart(x) and useRegister(y) - // uses, we may assign the same register to both operands due to - // interval splitting (bug 772830). Don't allow this for now. - if (use->policy() == LUse::REGISTER) { - if (use->usedAtStart()) { - if (!IsInputReused(*ins, use)) - hasUseRegisterAtStart = true; - } else { - hasUseRegister = true; - } - } - - JS_ASSERT(!(hasUseRegister && hasUseRegisterAtStart)); -#endif - - // Don't treat RECOVERED_INPUT uses as keeping the vreg alive. - if (use->policy() == LUse::RECOVERED_INPUT) - continue; - - CodePosition to; - if (use->isFixedRegister()) { - JS_ASSERT(!use->usedAtStart()); - AnyRegister reg = GetFixedRegister(vregs[use].def(), use); - if (!addFixedRangeAtHead(reg, inputOf(*ins), outputOf(*ins))) - return false; - to = inputOf(*ins); - } else { - to = use->usedAtStart() ? inputOf(*ins) : outputOf(*ins); - } - - LiveInterval *interval = vregs[use].getInterval(0); - if (!interval->addRangeAtHead(inputOf(block->firstId()), to)) - return false; - interval->addUse(new UsePosition(use, to)); - - live->insert(use->virtualRegister()); - } - } - } - - // Phis have simultaneous assignment semantics at block begin, so at - // the beginning of the block we can be sure that liveIn does not - // contain any phi outputs. - for (unsigned int i = 0; i < block->numPhis(); i++) { - LDefinition *def = block->getPhi(i)->getDef(0); - if (live->contains(def->virtualRegister())) { - live->remove(def->virtualRegister()); - } else { - // This is a dead phi, so add a dummy range over all phis. This - // can go away if we have an earlier dead code elimination pass. - if (!vregs[def].getInterval(0)->addRangeAtHead(inputOf(block->firstId()), - outputOf(block->firstId()))) - { - return false; - } - } - } - - if (mblock->isLoopHeader()) { - // A divergence from the published algorithm is required here, as - // our block order does not guarantee that blocks of a loop are - // contiguous. As a result, a single live interval spanning the - // loop is not possible. Additionally, we require liveIn in a later - // pass for resolution, so that must also be fixed up here. - MBasicBlock *loopBlock = mblock->backedge(); - while (true) { - // Blocks must already have been visited to have a liveIn set. - JS_ASSERT(loopBlock->id() >= mblock->id()); - - // Add an interval for this entire loop block - CodePosition from = inputOf(loopBlock->lir()->firstId()); - CodePosition to = outputOf(loopBlock->lir()->lastId()).next(); - - for (BitSet::Iterator liveRegId(*live); liveRegId; liveRegId++) { - if (!vregs[*liveRegId].getInterval(0)->addRange(from, to)) - return false; - } - - // Fix up the liveIn set to account for the new interval - liveIn[loopBlock->id()]->insertAll(live); - - // Make sure we don't visit this node again - loopDone->insert(loopBlock->id()); - - // If this is the loop header, any predecessors are either the - // backedge or out of the loop, so skip any predecessors of - // this block - if (loopBlock != mblock) { - for (size_t i = 0; i < loopBlock->numPredecessors(); i++) { - MBasicBlock *pred = loopBlock->getPredecessor(i); - if (loopDone->contains(pred->id())) - continue; - if (!loopWorkList.append(pred)) - return false; - } - } - - // Terminate loop if out of work. - if (loopWorkList.empty()) - break; - - // Grab the next block off the work list, skipping any OSR block. - do { - loopBlock = loopWorkList.popCopy(); - } while (loopBlock->lir() == graph.osrBlock()); - } - - // Clear the done set for other loops - loopDone->clear(); - } - - JS_ASSERT_IF(!mblock->numPredecessors(), live->empty()); - } - - validateVirtualRegisters(); - - // If the script has an infinite loop, there may be no MReturn and therefore - // no fixed intervals. Add a small range to fixedIntervalsUnion so that the - // rest of the allocator can assume it has at least one range. - if (fixedIntervalsUnion->numRanges() == 0) { - if (!fixedIntervalsUnion->addRangeAtHead(CodePosition(0, CodePosition::INPUT), - CodePosition(0, CodePosition::OUTPUT))) - { - return false; - } - } - - return true; -} - /* * Merge virtual register intervals into the UnhandledQueue, taking advantage * of their nearly-sorted ordering. @@ -872,7 +96,7 @@ LinearScanAllocator::allocateRegisters() Requirement *hint = current->hint(); IonSpew(IonSpew_RegAlloc, "Processing %d = [%u, %u] (pri=%d)", - current->reg() ? current->reg()->reg() : 0, current->start().pos(), + current->hasVreg() ? current->vreg() : 0, current->start().pos(), current->end().pos(), current->requirement()->priority()); // Shift active intervals to the inactive or handled sets as appropriate @@ -926,7 +150,7 @@ LinearScanAllocator::allocateRegisters() // If we don't really need this in a register, don't allocate one if (req->kind() != Requirement::REGISTER && hint->kind() == Requirement::NONE) { IonSpew(IonSpew_RegAlloc, " Eagerly spilling virtual register %d", - current->reg() ? current->reg()->reg() : 0); + current->hasVreg() ? current->vreg() : 0); if (!spill()) return false; continue; @@ -1010,7 +234,7 @@ LinearScanAllocator::resolveControlFlow() for (size_t j = 0; j < successor->numPhis(); j++) { LPhi *phi = successor->getPhi(j); JS_ASSERT(phi->numDefs() == 1); - VirtualRegister *vreg = &vregs[phi->getDef(0)]; + LinearScanVirtualRegister *vreg = &vregs[phi->getDef(0)]; LiveInterval *to = vreg->intervalFor(inputOf(successor->firstId())); JS_ASSERT(to); @@ -1030,7 +254,7 @@ LinearScanAllocator::resolveControlFlow() if (vreg->mustSpillAtDefinition() && !to->isSpill()) { // Make sure this phi is spilled at the loop header. LMoveGroup *moves = successor->getEntryMoveGroup(); - if (!moves->add(to->getAllocation(), to->reg()->canonicalSpill())) + if (!moves->add(to->getAllocation(), vregs[to->vreg()].canonicalSpill())) return false; } } @@ -1083,13 +307,13 @@ LinearScanAllocator::reifyAllocations() { // Iterate over each interval, ensuring that definitions are visited before uses. for (size_t j = 1; j < graph.numVirtualRegisters(); j++) { - VirtualRegister *reg = &vregs[j]; + LinearScanVirtualRegister *reg = &vregs[j]; if (mir->shouldCancel("LSRA Reification (main loop)")) return false; for (size_t k = 0; k < reg->numIntervals(); k++) { LiveInterval *interval = reg->getInterval(k); - JS_ASSERT(reg == interval->reg()); + JS_ASSERT(reg == &vregs[interval->vreg()]); if (!interval->numRanges()) continue; @@ -1253,10 +477,10 @@ LinearScanAllocator::findFirstNonCallSafepoint(CodePosition from) return i; } -static inline bool -IsSpilledAt(LiveInterval *interval, CodePosition pos) +inline bool +LinearScanAllocator::isSpilledAt(LiveInterval *interval, CodePosition pos) { - VirtualRegister *reg = interval->reg(); + LinearScanVirtualRegister *reg = &vregs[interval->vreg()]; if (!reg->canonicalSpill() || !reg->canonicalSpill()->isStackSlot()) return false; @@ -1274,7 +498,7 @@ LinearScanAllocator::populateSafepoints() size_t firstSafepoint = 0; for (uint32 i = 0; i < vregs.numVirtualRegisters(); i++) { - VirtualRegister *reg = &vregs[i]; + LinearScanVirtualRegister *reg = &vregs[i]; if (!reg->def() || (!IsTraceable(reg) && !IsNunbox(reg))) continue; @@ -1326,7 +550,7 @@ LinearScanAllocator::populateSafepoints() } } - if (IsSpilledAt(interval, inputOf(ins))) { + if (isSpilledAt(interval, inputOf(ins))) { #ifdef JS_PUNBOX64 if (reg->type() == LDefinition::BOX) { if (!safepoint->addValueSlot(reg->canonicalSpillSlot())) @@ -1340,9 +564,9 @@ LinearScanAllocator::populateSafepoints() } #ifdef JS_NUNBOX32 } else { - VirtualRegister *other = otherHalfOfNunbox(reg); - VirtualRegister *type = (reg->type() == LDefinition::TYPE) ? reg : other; - VirtualRegister *payload = (reg->type() == LDefinition::PAYLOAD) ? reg : other; + LinearScanVirtualRegister *other = otherHalfOfNunbox(reg); + LinearScanVirtualRegister *type = (reg->type() == LDefinition::TYPE) ? reg : other; + LinearScanVirtualRegister *payload = (reg->type() == LDefinition::PAYLOAD) ? reg : other; LiveInterval *typeInterval = type->intervalFor(inputOf(ins)); LiveInterval *payloadInterval = payload->intervalFor(inputOf(ins)); @@ -1358,8 +582,8 @@ LinearScanAllocator::populateSafepoints() if (payloadAlloc->isArgument()) continue; - if (IsSpilledAt(typeInterval, inputOf(ins)) && - IsSpilledAt(payloadInterval, inputOf(ins))) + if (isSpilledAt(typeInterval, inputOf(ins)) && + isSpilledAt(payloadInterval, inputOf(ins))) { // These two components of the Value are spilled // contiguously, so simply keep track of the base slot. @@ -1370,7 +594,7 @@ LinearScanAllocator::populateSafepoints() } if (!ins->isCall() && - (!IsSpilledAt(typeInterval, inputOf(ins)) || payloadAlloc->isGeneralReg())) + (!isSpilledAt(typeInterval, inputOf(ins)) || payloadAlloc->isGeneralReg())) { // Either the payload is on the stack but the type is // in a register, or the payload is in a register. In @@ -1407,13 +631,13 @@ LinearScanAllocator::splitInterval(LiveInterval *interval, CodePosition pos) // interval in the same virtual register. JS_ASSERT(interval->start() < pos && pos < interval->end()); - VirtualRegister *reg = interval->reg(); + LinearScanVirtualRegister *reg = &vregs[interval->vreg()]; // "Bogus" intervals cannot be split. JS_ASSERT(reg); // Do the split. - LiveInterval *newInterval = new LiveInterval(reg, interval->index() + 1); + LiveInterval *newInterval = new LiveInterval(interval->vreg(), interval->index() + 1); if (!interval->splitFrom(pos, newInterval)) return false; @@ -1424,7 +648,7 @@ LinearScanAllocator::splitInterval(LiveInterval *interval, CodePosition pos) return false; IonSpew(IonSpew_RegAlloc, " Split interval to %u = [%u, %u]/[%u, %u]", - interval->reg()->reg(), interval->start().pos(), + interval->vreg(), interval->start().pos(), interval->end().pos(), newInterval->start().pos(), newInterval->end().pos()); @@ -1461,7 +685,7 @@ LinearScanAllocator::splitBlockingIntervals(LAllocation allocation) for (IntervalIterator i(active.begin()); i != active.end(); i++) { if (i->getAllocation()->isRegister() && *i->getAllocation() == allocation) { IonSpew(IonSpew_RegAlloc, " Splitting active interval %u = [%u, %u]", - i->reg()->ins()->id(), i->start().pos(), i->end().pos()); + vregs[i->vreg()].ins()->id(), i->start().pos(), i->end().pos()); JS_ASSERT(i->start() != current->start()); JS_ASSERT(i->covers(current->start())); @@ -1481,7 +705,7 @@ LinearScanAllocator::splitBlockingIntervals(LAllocation allocation) for (IntervalIterator i(inactive.begin()); i != inactive.end(); ) { if (i->getAllocation()->isRegister() && *i->getAllocation() == allocation) { IonSpew(IonSpew_RegAlloc, " Splitting inactive interval %u = [%u, %u]", - i->reg()->ins()->id(), i->start().pos(), i->end().pos()); + vregs[i->vreg()].ins()->id(), i->start().pos(), i->end().pos()); LiveInterval *it = *i; CodePosition nextActive = it->nextCoveredAfter(current->start()); @@ -1512,7 +736,7 @@ LinearScanAllocator::assign(LAllocation allocation) current->setAllocation(allocation); // Split this interval at the next incompatible one - VirtualRegister *reg = current->reg(); + LinearScanVirtualRegister *reg = &vregs[current->vreg()]; if (reg) { CodePosition splitPos = current->firstIncompatibleUse(allocation); if (splitPos != CodePosition::MAX) { @@ -1528,7 +752,7 @@ LinearScanAllocator::assign(LAllocation allocation) if (reg && allocation.isMemory()) { if (reg->canonicalSpill()) { - JS_ASSERT(allocation == *current->reg()->canonicalSpill()); + JS_ASSERT(allocation == *reg->canonicalSpill()); // This interval is spilled more than once, so just always spill // it at its definition. @@ -1552,11 +776,11 @@ LinearScanAllocator::assign(LAllocation allocation) } #ifdef JS_NUNBOX32 -VirtualRegister * +LinearScanVirtualRegister * LinearScanAllocator::otherHalfOfNunbox(VirtualRegister *vreg) { signed offset = OffsetToOtherHalfOfNunbox(vreg->type()); - VirtualRegister *other = &vregs[vreg->def()->virtualRegister() + offset]; + LinearScanVirtualRegister *other = &vregs[vreg->def()->virtualRegister() + offset]; AssertTypesFormANunbox(vreg->type(), other->type()); return other; } @@ -1566,15 +790,17 @@ LinearScanAllocator::otherHalfOfNunbox(VirtualRegister *vreg) uint32 LinearScanAllocator::allocateSlotFor(const LiveInterval *interval) { + LinearScanVirtualRegister *reg = &vregs[interval->vreg()]; + SlotList *freed; - if (interval->reg()->type() == LDefinition::DOUBLE || IsNunbox(interval->reg())) + if (reg->type() == LDefinition::DOUBLE || IsNunbox(reg)) freed = &finishedDoubleSlots_; else freed = &finishedSlots_; if (!freed->empty()) { LiveInterval *maybeDead = freed->back(); - if (maybeDead->end() < interval->reg()->getInterval(0)->start()) { + if (maybeDead->end() < reg->getInterval(0)->start()) { // This spill slot is dead before the start of the interval trying // to reuse the slot, so reuse is safe. Otherwise, we could // encounter a situation where a stack slot is allocated and freed @@ -1585,7 +811,7 @@ LinearScanAllocator::allocateSlotFor(const LiveInterval *interval) // before the current interval, to avoid conflicting slot -> reg and // reg -> slot moves in the same movegroup. freed->popBack(); - VirtualRegister *dead = maybeDead->reg(); + LinearScanVirtualRegister *dead = &vregs[maybeDead->vreg()]; #ifdef JS_NUNBOX32 if (IsNunbox(dead)) return BaseOfNunboxSlot(dead->type(), dead->canonicalSpillSlot()); @@ -1594,9 +820,9 @@ LinearScanAllocator::allocateSlotFor(const LiveInterval *interval) } } - if (IsNunbox(interval->reg())) + if (IsNunbox(reg)) return stackSlotAllocator.allocateValueSlot(); - if (interval->reg()->isDouble()) + if (reg->isDouble()) return stackSlotAllocator.allocateDoubleSlot(); return stackSlotAllocator.allocateSlot(); } @@ -1607,18 +833,20 @@ LinearScanAllocator::spill() IonSpew(IonSpew_RegAlloc, " Decided to spill current interval"); // We can't spill bogus intervals - JS_ASSERT(current->reg()); + JS_ASSERT(current->hasVreg()); - if (current->reg()->canonicalSpill()) { + LinearScanVirtualRegister *reg = &vregs[current->vreg()]; + + if (reg->canonicalSpill()) { IonSpew(IonSpew_RegAlloc, " Allocating canonical spill location"); - return assign(*current->reg()->canonicalSpill()); + return assign(*reg->canonicalSpill()); } uint32 stackSlot; #if defined JS_NUNBOX32 - if (IsNunbox(current->reg())) { - VirtualRegister *other = otherHalfOfNunbox(current->reg()); + if (IsNunbox(reg)) { + LinearScanVirtualRegister *other = otherHalfOfNunbox(reg); if (other->canonicalSpill()) { // The other half of this nunbox already has a spill slot. To @@ -1631,7 +859,7 @@ LinearScanAllocator::spill() // one. stackSlot = allocateSlotFor(current); } - stackSlot -= OffsetOfNunboxSlot(current->reg()->type()); + stackSlot -= OffsetOfNunboxSlot(reg->type()); } else #endif { @@ -1639,13 +867,13 @@ LinearScanAllocator::spill() } JS_ASSERT(stackSlot <= stackSlotAllocator.stackHeight()); - return assign(LStackSlot(stackSlot, current->reg()->isDouble())); + return assign(LStackSlot(stackSlot, reg->isDouble())); } void LinearScanAllocator::freeAllocation(LiveInterval *interval, LAllocation *alloc) { - VirtualRegister *mine = interval->reg(); + LinearScanVirtualRegister *mine = &vregs[interval->vreg()]; if (!IsNunbox(mine)) { if (alloc->isStackSlot()) { if (alloc->toStackSlot()->isDouble()) @@ -1659,7 +887,7 @@ LinearScanAllocator::freeAllocation(LiveInterval *interval, LAllocation *alloc) #ifdef JS_NUNBOX32 // Special handling for nunboxes. We can only free the stack slot once we // know both intervals have been finished. - VirtualRegister *other = otherHalfOfNunbox(mine); + LinearScanVirtualRegister *other = otherHalfOfNunbox(mine); if (other->finished()) { if (!mine->canonicalSpill() && !other->canonicalSpill()) return; @@ -1667,7 +895,7 @@ LinearScanAllocator::freeAllocation(LiveInterval *interval, LAllocation *alloc) JS_ASSERT_IF(mine->canonicalSpill() && other->canonicalSpill(), mine->canonicalSpill()->isStackSlot() == other->canonicalSpill()->isStackSlot()); - VirtualRegister *candidate = mine->canonicalSpill() ? mine : other; + LinearScanVirtualRegister *candidate = mine->canonicalSpill() ? mine : other; if (!candidate->canonicalSpill()->isStackSlot()) return; @@ -1683,16 +911,18 @@ LinearScanAllocator::finishInterval(LiveInterval *interval) JS_ASSERT(!alloc->isUse()); // Toss out the bogus interval now that it's run its course - if (!interval->reg()) + if (!interval->hasVreg()) return; - // All spills should be equal to the canonical spill location. - JS_ASSERT_IF(alloc->isStackSlot(), *alloc == *interval->reg()->canonicalSpill()); + LinearScanVirtualRegister *reg = &vregs[interval->vreg()]; - bool lastInterval = interval->index() == (interval->reg()->numIntervals() - 1); + // All spills should be equal to the canonical spill location. + JS_ASSERT_IF(alloc->isStackSlot(), *alloc == *reg->canonicalSpill()); + + bool lastInterval = interval->index() == (reg->numIntervals() - 1); if (lastInterval) { freeAllocation(interval, alloc); - interval->reg()->setFinished(); + reg->setFinished(); } handled.pushBack(interval); @@ -1710,7 +940,7 @@ LinearScanAllocator::findBestFreeRegister(CodePosition *freeUntil) // Compute free-until positions for all registers CodePosition freeUntilPos[AnyRegister::Total]; - bool needFloat = current->reg()->isDouble(); + bool needFloat = vregs[current->vreg()].isDouble(); for (AnyRegisterIterator regs(allRegisters_); regs.more(); regs++) { AnyRegister reg = *regs; if (reg.isFloat() == needFloat) @@ -1752,7 +982,7 @@ LinearScanAllocator::findBestFreeRegister(CodePosition *freeUntil) if (current->index()) { // As an optimization, use the allocation from the previous interval if // it is available. - LiveInterval *previous = current->reg()->getInterval(current->index() - 1); + LiveInterval *previous = vregs[current->vreg()].getInterval(current->index() - 1); if (previous->getAllocation()->isRegister()) { AnyRegister prevReg = previous->getAllocation()->toRegister(); if (freeUntilPos[prevReg.code()] != CodePosition::MIN) @@ -1803,7 +1033,7 @@ LinearScanAllocator::findBestBlockedRegister(CodePosition *nextUsed) // Compute next-used positions for all registers CodePosition nextUsePos[AnyRegister::Total]; - bool needFloat = current->reg()->isDouble(); + bool needFloat = vregs[current->vreg()].isDouble(); for (AnyRegisterIterator regs(allRegisters_); regs.more(); regs++) { AnyRegister reg = *regs; if (reg.isFloat() == needFloat) @@ -1962,9 +1192,10 @@ LinearScanAllocator::validateAllocations() JS_ASSERT(*i != *j); JS_ASSERT(canCoexist(*i, *j)); } + LinearScanVirtualRegister *reg = &vregs[i->vreg()]; bool found = false; - for (size_t j = 0; j < i->reg()->numIntervals(); j++) { - if (i->reg()->getInterval(j) == *i) { + for (size_t j = 0; j < reg->numIntervals(); j++) { + if (reg->getInterval(j) == *i) { JS_ASSERT(j == i->index()); found = true; break; @@ -1974,43 +1205,6 @@ LinearScanAllocator::validateAllocations() } } -void -LiveInterval::validateRanges() -{ - Range *prev = NULL; - - for (size_t i = ranges_.length() - 1; i < ranges_.length(); i--) { - Range *range = &ranges_[i]; - - JS_ASSERT(range->from < range->to); - JS_ASSERT_IF(prev, prev->to <= range->from); - prev = range; - } -} - -void -LinearScanAllocator::validateVirtualRegisters() -{ - for (size_t i = 1; i < graph.numVirtualRegisters(); i++) { - VirtualRegister *reg = &vregs[i]; - - LiveInterval *prev = NULL; - for (size_t j = 0; j < reg->numIntervals(); j++) { - LiveInterval *interval = reg->getInterval(j); - JS_ASSERT(reg == interval->reg()); - JS_ASSERT(interval->index() == j); - - if (interval->numRanges() == 0) - continue; - - JS_ASSERT_IF(prev, prev->end() <= interval->start()); - interval->validateRanges(); - - prev = interval; - } - } -} - #endif // DEBUG bool @@ -2018,14 +1212,6 @@ LinearScanAllocator::go() { IonSpew(IonSpew_RegAlloc, "Beginning register allocation"); - IonSpew(IonSpew_RegAlloc, "Beginning creation of initial data structures"); - if (!createDataStructures()) - return false; - IonSpew(IonSpew_RegAlloc, "Creation of initial data structures completed"); - - if (mir->shouldCancel("LSRA Create Data Structures")) - return false; - IonSpew(IonSpew_RegAlloc, "Beginning liveness analysis"); if (!buildLivenessInfo()) return false; @@ -2079,8 +1265,7 @@ LinearScanAllocator::setIntervalRequirement(LiveInterval *interval) // This function computes requirement by virtual register, other types of // interval should have requirements set manually - VirtualRegister *reg = interval->reg(); - JS_ASSERT(reg); + LinearScanVirtualRegister *reg = &vregs[interval->vreg()]; if (interval->index() == 0) { // The first interval is the definition, so deal with any definition @@ -2134,7 +1319,7 @@ LinearScanAllocator::setIntervalRequirement(LiveInterval *interval) // Search other uses for hints. If the virtual register already has a // canonical spill location, we will eagerly spill this interval, so we // don't have to search for hints. - if (!fixedOp && !interval->reg()->canonicalSpill()) { + if (!fixedOp && !vregs[interval->vreg()].canonicalSpill()) { for (; usePos != interval->usesEnd(); usePos++) { LUse::Policy policy = usePos->use->policy(); if (policy == LUse::FIXED) { diff --git a/js/src/ion/LinearScan.h b/js/src/ion/LinearScan.h index e26b88666ddc..c478d3e2ded7 100644 --- a/js/src/ion/LinearScan.h +++ b/js/src/ion/LinearScan.h @@ -8,7 +8,7 @@ #ifndef js_ion_linearscan_h__ #define js_ion_linearscan_h__ -#include "RegisterAllocator.h" +#include "LiveRangeAllocator.h" #include "BitSet.h" #include "StackSlotAllocator.h" @@ -17,245 +17,9 @@ namespace js { namespace ion { -class VirtualRegister; - -class Requirement +class LinearScanVirtualRegister : public VirtualRegister { - public: - enum Kind { - NONE, - REGISTER, - FIXED, - SAME_AS_OTHER - }; - - Requirement() - : kind_(NONE) - { } - - Requirement(Kind kind) - : kind_(kind) - { - // These have dedicated constructors; - JS_ASSERT(kind != FIXED && kind != SAME_AS_OTHER); - } - - Requirement(Kind kind, CodePosition at) - : kind_(kind), - position_(at) - { } - - Requirement(LAllocation fixed) - : kind_(FIXED), - allocation_(fixed) - { } - - // Only useful as a hint, encodes where the fixed requirement is used to - // avoid allocating a fixed register too early. - Requirement(LAllocation fixed, CodePosition at) - : kind_(FIXED), - allocation_(fixed), - position_(at) - { } - - Requirement(uint32 vreg, CodePosition at) - : kind_(SAME_AS_OTHER), - allocation_(LUse(vreg, LUse::ANY)), - position_(at) - { } - - Kind kind() const { - return kind_; - } - - LAllocation allocation() const { - JS_ASSERT(!allocation_.isUse()); - return allocation_; - } - - uint32 virtualRegister() const { - JS_ASSERT(allocation_.isUse()); - return allocation_.toUse()->virtualRegister(); - } - - CodePosition pos() const { - return position_; - } - - int priority() const; - private: - Kind kind_; - LAllocation allocation_; - CodePosition position_; -}; - -struct UsePosition : public TempObject, - public InlineForwardListNode -{ - LUse *use; - CodePosition pos; - - UsePosition(LUse *use, CodePosition pos) : - use(use), - pos(pos) - { } -}; - -typedef InlineForwardListIterator UsePositionIterator; - -/* - * A live interval is a set of disjoint ranges of code positions where a - * virtual register is live. Linear scan register allocation operates on - * these intervals, splitting them as necessary and assigning allocations - * to them as it runs. - */ -class LiveInterval - : public InlineListNode, - public TempObject -{ - public: - /* - * A range is a contiguous sequence of CodePositions where the virtual - * register associated with this interval is live. - */ - struct Range { - Range(CodePosition f, CodePosition t) - : from(f), - to(t) - { - JS_ASSERT(from < to); - } - CodePosition from; - - // The end of this range, exclusive. - CodePosition to; - }; - - private: - Vector ranges_; - LAllocation alloc_; - VirtualRegister *reg_; - uint32 index_; - Requirement requirement_; - Requirement hint_; - InlineForwardList uses_; - size_t lastProcessedRange_; - - public: - - LiveInterval(VirtualRegister *reg, uint32 index) - : reg_(reg), - index_(index), - lastProcessedRange_(size_t(-1)) - { } - - bool addRange(CodePosition from, CodePosition to); - bool addRangeAtHead(CodePosition from, CodePosition to); - void setFrom(CodePosition from); - CodePosition intersect(LiveInterval *other); - bool covers(CodePosition pos); - CodePosition nextCoveredAfter(CodePosition pos); - - CodePosition start() const { - JS_ASSERT(!ranges_.empty()); - return ranges_.back().from; - } - - CodePosition end() const { - JS_ASSERT(!ranges_.empty()); - return ranges_.begin()->to; - } - - size_t numRanges() const { - return ranges_.length(); - } - const Range *getRange(size_t i) const { - return &ranges_[i]; - } - void setLastProcessedRange(size_t range, mozilla::DebugOnly pos) { - // If the range starts after pos, we may not be able to use - // it in the next lastProcessedRangeIfValid call. - JS_ASSERT(ranges_[range].from <= pos); - lastProcessedRange_ = range; - } - size_t lastProcessedRangeIfValid(CodePosition pos) const { - if (lastProcessedRange_ < ranges_.length() && ranges_[lastProcessedRange_].from <= pos) - return lastProcessedRange_; - return ranges_.length() - 1; - } - - LAllocation *getAllocation() { - return &alloc_; - } - void setAllocation(LAllocation alloc) { - alloc_ = alloc; - } - VirtualRegister *reg() const { - return reg_; - } - uint32 index() const { - return index_; - } - void setIndex(uint32 index) { - index_ = index; - } - Requirement *requirement() { - return &requirement_; - } - void setRequirement(const Requirement &requirement) { - // A SAME_AS_OTHER requirement complicates regalloc too much; it - // should only be used as hint. - JS_ASSERT(requirement.kind() != Requirement::SAME_AS_OTHER); - - // Fixed registers are handled with fixed intervals, so fixed requirements - // are only valid for non-register allocations.f - JS_ASSERT_IF(requirement.kind() == Requirement::FIXED, - !requirement.allocation().isRegister()); - - requirement_ = requirement; - } - Requirement *hint() { - return &hint_; - } - void setHint(const Requirement &hint) { - hint_ = hint; - } - bool isSpill() const { - return alloc_.isStackSlot(); - } - bool splitFrom(CodePosition pos, LiveInterval *after); - - void addUse(UsePosition *use); - UsePosition *nextUseAfter(CodePosition pos); - CodePosition nextUsePosAfter(CodePosition pos); - CodePosition firstIncompatibleUse(LAllocation alloc); - - UsePositionIterator usesBegin() const { - return uses_.begin(); - } - - UsePositionIterator usesEnd() const { - return uses_.end(); - } - -#ifdef DEBUG - void validateRanges(); -#endif -}; - -/* - * Represents all of the register allocation state associated with a virtual - * register, including all associated intervals and pointers to relevant LIR - * structures. - */ -class VirtualRegister -{ - uint32 reg_; - LBlock *block_; - LInstruction *ins_; - LDefinition *def_; - Vector intervals_; LAllocation *canonicalSpill_; CodePosition spillPosition_ ; @@ -265,65 +29,7 @@ class VirtualRegister // processed by freeAllocation(). bool finished_ : 1; - // Whether def_ is a temp or an output. - bool isTemp_ : 1; - public: - bool init(uint32 reg, LBlock *block, LInstruction *ins, LDefinition *def, bool isTemp) { - reg_ = reg; - block_ = block; - ins_ = ins; - def_ = def; - isTemp_ = isTemp; - LiveInterval *initial = new LiveInterval(this, 0); - if (!initial) - return false; - return intervals_.append(initial); - } - uint32 reg() { - return reg_; - } - LBlock *block() { - return block_; - } - LInstruction *ins() { - return ins_; - } - LDefinition *def() const { - return def_; - } - LDefinition::Type type() const { - return def()->type(); - } - bool isTemp() const { - return isTemp_; - } - size_t numIntervals() const { - return intervals_.length(); - } - LiveInterval *getInterval(size_t i) const { - return intervals_[i]; - } - LiveInterval *lastInterval() const { - JS_ASSERT(numIntervals() > 0); - return getInterval(numIntervals() - 1); - } - bool addInterval(LiveInterval *interval) { - JS_ASSERT(interval->numRanges()); - - // Preserve ascending order for faster lookups. - LiveInterval **found = NULL; - LiveInterval **i; - for (i = intervals_.begin(); i != intervals_.end(); i++) { - if (!found && interval->start() < (*i)->start()) - found = i; - if (found) - (*i)->setIndex((*i)->index() + 1); - } - if (!found) - found = intervals_.end(); - return intervals_.insert(found, interval); - } void setCanonicalSpill(LAllocation *alloc) { canonicalSpill_ = alloc; } @@ -339,9 +45,6 @@ class VirtualRegister bool finished() const { return finished_; } - bool isDouble() const { - return def_->type() == LDefinition::DOUBLE; - } void setSpillAtDefinition(CodePosition pos) { spillAtDefinition_ = true; setSpillPosition(pos); @@ -355,63 +58,9 @@ class VirtualRegister void setSpillPosition(CodePosition pos) { spillPosition_ = pos; } - - LiveInterval *intervalFor(CodePosition pos); - LiveInterval *getFirstInterval(); }; -class VirtualRegisterMap -{ - private: - VirtualRegister *vregs_; - uint32 numVregs_; - - public: - VirtualRegisterMap() - : vregs_(NULL), - numVregs_(0) - { } - - bool init(MIRGenerator *gen, uint32 numVregs) { - vregs_ = gen->allocate(numVregs); - numVregs_ = numVregs; - if (!vregs_) - return false; - memset(vregs_, 0, sizeof(VirtualRegister) * numVregs); - return true; - } - VirtualRegister &operator[](unsigned int index) { - JS_ASSERT(index < numVregs_); - return vregs_[index]; - } - VirtualRegister &operator[](const LAllocation *alloc) { - JS_ASSERT(alloc->isUse()); - JS_ASSERT(alloc->toUse()->virtualRegister() < numVregs_); - return vregs_[alloc->toUse()->virtualRegister()]; - } - VirtualRegister &operator[](const LDefinition *def) { - JS_ASSERT(def->virtualRegister() < numVregs_); - return vregs_[def->virtualRegister()]; - } - uint32 numVirtualRegisters() const { - return numVregs_; - } -}; - -typedef HashMap, - IonAllocPolicy> InstructionMap; - -typedef HashMap, - IonAllocPolicy> LiveMap; - -typedef InlineList::iterator IntervalIterator; -typedef InlineList::reverse_iterator IntervalReverseIterator; - -class LinearScanAllocator : public RegisterAllocator +class LinearScanAllocator : public LiveRangeAllocator { friend class C1Spewer; friend class JSONSpewer; @@ -430,15 +79,6 @@ class LinearScanAllocator : public RegisterAllocator LiveInterval *dequeue(); }; - // Computed inforamtion - BitSet **liveIn; - VirtualRegisterMap vregs; - FixedArityList fixedIntervals; - - // Union of all ranges in fixedIntervals, used to quickly determine - // whether an interval intersects with a fixed register. - LiveInterval *fixedIntervalsUnion; - // Allocation state StackSlotAllocator stackSlotAllocator; @@ -454,8 +94,6 @@ class LinearScanAllocator : public RegisterAllocator InlineList handled; LiveInterval *current; - bool createDataStructures(); - bool buildLivenessInfo(); bool allocateRegisters(); bool resolveControlFlow(); bool reifyAllocations(); @@ -481,30 +119,23 @@ class LinearScanAllocator : public RegisterAllocator void setIntervalRequirement(LiveInterval *interval); size_t findFirstSafepoint(LiveInterval *interval, size_t firstSafepoint); size_t findFirstNonCallSafepoint(CodePosition from); - - bool addFixedRangeAtHead(AnyRegister reg, CodePosition from, CodePosition to) { - if (!fixedIntervals[reg.code()]->addRangeAtHead(from, to)) - return false; - return fixedIntervalsUnion->addRangeAtHead(from, to); - } + bool isSpilledAt(LiveInterval *interval, CodePosition pos); #ifdef DEBUG void validateIntervals(); void validateAllocations(); - void validateVirtualRegisters(); #else inline void validateIntervals() { }; inline void validateAllocations() { }; - inline void validateVirtualRegisters() { }; #endif #ifdef JS_NUNBOX32 - VirtualRegister *otherHalfOfNunbox(VirtualRegister *vreg); + LinearScanVirtualRegister *otherHalfOfNunbox(VirtualRegister *vreg); #endif public: LinearScanAllocator(MIRGenerator *mir, LIRGenerator *lir, LIRGraph &graph) - : RegisterAllocator(mir, lir, graph) + : LiveRangeAllocator(mir, lir, graph) { } diff --git a/js/src/ion/LiveRangeAllocator.cpp b/js/src/ion/LiveRangeAllocator.cpp new file mode 100644 index 000000000000..ab2d21f8e0f1 --- /dev/null +++ b/js/src/ion/LiveRangeAllocator.cpp @@ -0,0 +1,742 @@ +/* -*- Mode: C++; tab-width: 4; indent-tabs-mode: nil; c-basic-offset: 4 -*- + * vim: set ts=4 sw=4 et tw=99: + * + * This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ + +#include "LiveRangeAllocator.h" + +#include "LinearScan.h" + +using namespace js; +using namespace js::ion; + +using mozilla::DebugOnly; + +int +Requirement::priority() const +{ + switch (kind_) { + case Requirement::FIXED: + return 0; + + case Requirement::REGISTER: + return 1; + + case Requirement::NONE: + return 2; + + default: + JS_NOT_REACHED("Unknown requirement kind."); + return -1; + } +} + +bool +LiveInterval::addRangeAtHead(CodePosition from, CodePosition to) +{ + JS_ASSERT(from < to); + + Range newRange(from, to); + + if (ranges_.empty()) + return ranges_.append(newRange); + + Range &first = ranges_.back(); + if (to < first.from) + return ranges_.append(newRange); + + if (to == first.from) { + first.from = from; + return true; + } + + JS_ASSERT(from < first.to); + JS_ASSERT(to > first.from); + if (from < first.from) + first.from = from; + if (to > first.to) + first.to = to; + + return true; +} + +bool +LiveInterval::addRange(CodePosition from, CodePosition to) +{ + JS_ASSERT(from < to); + + Range newRange(from, to); + + Range *i; + // Find the location to insert the new range + for (i = ranges_.end() - 1; i >= ranges_.begin(); i--) { + if (newRange.from <= i->to) { + if (i->from < newRange.from) + newRange.from = i->from; + break; + } + } + // Perform coalescing on overlapping ranges + for (; i >= ranges_.begin(); i--) { + if (newRange.to < i->from) + break; + if (newRange.to < i->to) + newRange.to = i->to; + ranges_.erase(i); + } + + return ranges_.insert(i + 1, newRange); +} + +void +LiveInterval::setFrom(CodePosition from) +{ + while (!ranges_.empty()) { + if (ranges_.back().to < from) { + ranges_.erase(&ranges_.back()); + } else { + if (from == ranges_.back().to) + ranges_.erase(&ranges_.back()); + else + ranges_.back().from = from; + break; + } + } +} + +bool +LiveInterval::covers(CodePosition pos) +{ + if (pos < start() || pos >= end()) + return false; + + // Loop over the ranges in ascending order. + size_t i = lastProcessedRangeIfValid(pos); + for (; i < ranges_.length(); i--) { + if (pos < ranges_[i].from) + return false; + setLastProcessedRange(i, pos); + if (pos < ranges_[i].to) + return true; + } + return false; +} + +CodePosition +LiveInterval::nextCoveredAfter(CodePosition pos) +{ + for (size_t i = 0; i < ranges_.length(); i++) { + if (ranges_[i].to <= pos) { + if (i) + return ranges_[i-1].from; + break; + } + if (ranges_[i].from <= pos) + return pos; + } + return CodePosition::MIN; +} + +CodePosition +LiveInterval::intersect(LiveInterval *other) +{ + if (start() > other->start()) + return other->intersect(this); + + // Loop over the ranges in ascending order. As an optimization, + // try to start at the last processed range. + size_t i = lastProcessedRangeIfValid(other->start()); + size_t j = other->ranges_.length() - 1; + if (i >= ranges_.length() || j >= other->ranges_.length()) + return CodePosition::MIN; + + while (true) { + const Range &r1 = ranges_[i]; + const Range &r2 = other->ranges_[j]; + + if (r1.from <= r2.from) { + if (r1.from <= other->start()) + setLastProcessedRange(i, other->start()); + if (r2.from < r1.to) + return r2.from; + if (i == 0 || ranges_[i-1].from > other->end()) + break; + i--; + } else { + if (r1.from < r2.to) + return r1.from; + if (j == 0 || other->ranges_[j-1].from > end()) + break; + j--; + } + } + + return CodePosition::MIN; +} + +/* + * This function takes the callee interval and moves all ranges following or + * including provided position to the target interval. Additionally, if a + * range in the callee interval spans the given position, it is split and the + * latter half is placed in the target interval. + * + * This function should only be called if it is known that the interval should + * actually be split (and, presumably, a move inserted). As such, it is an + * error for the caller to request a split that moves all intervals into the + * target. Doing so will trip the assertion at the bottom of the function. + */ +bool +LiveInterval::splitFrom(CodePosition pos, LiveInterval *after) +{ + JS_ASSERT(pos >= start() && pos < end()); + JS_ASSERT(after->ranges_.empty()); + + // Move all intervals over to the target + size_t bufferLength = ranges_.length(); + Range *buffer = ranges_.extractRawBuffer(); + if (!buffer) + return false; + after->ranges_.replaceRawBuffer(buffer, bufferLength); + + // Move intervals back as required + for (Range *i = &after->ranges_.back(); i >= after->ranges_.begin(); i--) { + if (pos >= i->to) + continue; + + if (pos > i->from) { + // Split the range + Range split(i->from, pos); + i->from = pos; + if (!ranges_.append(split)) + return false; + } + if (!ranges_.append(i + 1, after->ranges_.end())) + return false; + after->ranges_.shrinkBy(after->ranges_.end() - i - 1); + break; + } + + // Split the linked list of use positions + UsePosition *prev = NULL; + for (UsePositionIterator usePos(usesBegin()); usePos != usesEnd(); usePos++) { + if (usePos->pos > pos) + break; + prev = *usePos; + } + + uses_.splitAfter(prev, &after->uses_); + return true; +} + +void +LiveInterval::addUse(UsePosition *use) +{ + // Insert use positions in ascending order. Note that instructions + // are visited in reverse order, so in most cases the loop terminates + // at the first iteration and the use position will be added to the + // front of the list. + UsePosition *prev = NULL; + for (UsePositionIterator current(usesBegin()); current != usesEnd(); current++) { + if (current->pos >= use->pos) + break; + prev = *current; + } + + if (prev) + uses_.insertAfter(prev, use); + else + uses_.pushFront(use); +} + +UsePosition * +LiveInterval::nextUseAfter(CodePosition after) +{ + for (UsePositionIterator usePos(usesBegin()); usePos != usesEnd(); usePos++) { + if (usePos->pos >= after) { + LUse::Policy policy = usePos->use->policy(); + JS_ASSERT(policy != LUse::RECOVERED_INPUT); + if (policy != LUse::KEEPALIVE) + return *usePos; + } + } + return NULL; +} + +/* + * This function locates the first "real" use of this interval that follows + * the given code position. Non-"real" uses are currently just snapshots, + * which keep virtual registers alive but do not result in the + * generation of code that use them. + */ +CodePosition +LiveInterval::nextUsePosAfter(CodePosition after) +{ + UsePosition *min = nextUseAfter(after); + return min ? min->pos : CodePosition::MAX; +} + +/* + * This function finds the position of the first use of this interval + * that is incompatible with the provideded allocation. For example, + * a use with a REGISTER policy would be incompatible with a stack slot + * allocation. + */ +CodePosition +LiveInterval::firstIncompatibleUse(LAllocation alloc) +{ + for (UsePositionIterator usePos(usesBegin()); usePos != usesEnd(); usePos++) { + if (!UseCompatibleWith(usePos->use, alloc)) + return usePos->pos; + } + return CodePosition::MAX; +} + +LiveInterval * +VirtualRegister::intervalFor(CodePosition pos) +{ + for (LiveInterval **i = intervals_.begin(); i != intervals_.end(); i++) { + if ((*i)->covers(pos)) + return *i; + if (pos < (*i)->end()) + break; + } + return NULL; +} + +LiveInterval * +VirtualRegister::getFirstInterval() +{ + JS_ASSERT(!intervals_.empty()); + return intervals_[0]; +} + +// Dummy function to instantiate LiveRangeAllocator for each template instance. +void +EnsureLiveRangeAllocatorInstantiation(MIRGenerator *mir, LIRGenerator *lir, LIRGraph &graph) +{ + LiveRangeAllocator allocator(mir, lir, graph); + allocator.buildLivenessInfo(); +} + +#ifdef DEBUG +static inline bool +NextInstructionHasFixedUses(LBlock *block, LInstruction *ins) +{ + LInstructionIterator iter(block->begin(ins)); + iter++; + for (LInstruction::InputIterator alloc(**iter); alloc.more(); alloc.next()) { + if (alloc->isUse() && alloc->toUse()->isFixedRegister()) + return true; + } + return false; +} + +// Returns true iff ins has a def/temp reusing the input allocation. +static bool +IsInputReused(LInstruction *ins, LUse *use) +{ + for (size_t i = 0; i < ins->numDefs(); i++) { + if (ins->getDef(i)->policy() == LDefinition::MUST_REUSE_INPUT && + ins->getOperand(ins->getDef(i)->getReusedInput())->toUse() == use) + { + return true; + } + } + + for (size_t i = 0; i < ins->numTemps(); i++) { + if (ins->getTemp(i)->policy() == LDefinition::MUST_REUSE_INPUT && + ins->getOperand(ins->getTemp(i)->getReusedInput())->toUse() == use) + { + return true; + } + } + + return false; +} +#endif + +/* + * This function pre-allocates and initializes as much global state as possible + * to avoid littering the algorithms with memory management cruft. + */ +template +bool +LiveRangeAllocator::init() +{ + if (!RegisterAllocator::init()) + return false; + + liveIn = lir->mir()->allocate(graph.numBlockIds()); + if (!liveIn) + return false; + + // Initialize fixed intervals. + for (size_t i = 0; i < AnyRegister::Total; i++) { + AnyRegister reg = AnyRegister::FromCode(i); + LiveInterval *interval = new LiveInterval(0); + interval->setAllocation(LAllocation(reg)); + fixedIntervals[i] = interval; + } + + fixedIntervalsUnion = new LiveInterval(0); + + if (!vregs.init(lir->mir(), graph.numVirtualRegisters())) + return false; + + // Build virtual register objects + for (size_t i = 0; i < graph.numBlocks(); i++) { + if (mir->shouldCancel("LSRA create data structures (main loop)")) + return false; + + LBlock *block = graph.getBlock(i); + for (LInstructionIterator ins = block->begin(); ins != block->end(); ins++) { + for (size_t j = 0; j < ins->numDefs(); j++) { + LDefinition *def = ins->getDef(j); + if (def->policy() != LDefinition::PASSTHROUGH) { + uint32 reg = def->virtualRegister(); + if (!vregs[reg].init(reg, block, *ins, def, /* isTemp */ false)) + return false; + } + } + + for (size_t j = 0; j < ins->numTemps(); j++) { + LDefinition *def = ins->getTemp(j); + if (def->isBogusTemp()) + continue; + if (!vregs[def].init(def->virtualRegister(), block, *ins, def, /* isTemp */ true)) + return false; + } + } + for (size_t j = 0; j < block->numPhis(); j++) { + LPhi *phi = block->getPhi(j); + LDefinition *def = phi->getDef(0); + if (!vregs[def].init(phi->id(), block, phi, def, /* isTemp */ false)) + return false; + } + } + + return true; +} + +/* + * This function builds up liveness intervals for all virtual registers + * defined in the function. Additionally, it populates the liveIn array with + * information about which registers are live at the beginning of a block, to + * aid resolution and reification in a later phase. + * + * The algorithm is based on the one published in: + * + * Wimmer, Christian, and Michael Franz. "Linear Scan Register Allocation on + * SSA Form." Proceedings of the International Symposium on Code Generation + * and Optimization. Toronto, Ontario, Canada, ACM. 2010. 170-79. PDF. + * + * The algorithm operates on blocks ordered such that dominators of a block + * are before the block itself, and such that all blocks of a loop are + * contiguous. It proceeds backwards over the instructions in this order, + * marking registers live at their uses, ending their live intervals at + * definitions, and recording which registers are live at the top of every + * block. To deal with loop backedges, variables live at the beginning of + * a loop gain an interval covering the entire loop. + */ +template +bool +LiveRangeAllocator::buildLivenessInfo() +{ + if (!init()) + return false; + + Vector loopWorkList; + BitSet *loopDone = BitSet::New(graph.numBlockIds()); + if (!loopDone) + return false; + + for (size_t i = graph.numBlocks(); i > 0; i--) { + if (mir->shouldCancel("LSRA Build Liveness Info (main loop)")) + return false; + + LBlock *block = graph.getBlock(i - 1); + MBasicBlock *mblock = block->mir(); + + BitSet *live = BitSet::New(graph.numVirtualRegisters()); + if (!live) + return false; + liveIn[mblock->id()] = live; + + // Propagate liveIn from our successors to us + for (size_t i = 0; i < mblock->lastIns()->numSuccessors(); i++) { + MBasicBlock *successor = mblock->lastIns()->getSuccessor(i); + // Skip backedges, as we fix them up at the loop header. + if (mblock->id() < successor->id()) + live->insertAll(liveIn[successor->id()]); + } + + // Add successor phis + if (mblock->successorWithPhis()) { + LBlock *phiSuccessor = mblock->successorWithPhis()->lir(); + for (unsigned int j = 0; j < phiSuccessor->numPhis(); j++) { + LPhi *phi = phiSuccessor->getPhi(j); + LAllocation *use = phi->getOperand(mblock->positionInPhiSuccessor()); + uint32 reg = use->toUse()->virtualRegister(); + live->insert(reg); + } + } + + // Variables are assumed alive for the entire block, a define shortens + // the interval to the point of definition. + for (BitSet::Iterator liveRegId(*live); liveRegId; liveRegId++) { + if (!vregs[*liveRegId].getInterval(0)->addRangeAtHead(inputOf(block->firstId()), + outputOf(block->lastId()).next())) + { + return false; + } + } + + // Shorten the front end of live intervals for live variables to their + // point of definition, if found. + for (LInstructionReverseIterator ins = block->rbegin(); ins != block->rend(); ins++) { + // Calls may clobber registers, so force a spill and reload around the callsite. + if (ins->isCall()) { + for (AnyRegisterIterator iter(allRegisters_); iter.more(); iter++) { + if (!addFixedRangeAtHead(*iter, inputOf(*ins), outputOf(*ins))) + return false; + } + } + + for (size_t i = 0; i < ins->numDefs(); i++) { + if (ins->getDef(i)->policy() != LDefinition::PASSTHROUGH) { + LDefinition *def = ins->getDef(i); + + CodePosition from; + if (def->policy() == LDefinition::PRESET && def->output()->isRegister()) { + // The fixed range covers the current instruction so the + // interval for the virtual register starts at the next + // instruction. If the next instruction has a fixed use, + // this can lead to unnecessary register moves. To avoid + // special handling for this, assert the next instruction + // has no fixed uses. defineFixed guarantees this by inserting + // an LNop. + JS_ASSERT(!NextInstructionHasFixedUses(block, *ins)); + AnyRegister reg = def->output()->toRegister(); + if (!addFixedRangeAtHead(reg, inputOf(*ins), outputOf(*ins).next())) + return false; + from = outputOf(*ins).next(); + } else { + from = inputOf(*ins); + } + + if (def->policy() == LDefinition::MUST_REUSE_INPUT) { + // MUST_REUSE_INPUT is implemented by allocating an output + // register and moving the input to it. Register hints are + // used to avoid unnecessary moves. We give the input an + // LUse::ANY policy to avoid allocating a register for the + // input. + LUse *inputUse = ins->getOperand(def->getReusedInput())->toUse(); + JS_ASSERT(inputUse->policy() == LUse::REGISTER); + JS_ASSERT(inputUse->usedAtStart()); + *inputUse = LUse(inputUse->virtualRegister(), LUse::ANY, /* usedAtStart = */ true); + } + + LiveInterval *interval = vregs[def].getInterval(0); + interval->setFrom(from); + + // Ensure that if there aren't any uses, there's at least + // some interval for the output to go into. + if (interval->numRanges() == 0) { + if (!interval->addRangeAtHead(from, from.next())) + return false; + } + live->remove(def->virtualRegister()); + } + } + + for (size_t i = 0; i < ins->numTemps(); i++) { + LDefinition *temp = ins->getTemp(i); + if (temp->isBogusTemp()) + continue; + if (ins->isCall()) { + JS_ASSERT(temp->isPreset()); + continue; + } + + if (temp->policy() == LDefinition::PRESET) { + AnyRegister reg = temp->output()->toRegister(); + if (!addFixedRangeAtHead(reg, inputOf(*ins), outputOf(*ins))) + return false; + } else { + if (!vregs[temp].getInterval(0)->addRangeAtHead(inputOf(*ins), outputOf(*ins))) + return false; + } + } + + DebugOnly hasUseRegister = false; + DebugOnly hasUseRegisterAtStart = false; + + for (LInstruction::InputIterator alloc(**ins); alloc.more(); alloc.next()) { + if (alloc->isUse()) { + LUse *use = alloc->toUse(); + + // The first instruction, LLabel, has no uses. + JS_ASSERT(inputOf(*ins) > outputOf(block->firstId())); + + // Call uses should always be at-start or fixed, since the fixed intervals + // use all registers. + JS_ASSERT_IF(ins->isCall() && !alloc.isSnapshotInput(), + use->isFixedRegister() || use->usedAtStart()); + +#ifdef DEBUG + // Don't allow at-start call uses if there are temps of the same kind, + // so that we don't assign the same register. + if (ins->isCall() && use->usedAtStart()) { + for (size_t i = 0; i < ins->numTemps(); i++) + JS_ASSERT(vregs[ins->getTemp(i)].isDouble() != vregs[use].isDouble()); + } + + // If there are both useRegisterAtStart(x) and useRegister(y) + // uses, we may assign the same register to both operands due to + // interval splitting (bug 772830). Don't allow this for now. + if (use->policy() == LUse::REGISTER) { + if (use->usedAtStart()) { + if (!IsInputReused(*ins, use)) + hasUseRegisterAtStart = true; + } else { + hasUseRegister = true; + } + } + + JS_ASSERT(!(hasUseRegister && hasUseRegisterAtStart)); +#endif + + // Don't treat RECOVERED_INPUT uses as keeping the vreg alive. + if (use->policy() == LUse::RECOVERED_INPUT) + continue; + + CodePosition to; + if (use->isFixedRegister()) { + JS_ASSERT(!use->usedAtStart()); + AnyRegister reg = GetFixedRegister(vregs[use].def(), use); + if (!addFixedRangeAtHead(reg, inputOf(*ins), outputOf(*ins))) + return false; + to = inputOf(*ins); + } else { + to = use->usedAtStart() ? inputOf(*ins) : outputOf(*ins); + } + + LiveInterval *interval = vregs[use].getInterval(0); + if (!interval->addRangeAtHead(inputOf(block->firstId()), to)) + return false; + interval->addUse(new UsePosition(use, to)); + + live->insert(use->virtualRegister()); + } + } + } + + // Phis have simultaneous assignment semantics at block begin, so at + // the beginning of the block we can be sure that liveIn does not + // contain any phi outputs. + for (unsigned int i = 0; i < block->numPhis(); i++) { + LDefinition *def = block->getPhi(i)->getDef(0); + if (live->contains(def->virtualRegister())) { + live->remove(def->virtualRegister()); + } else { + // This is a dead phi, so add a dummy range over all phis. This + // can go away if we have an earlier dead code elimination pass. + if (!vregs[def].getInterval(0)->addRangeAtHead(inputOf(block->firstId()), + outputOf(block->firstId()))) + { + return false; + } + } + } + + if (mblock->isLoopHeader()) { + // A divergence from the published algorithm is required here, as + // our block order does not guarantee that blocks of a loop are + // contiguous. As a result, a single live interval spanning the + // loop is not possible. Additionally, we require liveIn in a later + // pass for resolution, so that must also be fixed up here. + MBasicBlock *loopBlock = mblock->backedge(); + while (true) { + // Blocks must already have been visited to have a liveIn set. + JS_ASSERT(loopBlock->id() >= mblock->id()); + + // Add an interval for this entire loop block + CodePosition from = inputOf(loopBlock->lir()->firstId()); + CodePosition to = outputOf(loopBlock->lir()->lastId()).next(); + + for (BitSet::Iterator liveRegId(*live); liveRegId; liveRegId++) { + if (!vregs[*liveRegId].getInterval(0)->addRange(from, to)) + return false; + } + + // Fix up the liveIn set to account for the new interval + liveIn[loopBlock->id()]->insertAll(live); + + // Make sure we don't visit this node again + loopDone->insert(loopBlock->id()); + + // If this is the loop header, any predecessors are either the + // backedge or out of the loop, so skip any predecessors of + // this block + if (loopBlock != mblock) { + for (size_t i = 0; i < loopBlock->numPredecessors(); i++) { + MBasicBlock *pred = loopBlock->getPredecessor(i); + if (loopDone->contains(pred->id())) + continue; + if (!loopWorkList.append(pred)) + return false; + } + } + + // Terminate loop if out of work. + if (loopWorkList.empty()) + break; + + // Grab the next block off the work list, skipping any OSR block. + do { + loopBlock = loopWorkList.popCopy(); + } while (loopBlock->lir() == graph.osrBlock()); + } + + // Clear the done set for other loops + loopDone->clear(); + } + + JS_ASSERT_IF(!mblock->numPredecessors(), live->empty()); + } + + validateVirtualRegisters(); + + // If the script has an infinite loop, there may be no MReturn and therefore + // no fixed intervals. Add a small range to fixedIntervalsUnion so that the + // rest of the allocator can assume it has at least one range. + if (fixedIntervalsUnion->numRanges() == 0) { + if (!fixedIntervalsUnion->addRangeAtHead(CodePosition(0, CodePosition::INPUT), + CodePosition(0, CodePosition::OUTPUT))) + { + return false; + } + } + + return true; +} + +#ifdef DEBUG + +void +LiveInterval::validateRanges() +{ + Range *prev = NULL; + + for (size_t i = ranges_.length() - 1; i < ranges_.length(); i--) { + Range *range = &ranges_[i]; + + JS_ASSERT(range->from < range->to); + JS_ASSERT_IF(prev, prev->to <= range->from); + prev = range; + } +} + +#endif // DEBUG diff --git a/js/src/ion/LiveRangeAllocator.h b/js/src/ion/LiveRangeAllocator.h new file mode 100644 index 000000000000..d92a44a6114d --- /dev/null +++ b/js/src/ion/LiveRangeAllocator.h @@ -0,0 +1,519 @@ +/* -*- Mode: C++; tab-width: 4; indent-tabs-mode: nil; c-basic-offset: 4 -*- + * vim: set ts=4 sw=4 et tw=99: + * + * This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ + +#ifndef js_ion_liverangeallocator_h__ +#define js_ion_liverangeallocator_h__ + +#include "RegisterAllocator.h" + +// Common structures and functions used by register allocators that operate on +// virtual register live ranges. + +namespace js { +namespace ion { + +class Requirement +{ + public: + enum Kind { + NONE, + REGISTER, + FIXED, + SAME_AS_OTHER + }; + + Requirement() + : kind_(NONE) + { } + + Requirement(Kind kind) + : kind_(kind) + { + // These have dedicated constructors; + JS_ASSERT(kind != FIXED && kind != SAME_AS_OTHER); + } + + Requirement(Kind kind, CodePosition at) + : kind_(kind), + position_(at) + { } + + Requirement(LAllocation fixed) + : kind_(FIXED), + allocation_(fixed) + { } + + // Only useful as a hint, encodes where the fixed requirement is used to + // avoid allocating a fixed register too early. + Requirement(LAllocation fixed, CodePosition at) + : kind_(FIXED), + allocation_(fixed), + position_(at) + { } + + Requirement(uint32 vreg, CodePosition at) + : kind_(SAME_AS_OTHER), + allocation_(LUse(vreg, LUse::ANY)), + position_(at) + { } + + Kind kind() const { + return kind_; + } + + LAllocation allocation() const { + JS_ASSERT(!allocation_.isUse()); + return allocation_; + } + + uint32 virtualRegister() const { + JS_ASSERT(allocation_.isUse()); + return allocation_.toUse()->virtualRegister(); + } + + CodePosition pos() const { + return position_; + } + + int priority() const; + + private: + Kind kind_; + LAllocation allocation_; + CodePosition position_; +}; + +struct UsePosition : public TempObject, + public InlineForwardListNode +{ + LUse *use; + CodePosition pos; + + UsePosition(LUse *use, CodePosition pos) : + use(use), + pos(pos) + { } +}; + +typedef InlineForwardListIterator UsePositionIterator; + +static inline bool +UseCompatibleWith(const LUse *use, LAllocation alloc) +{ + switch (use->policy()) { + case LUse::ANY: + case LUse::KEEPALIVE: + return alloc.isRegister() || alloc.isMemory(); + case LUse::REGISTER: + return alloc.isRegister(); + case LUse::FIXED: + // Fixed uses are handled using fixed intervals. The + // UsePosition is only used as hint. + return alloc.isRegister(); + default: + JS_NOT_REACHED("Unknown use policy"); + } + return false; +} + +#ifdef DEBUG + +static inline bool +DefinitionCompatibleWith(LInstruction *ins, const LDefinition *def, LAllocation alloc) +{ + if (ins->isPhi()) { + if (def->type() == LDefinition::DOUBLE) + return alloc.isFloatReg() || alloc.kind() == LAllocation::DOUBLE_SLOT; + return alloc.isGeneralReg() || alloc.kind() == LAllocation::STACK_SLOT; + } + + switch (def->policy()) { + case LDefinition::DEFAULT: + if (!alloc.isRegister()) + return false; + return alloc.isFloatReg() == (def->type() == LDefinition::DOUBLE); + case LDefinition::PRESET: + return alloc == *def->output(); + case LDefinition::MUST_REUSE_INPUT: + if (!alloc.isRegister() || !ins->numOperands()) + return false; + return alloc == *ins->getOperand(def->getReusedInput()); + case LDefinition::PASSTHROUGH: + return true; + default: + JS_NOT_REACHED("Unknown definition policy"); + } + return false; +} + +#endif // DEBUG + +/* + * A live interval is a set of disjoint ranges of code positions where a + * virtual register is live. Register allocation operates on these intervals, + * splitting them as necessary and assigning allocations to them as it runs. + */ +class LiveInterval + : public InlineListNode, + public TempObject +{ + public: + /* + * A range is a contiguous sequence of CodePositions where the virtual + * register associated with this interval is live. + */ + struct Range { + Range(CodePosition f, CodePosition t) + : from(f), + to(t) + { + JS_ASSERT(from < to); + } + CodePosition from; + + // The end of this range, exclusive. + CodePosition to; + }; + + private: + Vector ranges_; + LAllocation alloc_; + uint32 vreg_; + uint32 index_; + Requirement requirement_; + Requirement hint_; + InlineForwardList uses_; + size_t lastProcessedRange_; + + public: + + LiveInterval(uint32 vreg, uint32 index) + : vreg_(vreg), + index_(index), + lastProcessedRange_(size_t(-1)) + { } + + LiveInterval(uint32 index) + : vreg_(UINT32_MAX), + index_(index), + lastProcessedRange_(size_t(-1)) + { } + + bool addRange(CodePosition from, CodePosition to); + bool addRangeAtHead(CodePosition from, CodePosition to); + void setFrom(CodePosition from); + CodePosition intersect(LiveInterval *other); + bool covers(CodePosition pos); + CodePosition nextCoveredAfter(CodePosition pos); + + CodePosition start() const { + JS_ASSERT(!ranges_.empty()); + return ranges_.back().from; + } + + CodePosition end() const { + JS_ASSERT(!ranges_.empty()); + return ranges_.begin()->to; + } + + size_t numRanges() const { + return ranges_.length(); + } + const Range *getRange(size_t i) const { + return &ranges_[i]; + } + void setLastProcessedRange(size_t range, mozilla::DebugOnly pos) { + // If the range starts after pos, we may not be able to use + // it in the next lastProcessedRangeIfValid call. + JS_ASSERT(ranges_[range].from <= pos); + lastProcessedRange_ = range; + } + size_t lastProcessedRangeIfValid(CodePosition pos) const { + if (lastProcessedRange_ < ranges_.length() && ranges_[lastProcessedRange_].from <= pos) + return lastProcessedRange_; + return ranges_.length() - 1; + } + + LAllocation *getAllocation() { + return &alloc_; + } + void setAllocation(LAllocation alloc) { + alloc_ = alloc; + } + bool hasVreg() const { + return vreg_ != UINT32_MAX; + } + uint32 vreg() const { + JS_ASSERT(hasVreg()); + return vreg_; + } + uint32 index() const { + return index_; + } + void setIndex(uint32 index) { + index_ = index; + } + Requirement *requirement() { + return &requirement_; + } + void setRequirement(const Requirement &requirement) { + // A SAME_AS_OTHER requirement complicates regalloc too much; it + // should only be used as hint. + JS_ASSERT(requirement.kind() != Requirement::SAME_AS_OTHER); + + // Fixed registers are handled with fixed intervals, so fixed requirements + // are only valid for non-register allocations.f + JS_ASSERT_IF(requirement.kind() == Requirement::FIXED, + !requirement.allocation().isRegister()); + + requirement_ = requirement; + } + Requirement *hint() { + return &hint_; + } + void setHint(const Requirement &hint) { + hint_ = hint; + } + bool isSpill() const { + return alloc_.isStackSlot(); + } + bool splitFrom(CodePosition pos, LiveInterval *after); + + void addUse(UsePosition *use); + UsePosition *nextUseAfter(CodePosition pos); + CodePosition nextUsePosAfter(CodePosition pos); + CodePosition firstIncompatibleUse(LAllocation alloc); + + UsePositionIterator usesBegin() const { + return uses_.begin(); + } + + UsePositionIterator usesEnd() const { + return uses_.end(); + } + +#ifdef DEBUG + void validateRanges(); +#endif +}; + +/* + * Represents all of the register allocation state associated with a virtual + * register, including all associated intervals and pointers to relevant LIR + * structures. + */ +class VirtualRegister +{ + uint32 id_; + LBlock *block_; + LInstruction *ins_; + LDefinition *def_; + Vector intervals_; + + // Whether def_ is a temp or an output. + bool isTemp_ : 1; + + public: + bool init(uint32 id, LBlock *block, LInstruction *ins, LDefinition *def, bool isTemp) { + id_ = id; + block_ = block; + ins_ = ins; + def_ = def; + isTemp_ = isTemp; + LiveInterval *initial = new LiveInterval(def->virtualRegister(), 0); + if (!initial) + return false; + return intervals_.append(initial); + } + uint32 id() { + return id_; + } + LBlock *block() { + return block_; + } + LInstruction *ins() { + return ins_; + } + LDefinition *def() const { + return def_; + } + LDefinition::Type type() const { + return def()->type(); + } + bool isTemp() const { + return isTemp_; + } + size_t numIntervals() const { + return intervals_.length(); + } + LiveInterval *getInterval(size_t i) const { + return intervals_[i]; + } + LiveInterval *lastInterval() const { + JS_ASSERT(numIntervals() > 0); + return getInterval(numIntervals() - 1); + } + bool addInterval(LiveInterval *interval) { + JS_ASSERT(interval->numRanges()); + + // Preserve ascending order for faster lookups. + LiveInterval **found = NULL; + LiveInterval **i; + for (i = intervals_.begin(); i != intervals_.end(); i++) { + if (!found && interval->start() < (*i)->start()) + found = i; + if (found) + (*i)->setIndex((*i)->index() + 1); + } + if (!found) + found = intervals_.end(); + return intervals_.insert(found, interval); + } + bool isDouble() const { + return def_->type() == LDefinition::DOUBLE; + } + + LiveInterval *intervalFor(CodePosition pos); + LiveInterval *getFirstInterval(); +}; + +// Index of the virtual registers in a graph. VREG is a subclass of +// VirtualRegister extended with any allocator specific state for the vreg. +template +class VirtualRegisterMap +{ + private: + VREG *vregs_; + uint32 numVregs_; + + public: + VirtualRegisterMap() + : vregs_(NULL), + numVregs_(0) + { } + + bool init(MIRGenerator *gen, uint32 numVregs) { + vregs_ = gen->allocate(numVregs); + numVregs_ = numVregs; + if (!vregs_) + return false; + memset(vregs_, 0, sizeof(VREG) * numVregs); + return true; + } + VREG &operator[](unsigned int index) { + JS_ASSERT(index < numVregs_); + return vregs_[index]; + } + VREG &operator[](const LAllocation *alloc) { + JS_ASSERT(alloc->isUse()); + JS_ASSERT(alloc->toUse()->virtualRegister() < numVregs_); + return vregs_[alloc->toUse()->virtualRegister()]; + } + VREG &operator[](const LDefinition *def) { + JS_ASSERT(def->virtualRegister() < numVregs_); + return vregs_[def->virtualRegister()]; + } + uint32 numVirtualRegisters() const { + return numVregs_; + } +}; + +static inline AnyRegister +GetFixedRegister(LDefinition *def, LUse *use) +{ + return def->type() == LDefinition::DOUBLE + ? AnyRegister(FloatRegister::FromCode(use->registerCode())) + : AnyRegister(Register::FromCode(use->registerCode())); +} + +static inline bool +IsNunbox(VirtualRegister *vreg) +{ +#ifdef JS_NUNBOX32 + return (vreg->type() == LDefinition::TYPE || + vreg->type() == LDefinition::PAYLOAD); +#else + return false; +#endif +} + +static inline bool +IsTraceable(VirtualRegister *reg) +{ + if (reg->type() == LDefinition::OBJECT) + return true; +#ifdef JS_PUNBOX64 + if (reg->type() == LDefinition::BOX) + return true; +#endif + return false; +} + +typedef InlineList::iterator IntervalIterator; +typedef InlineList::reverse_iterator IntervalReverseIterator; + +template +class LiveRangeAllocator : public RegisterAllocator +{ + protected: + // Computed inforamtion + BitSet **liveIn; + VirtualRegisterMap vregs; + FixedArityList fixedIntervals; + + // Union of all ranges in fixedIntervals, used to quickly determine + // whether an interval intersects with a fixed register. + LiveInterval *fixedIntervalsUnion; + + public: + LiveRangeAllocator(MIRGenerator *mir, LIRGenerator *lir, LIRGraph &graph) + : RegisterAllocator(mir, lir, graph), + liveIn(NULL), + fixedIntervalsUnion(NULL) + { + } + + bool buildLivenessInfo(); + + protected: + bool init(); + + bool addFixedRangeAtHead(AnyRegister reg, CodePosition from, CodePosition to) { + if (!fixedIntervals[reg.code()]->addRangeAtHead(from, to)) + return false; + return fixedIntervalsUnion->addRangeAtHead(from, to); + } + + void validateVirtualRegisters() + { +#ifdef DEBUG + for (size_t i = 1; i < graph.numVirtualRegisters(); i++) { + VirtualRegister *reg = &vregs[i]; + + LiveInterval *prev = NULL; + for (size_t j = 0; j < reg->numIntervals(); j++) { + LiveInterval *interval = reg->getInterval(j); + JS_ASSERT(interval->vreg() == i); + JS_ASSERT(interval->index() == j); + + if (interval->numRanges() == 0) + continue; + + JS_ASSERT_IF(prev, prev->end() <= interval->start()); + interval->validateRanges(); + + prev = interval; + } + } +#endif + } +}; + +} // namespace ion +} // namespace js + +#endif From c2eb04641e1573ba3b4c1fc391aa0cab2baa52f6 Mon Sep 17 00:00:00 2001 From: Trevor Saunders Date: Sat, 1 Dec 2012 19:57:25 -0500 Subject: [PATCH 02/12] bug 801466 - part 5 uriloader/ and rdf/ r=ehsan --- rdf/base/src/nsRDFContentSink.cpp | 7 ++-- rdf/datasource/src/nsFileSystemDataSource.cpp | 35 ++++++------------- .../exthandler/nsExternalHelperAppService.cpp | 20 ++++------- 3 files changed, 21 insertions(+), 41 deletions(-) diff --git a/rdf/base/src/nsRDFContentSink.cpp b/rdf/base/src/nsRDFContentSink.cpp index 97e7f0609e9b..1a4e02822d7c 100644 --- a/rdf/base/src/nsRDFContentSink.cpp +++ b/rdf/base/src/nsRDFContentSink.cpp @@ -56,7 +56,6 @@ #include "nsTArray.h" #include "nsXPIDLString.h" #include "prlog.h" -#include "prmem.h" #include "rdf.h" #include "rdfutil.h" #include "nsReadableUtils.h" @@ -356,7 +355,7 @@ RDFContentSinkImpl::~RDFContentSinkImpl() delete mContextStack; } - PR_FREEIF(mText); + moz_free(mText); if (--gRefCnt == 0) { @@ -763,7 +762,7 @@ RDFContentSinkImpl::AddText(const PRUnichar* aText, int32_t aLength) { // Create buffer when we first need it if (0 == mTextSize) { - mText = (PRUnichar *) PR_MALLOC(sizeof(PRUnichar) * 4096); + mText = (PRUnichar *) moz_malloc(sizeof(PRUnichar) * 4096); if (!mText) { return NS_ERROR_OUT_OF_MEMORY; } @@ -781,7 +780,7 @@ RDFContentSinkImpl::AddText(const PRUnichar* aText, int32_t aLength) int32_t newSize = (2 * mTextSize > (mTextSize + aLength)) ? (2 * mTextSize) : (mTextSize + aLength); PRUnichar* newText = - (PRUnichar *) PR_REALLOC(mText, sizeof(PRUnichar) * newSize); + (PRUnichar *) moz_realloc(mText, sizeof(PRUnichar) * newSize); if (!newText) return NS_ERROR_OUT_OF_MEMORY; mTextSize = newSize; diff --git a/rdf/datasource/src/nsFileSystemDataSource.cpp b/rdf/datasource/src/nsFileSystemDataSource.cpp index ea669a2062e8..ceb5f7af7d17 100644 --- a/rdf/datasource/src/nsFileSystemDataSource.cpp +++ b/rdf/datasource/src/nsFileSystemDataSource.cpp @@ -18,13 +18,6 @@ #include "nsXPIDLString.h" #include "nsRDFCID.h" #include "rdfutil.h" -#include "plhash.h" -#include "plstr.h" -#include "prlong.h" -#include "prlog.h" -#include "prmem.h" -#include "prprf.h" -#include "prio.h" #include "rdf.h" #include "nsEnumeratorUtils.h" #include "nsIURL.h" @@ -871,7 +864,6 @@ FileSystemDataSource::GetVolumeList(nsISimpleEnumerator** aResult) int32_t driveType; PRUnichar drive[32]; int32_t volNum; - char *url; for (volNum = 0; volNum < 26; volNum++) { @@ -880,15 +872,13 @@ FileSystemDataSource::GetVolumeList(nsISimpleEnumerator** aResult) driveType = GetDriveTypeW(drive); if (driveType != DRIVE_UNKNOWN && driveType != DRIVE_NO_ROOT_DIR) { - if (nullptr != (url = PR_smprintf("file:///%c|/", volNum + 'A'))) - { - rv = mRDFService->GetResource(nsDependentCString(url), - getter_AddRefs(vol)); - PR_Free(url); + nsAutoCString url; + url.AppendPrintf("file:///%c|/", volNum + 'A'); + rv = mRDFService->GetResource(url, getter_AddRefs(vol)); + if (NS_FAILED(rv)) + return rv; - if (NS_FAILED(rv)) return rv; - volumes->AppendElement(vol); - } + volumes->AppendElement(vol); } } #endif @@ -901,7 +891,6 @@ FileSystemDataSource::GetVolumeList(nsISimpleEnumerator** aResult) #ifdef XP_OS2 ULONG ulDriveNo = 0; ULONG ulDriveMap = 0; - char *url; rv = DosQueryCurrentDisk(&ulDriveNo, &ulDriveMap); if (NS_FAILED(rv)) @@ -911,14 +900,12 @@ FileSystemDataSource::GetVolumeList(nsISimpleEnumerator** aResult) { if (((ulDriveMap << (31 - volNum)) >> 31)) { - if (nullptr != (url = PR_smprintf("file:///%c|/", volNum + 'A'))) - { - rv = mRDFService->GetResource(nsDependentCString(url), getter_AddRefs(vol)); - PR_Free(url); + nsAutoCString url; + url.AppendPrintf("file:///%c|/", volNum + 'A'); + rv = mRDFService->GetResource(nsDependentCString(url), getter_AddRefs(vol)); - if (NS_FAILED(rv)) return rv; - volumes->AppendElement(vol); - } + if (NS_FAILED(rv)) return rv; + volumes->AppendElement(vol); } } diff --git a/uriloader/exthandler/nsExternalHelperAppService.cpp b/uriloader/exthandler/nsExternalHelperAppService.cpp index 9e29504a6032..ab6073d3b49e 100644 --- a/uriloader/exthandler/nsExternalHelperAppService.cpp +++ b/uriloader/exthandler/nsExternalHelperAppService.cpp @@ -11,6 +11,7 @@ #include "base/basictypes.h" /* This must occur *after* base/basictypes.h to avoid typedefs conflicts. */ +#include "mozilla/Base64.h" #include "mozilla/Util.h" #include "mozilla/dom/ContentChild.h" @@ -26,6 +27,7 @@ #include "nsIDirectoryService.h" #include "nsAppDirectoryServiceDefs.h" #include "nsICategoryManager.h" +#include "nsDependentSubstring.h" #include "nsXPIDLString.h" #include "nsUnicharUtils.h" #include "nsIStringEnumerator.h" @@ -97,8 +99,6 @@ #include "nsLocalHandlerApp.h" #include "nsIRandomGenerator.h" -#include "plbase64.h" -#include "prmem.h" #include "ContentChild.h" #include "nsXULAppAPI.h" @@ -1305,20 +1305,14 @@ nsresult nsExternalAppHandler::SetUpTempFile(nsIChannel * aChannel) rv = rg->GenerateRandomBytes(requiredBytesLength, &buffer); NS_ENSURE_SUCCESS(rv, rv); - char *b64 = PL_Base64Encode(reinterpret_cast(buffer), - requiredBytesLength, nullptr); + nsAutoCString tempLeafName; + nsDependentCSubstring randomData(reinterpret_cast(buffer), requiredBytesLength); + rv = Base64Encode(randomData, tempLeafName); NS_Free(buffer); buffer = nullptr; + NS_ENSURE_SUCCESS(rv, rv); - if (!b64) - return NS_ERROR_OUT_OF_MEMORY; - - NS_ASSERTION(strlen(b64) >= wantedFileNameLength, - "not enough bytes produced for conversion!"); - - nsAutoCString tempLeafName(b64, wantedFileNameLength); - PR_Free(b64); - b64 = nullptr; + tempLeafName.Truncate(wantedFileNameLength); // Base64 characters are alphanumeric (a-zA-Z0-9) and '+' and '/', so we need // to replace illegal characters -- notably '/' From 80f0ffa28b83c5f6ca310178886f6cb491651696 Mon Sep 17 00:00:00 2001 From: Amod Narvekar Date: Sat, 1 Dec 2012 21:20:44 -0500 Subject: [PATCH 03/12] Bug 781346 - Expose local profie directory to OS.Constants.Path. r=yoric --- dom/system/OSFileConstants.cpp | 6 ++++++ .../components/osfile/tests/mochi/main_test_osfile_async.js | 1 + 2 files changed, 7 insertions(+) diff --git a/dom/system/OSFileConstants.cpp b/dom/system/OSFileConstants.cpp index 41bf8b6007e3..75a41dc05e4a 100644 --- a/dom/system/OSFileConstants.cpp +++ b/dom/system/OSFileConstants.cpp @@ -64,6 +64,7 @@ typedef struct { nsString libDir; nsString tmpDir; nsString profileDir; + nsString localProfileDir; } Paths; /** @@ -133,6 +134,7 @@ nsresult InitOSFileConstants() GetPathToSpecialDir(NS_OS_TEMP_DIR, paths->tmpDir); GetPathToSpecialDir(NS_APP_USER_PROFILE_50_DIR, paths->profileDir); + GetPathToSpecialDir(NS_APP_USER_PROFILE_LOCAL_50_DIR, paths->localProfileDir); gPaths = paths.forget(); return NS_OK; @@ -676,6 +678,10 @@ bool DefineOSFileConstants(JSContext *cx, JSObject *global) return false; } + if (!SetStringProperty(cx, objPath, "localProfileDir", gPaths->localProfileDir)) { + return false; + } + return true; } diff --git a/toolkit/components/osfile/tests/mochi/main_test_osfile_async.js b/toolkit/components/osfile/tests/mochi/main_test_osfile_async.js index 480486dbe814..24aa1cd61037 100644 --- a/toolkit/components/osfile/tests/mochi/main_test_osfile_async.js +++ b/toolkit/components/osfile/tests/mochi/main_test_osfile_async.js @@ -170,6 +170,7 @@ let test_path = maketest("path", function path(test) { test.ok(OS.Constants.Path, "OS.Constants.Path exists"); test.is(OS.Constants.Path.tmpDir, Services.dirsvc.get("TmpD", Components.interfaces.nsIFile).path, "OS.Constants.Path.tmpDir is correct"); test.is(OS.Constants.Path.profileDir, Services.dirsvc.get("ProfD", Components.interfaces.nsIFile).path, "OS.Constants.Path.profileDir is correct"); + test.is(OS.Constants.Path.localProfileDir, Services.dirsvc.get("ProfLD", Components.interfaces.nsIFile).path, "OS.Constants.Path.localProfileDir is correct"); }); }); From 81aa9b4c66260ecbed3e96773f7ff98d99a34f5d Mon Sep 17 00:00:00 2001 From: Tim Taubert Date: Fri, 30 Nov 2012 12:56:53 +0100 Subject: [PATCH 04/12] Bug 811953 - [keyboard] Submitting a form from a text field with the keyboard up persists the keyboard. r=vingtetun --- b2g/chrome/content/forms.js | 17 +++-------------- 1 file changed, 3 insertions(+), 14 deletions(-) diff --git a/b2g/chrome/content/forms.js b/b2g/chrome/content/forms.js index 8506deb6883b..d4b97e7dbd36 100644 --- a/b2g/chrome/content/forms.js +++ b/b2g/chrome/content/forms.js @@ -23,7 +23,6 @@ XPCOMUtils.defineLazyGetter(this, "domWindowUtils", function () { .getInterface(Ci.nsIDOMWindowUtils); }); -const FOCUS_CHANGE_DELAY = 20; const RESIZE_SCROLL_DELAY = 20; let HTMLInputElement = Ci.nsIDOMHTMLInputElement; @@ -51,7 +50,6 @@ let FormAssistant = { isKeyboardOpened: false, selectionStart: 0, selectionEnd: 0, - blurTimeout: null, scrollIntoViewTimeout: null, _focusedElement: null, @@ -95,22 +93,13 @@ let FormAssistant = { if (this.isTextInputElement(target) && this.isIMEDisabled()) return; - if (target && this.isFocusableElement(target)) { - if (this.blurTimeout) { - this.blurTimeout = content.clearTimeout(this.blurTimeout); - this.handleIMEStateDisabled(); - } + if (target && this.isFocusableElement(target)) this.handleIMEStateEnabled(target); - } break; case "blur": - if (this.focusedElement) { - this.blurTimeout = content.setTimeout(function () { - this.blurTimeout = null; - this.handleIMEStateDisabled(); - }.bind(this), FOCUS_CHANGE_DELAY); - } + if (this.focusedElement) + this.handleIMEStateDisabled(); break; case 'mousedown': From 4704960aa6df6266230fd19d0325997e3274b81c Mon Sep 17 00:00:00 2001 From: Doug Turner Date: Sat, 1 Dec 2012 21:21:56 -0500 Subject: [PATCH 05/12] Bug 813758 - Ensure permission for geolocation is tested in the parent process. r=bent --- dom/ipc/ContentParent.cpp | 25 ++++++++++++++++++------- 1 file changed, 18 insertions(+), 7 deletions(-) diff --git a/dom/ipc/ContentParent.cpp b/dom/ipc/ContentParent.cpp index 18227991c84e..85c61bc7b6d2 100644 --- a/dom/ipc/ContentParent.cpp +++ b/dom/ipc/ContentParent.cpp @@ -1833,6 +1833,9 @@ ContentParent::RecvAsyncMessage(const nsString& aMsg, bool ContentParent::RecvAddGeolocationListener() { + if (!AssertAppProcessPermission(this, "geolocation")) { + return false; + } if (mGeolocationWatchID == -1) { nsCOMPtr geo = do_GetService("@mozilla.org/geolocation;1"); if (!geo) { @@ -1847,20 +1850,28 @@ ContentParent::RecvAddGeolocationListener() bool ContentParent::RecvRemoveGeolocationListener() { - if (mGeolocationWatchID != -1) { - nsCOMPtr geo = do_GetService("@mozilla.org/geolocation;1"); - if (!geo) { - return true; - } - geo->ClearWatch(mGeolocationWatchID); - mGeolocationWatchID = -1; + if (mGeolocationWatchID == -1) { + return true; } + + if (!AssertAppProcessPermission(this, "geolocation")) { + return false; + } + nsCOMPtr geo = do_GetService("@mozilla.org/geolocation;1"); + if (!geo) { + return true; + } + geo->ClearWatch(mGeolocationWatchID); + mGeolocationWatchID = -1; return true; } NS_IMETHODIMP ContentParent::HandleEvent(nsIDOMGeoPosition* postion) { + if (!AssertAppProcessPermission(this, "geolocation")) { + return NS_ERROR_FAILURE; + } unused << SendGeolocationUpdate(GeoPosition(postion)); return NS_OK; } From 73cc1fe712b89ccc6b5b8bcacd3272aba6dfa29b Mon Sep 17 00:00:00 2001 From: Doug Turner Date: Sat, 1 Dec 2012 21:22:30 -0500 Subject: [PATCH 06/12] Bug 816335 - Remove Network Geolocation Provider from B2G. r=cjones, a=blocking-basecamp --- b2g/installer/package-manifest.in | 4 ---- 1 file changed, 4 deletions(-) diff --git a/b2g/installer/package-manifest.in b/b2g/installer/package-manifest.in index dad20310e996..be7fb19d6fc0 100644 --- a/b2g/installer/package-manifest.in +++ b/b2g/installer/package-manifest.in @@ -377,10 +377,6 @@ @BINPATH@/components/nsHelperAppDlg.js @BINPATH@/components/nsDownloadManagerUI.manifest @BINPATH@/components/nsDownloadManagerUI.js -@BINPATH@/components/NetworkGeolocationProvider.manifest -@BINPATH@/components/NetworkGeolocationProvider.js -@BINPATH@/components/GPSDGeolocationProvider.manifest -@BINPATH@/components/GPSDGeolocationProvider.js @BINPATH@/components/nsSidebar.manifest @BINPATH@/components/nsSidebar.js #ifndef MOZ_WIDGET_GONK From ede8ebce3eec4b25c3e69e8cb9405526e06ac3b9 Mon Sep 17 00:00:00 2001 From: Masatoshi Kimura Date: Sat, 1 Dec 2012 21:23:26 -0500 Subject: [PATCH 07/12] Bug 816487 - Allow all ASCII characters for WebIDL enum. r=bz --- dom/bindings/Codegen.py | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/dom/bindings/Codegen.py b/dom/bindings/Codegen.py index 698fdbd34b45..0cdac91cdcb8 100644 --- a/dom/bindings/Codegen.py +++ b/dom/bindings/Codegen.py @@ -4452,15 +4452,19 @@ def getEnumValueName(value): # characters in them. Deal with the former by returning "_empty", # deal with possible name collisions from that by throwing if the # enum value is actually "_empty", and throw on any value - # containing chars other than [a-z] or '-' for now. Replace '-' with '_'. - value = value.replace('-', '_') + # containing non-ASCII chars for now. Replace all chars other than + # [0-9A-Za-z_] with '_'. + if re.match("[^\x20-\x7E]", value): + raise SyntaxError('Enum value "' + value + '" contains non-ASCII characters') + if re.match("^[0-9]", value): + raise SyntaxError('Enum value "' + value + '" starts with a digit') + value = re.sub(r'[^0-9A-Za-z_]', '_', value) + if re.match("^_[A-Z]|__", value): + raise SyntaxError('Enum value "' + value + '" is reserved by the C++ spec') if value == "_empty": raise SyntaxError('"_empty" is not an IDL enum value we support yet') if value == "": return "_empty" - if not re.match("^[a-z_]+$", value): - raise SyntaxError('Enum value "' + value + '" contains characters ' - 'outside [a-z_]') return MakeNativeName(value) class CGEnum(CGThing): From b38e6e22a7e61b43668c0d159f1a6bbe48dd1539 Mon Sep 17 00:00:00 2001 From: Ralph Giles Date: Fri, 30 Nov 2012 15:02:23 -0800 Subject: [PATCH 08/12] Bug 817179 - Fix an issue with Opus padding larger than 16. r=tterribe Packets with with more than 2^24 padding length bytes could overflow the packet length calculation. This change avoids the wrap around behaviour. --- media/libopus/README_MOZILLA | 2 +- media/libopus/padding.patch | 40 ++++++++++++++++++++++++++++++++ media/libopus/src/opus_decoder.c | 4 +--- media/libopus/update.sh | 3 ++- 4 files changed, 44 insertions(+), 5 deletions(-) create mode 100644 media/libopus/padding.patch diff --git a/media/libopus/README_MOZILLA b/media/libopus/README_MOZILLA index 2c0518e7ce28..148e726df73e 100644 --- a/media/libopus/README_MOZILLA +++ b/media/libopus/README_MOZILLA @@ -8,4 +8,4 @@ files after the copy step. The upstream repository is https://git.xiph.org/opus.git -The git tag/revision used was 1.0.0. +The git tag/revision used was v1.0.0. diff --git a/media/libopus/padding.patch b/media/libopus/padding.patch new file mode 100644 index 000000000000..cf32214ff444 --- /dev/null +++ b/media/libopus/padding.patch @@ -0,0 +1,40 @@ +From 9345aaa5ca1c2fb7d62981b2a538e0ce20612c38 Mon Sep 17 00:00:00 2001 +From: Jean-Marc Valin +Date: Fri, 30 Nov 2012 17:36:36 -0500 +Subject: [PATCH] Fixes an out-of-bounds read issue with the padding handling + code + +This was reported by Juri Aedla and is limited to reading memory up +to about 60 kB beyond the compressed buffer. This can only be triggered +by a compressed packet more than about 16 MB long, so it's not a problem +for RTP. In theory, it *could* crash an Ogg decoder if the memory just after +the incoming packet is out-of-range. +--- + src/opus_decoder.c | 4 +--- + 1 file changed, 1 insertion(+), 3 deletions(-) + +diff --git a/src/opus_decoder.c b/src/opus_decoder.c +index 167e4e4..0be6730 100644 +--- a/src/opus_decoder.c ++++ b/src/opus_decoder.c +@@ -641,16 +641,14 @@ static int opus_packet_parse_impl(const unsigned char *data, opus_int32 len, + /* Padding flag is bit 6 */ + if (ch&0x40) + { +- int padding=0; + int p; + do { + if (len<=0) + return OPUS_INVALID_PACKET; + p = *data++; + len--; +- padding += p==255 ? 254: p; ++ len -= p==255 ? 254: p; + } while (p==255); +- len -= padding; + } + if (len<0) + return OPUS_INVALID_PACKET; +-- +1.7.11.7 + diff --git a/media/libopus/src/opus_decoder.c b/media/libopus/src/opus_decoder.c index 0cc56f84d130..8a30fbcec9d9 100644 --- a/media/libopus/src/opus_decoder.c +++ b/media/libopus/src/opus_decoder.c @@ -595,16 +595,14 @@ static int opus_packet_parse_impl(const unsigned char *data, int len, /* Padding flag is bit 6 */ if (ch&0x40) { - int padding=0; int p; do { if (len<=0) return OPUS_INVALID_PACKET; p = *data++; len--; - padding += p==255 ? 254: p; + len -= p==255 ? 254: p; } while (p==255); - len -= padding; } if (len<0) return OPUS_INVALID_PACKET; diff --git a/media/libopus/update.sh b/media/libopus/update.sh index f02fd0b49320..a9ff6187b6e0 100644 --- a/media/libopus/update.sh +++ b/media/libopus/update.sh @@ -63,4 +63,5 @@ sed -e "s/^The git tag\/revision used was .*/The git tag\/revision used was ${ve mv ${TARGET}/README_MOZILLA+ ${TARGET}/README_MOZILLA # apply outstanding local patches -patch -p3 < ./bug776661.patch +patch -p3 < bug776661.patch +patch -p1 < padding.patch From 6187babbacd2ec389b440da1d7d4e4a9d488b5fa Mon Sep 17 00:00:00 2001 From: Jan Beich Date: Sat, 1 Dec 2012 21:25:24 -0500 Subject: [PATCH 09/12] Bug 817267 - dirfd() is a macro on some BSDs, fix build there. r=yoric --- dom/system/OSFileConstants.cpp | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/dom/system/OSFileConstants.cpp b/dom/system/OSFileConstants.cpp index 75a41dc05e4a..63fc6b1864b2 100644 --- a/dom/system/OSFileConstants.cpp +++ b/dom/system/OSFileConstants.cpp @@ -41,6 +41,11 @@ #include "OSFileConstants.h" #include "nsIOSFileConstantsService.h" +#if defined(__DragonFly__) || defined(__FreeBSD__) \ + || defined(__NetBSD__) || defined(__OpenBSD__) +#define __dd_fd dd_fd +#endif + /** * This module defines the basic libc constants (error numbers, open modes, * etc.) used by OS.File and possibly other OS-bound JavaScript libraries. @@ -383,8 +388,8 @@ static dom::ConstantSpec gLibcProperties[] = { "OSFILE_OFFSETOF_DIRENT_D_TYPE", INT_TO_JSVAL(offsetof (struct dirent, d_type)) }, #endif // defined(DT_UNKNOWN) - // Under MacOS X, |dirfd| is a macro rather than a function, so we - // need a little help to get it to work + // Under MacOS X and BSDs, |dirfd| is a macro rather than a + // function, so we need a little help to get it to work #if defined(dirfd) { "OSFILE_SIZEOF_DIR", INT_TO_JSVAL(sizeof (DIR)) }, From 96292fe7f4be177916f1eb339b8c7289cc2f9ad5 Mon Sep 17 00:00:00 2001 From: Jim Blandy Date: Sat, 1 Dec 2012 21:25:16 -0800 Subject: [PATCH 10/12] Bug 775789: Install .gdbinit correctly in $(DIST)/bin. r=glandium --HG-- rename : .gdbinit => build/.gdbinit --- Makefile.in | 4 ---- .gdbinit => build/.gdbinit | 6 ++++++ build/Makefile.in | 6 ++++++ 3 files changed, 12 insertions(+), 4 deletions(-) rename .gdbinit => build/.gdbinit (95%) diff --git a/Makefile.in b/Makefile.in index 804ef012dbda..9a1c03ea299d 100644 --- a/Makefile.in +++ b/Makefile.in @@ -212,10 +212,6 @@ maybe_clobber_profiledbuild: find $(DIST)/$(MOZ_APP_NAME) -name "*.pgc" -exec mv {} $(DIST)/bin \; endif -# put in our default gdbinit so that the gdb debugging experience is happier. -libs:: .gdbinit - $(INSTALL) $< $(DIST)/bin - .PHONY: maybe_clobber_profiledbuild # Look for R_386_PC32 relocations in shared libs, these diff --git a/.gdbinit b/build/.gdbinit similarity index 95% rename from .gdbinit rename to build/.gdbinit index 51d043b48607..3f33205530b4 100644 --- a/.gdbinit +++ b/build/.gdbinit @@ -1,5 +1,11 @@ # .gdbinit file for debugging Mozilla +# You may need to put an 'add-auto-load-safe-path' command in your +# $HOME/.gdbinit file to get GDB to trust this file. If your builds are +# generally in $HOME/moz, then you can say: +# +# add-auto-load-safe-path ~/moz + # Don't stop for the SIG32/33/etc signals that Flash produces handle SIG32 noprint nostop pass handle SIG33 noprint nostop pass diff --git a/build/Makefile.in b/build/Makefile.in index c918613fbb24..d77aef3d5fc2 100644 --- a/build/Makefile.in +++ b/build/Makefile.in @@ -97,6 +97,12 @@ endif endif +# Put a useful .gdbinit in the bin directory, to be picked up automatically +# by GDB when we debug executables there. +GDBINIT_FILES := .gdbinit +GDBINIT_DEST = $(FINAL_TARGET) +INSTALL_TARGETS += GDBINIT + include $(topsrcdir)/config/rules.mk # we install to _leaktest/ From a634905074b8244f9bc9f9a3223074829af0a32d Mon Sep 17 00:00:00 2001 From: Crypt Date: Mon, 26 Nov 2012 02:36:43 -0800 Subject: [PATCH 11/12] Bug 810220 - Patch to fix SDP Codec Negotiation Issues r=ekr,jesup --- .../src/sipcc/core/common/vcm_util.c | 69 +++++++ .../signaling/src/sipcc/core/gsm/gsm_sdp.c | 194 ++++++++++++------ .../signaling/src/sipcc/core/gsm/h/fsm.h | 22 +- .../webrtc/signaling/src/sipcc/core/gsm/lsm.c | 45 ++-- .../src/sipcc/core/includes/vcm_util.h | 1 + .../signaling/test/signaling_unittests.cpp | 41 +++- 6 files changed, 277 insertions(+), 95 deletions(-) diff --git a/media/webrtc/signaling/src/sipcc/core/common/vcm_util.c b/media/webrtc/signaling/src/sipcc/core/common/vcm_util.c index f4daf87ce8ce..1a2c29fc1af7 100644 --- a/media/webrtc/signaling/src/sipcc/core/common/vcm_util.c +++ b/media/webrtc/signaling/src/sipcc/core/common/vcm_util.c @@ -82,3 +82,72 @@ vcm_media_payload_type_t vcmRtpToMediaPayload (int32_t ptype, } return type; } + +#define EXTRACT_DYNAMIC_PAYLOAD_TYPE(PTYPE) ((PTYPE)>>16) +#define CLEAR_DYNAMIC_PAYLOAD_TYPE(PTYPE) (PTYPE & 0x0000FFFF) +#define CHECK_DYNAMIC_PAYLOAD_TYPE(PTYPE) (PTYPE & 0xFFFF0000) +/* TODO -- This function is temporary, to deal with the fact + that we currently have two different codec representations. + It should be removed when the media types are unified. */ +int32_t mediaPayloadToVcmRtp (vcm_media_payload_type_t payload_in) +{ + int vcmPayload = -1; + int rtp_payload = -1; + + if (CHECK_DYNAMIC_PAYLOAD_TYPE(payload_in)) { + vcmPayload = CLEAR_DYNAMIC_PAYLOAD_TYPE(payload_in); + } else { + //static payload type + vcmPayload = payload_in; + } + + switch(vcmPayload) { + case VCM_Media_Payload_G711Ulaw64k: + rtp_payload = RTP_PCMU; + break; + + case VCM_Media_Payload_G729: + rtp_payload = RTP_G729; + break; + + case VCM_Media_Payload_Wide_Band_256k: + rtp_payload = RTP_L16; + break; + + case VCM_Media_Payload_G722_64k: + rtp_payload = RTP_G722; + break; + + case VCM_Media_Payload_ILBC20: + case VCM_Media_Payload_ILBC30: + rtp_payload = RTP_ILBC; + break; + + case VCM_Media_Payload_ISAC: + rtp_payload = RTP_ISAC; + break; + + case VCM_Media_Payload_H263: + rtp_payload = RTP_H263; + break; + + case VCM_Media_Payload_H264: + rtp_payload = RTP_H264_P0; + break; + + case VCM_Media_Payload_VP8: + rtp_payload = RTP_VP8; + break; + + case VCM_Media_Payload_OPUS: + rtp_payload = RTP_OPUS; + break; + + default: + rtp_payload = RTP_NONE; + break; + } + + return rtp_payload; + +} diff --git a/media/webrtc/signaling/src/sipcc/core/gsm/gsm_sdp.c b/media/webrtc/signaling/src/sipcc/core/gsm/gsm_sdp.c index a2fa101c5a53..9eba893268df 100644 --- a/media/webrtc/signaling/src/sipcc/core/gsm/gsm_sdp.c +++ b/media/webrtc/signaling/src/sipcc/core/gsm/gsm_sdp.c @@ -24,6 +24,7 @@ #include "prlog.h" #include "plstr.h" #include "sdp_private.h" +#include "vcm_util.h" //TODO Need to place this in a portable location #define MULTICAST_START_ADDRESS 0xe1000000 @@ -348,6 +349,8 @@ gsmsdp_free_media (fsmdef_media_t *media) if(media->payloads != NULL) { cpr_free(media->payloads); + cpr_free(media->local_dpt_list); + cpr_free(media->remote_dpt_list); media->num_payloads = 0; } /* @@ -388,7 +391,6 @@ gsmsdp_init_media (fsmdef_media_t *media) media->mode = (uint16_t)vcmGetILBCMode(); media->vad = VCM_VAD_OFF; /* Default to audio codec */ - media->payload = RTP_NONE; media->level = 0; media->dest_port = 0; media->dest_addr = ip_addr_invalid; @@ -414,11 +416,14 @@ gsmsdp_init_media (fsmdef_media_t *media) media->previous_sdp.direction = SDP_DIRECTION_INACTIVE; media->previous_sdp.packetization_period = media->packetization_period; media->previous_sdp.max_packetization_period = media->max_packetization_period; - media->previous_sdp.payload_type = media->payload; - media->previous_sdp.local_payload_type = media->payload; + media->previous_sdp.payload_type = + media->num_payloads ? media->payloads[0] : RTP_NONE; + media->previous_sdp.local_payload_type = + media->num_payloads ? media->payloads[0] : RTP_NONE; media->previous_sdp.tias_bw = SDP_INVALID_VALUE; media->previous_sdp.profile_level = 0; - media->local_dynamic_payload_type_value = media->payload; + media->local_dynamic_payload_type_value = + media->num_payloads ? media->payloads[0] : RTP_NONE; media->hold = FSM_HOLD_NONE; media->flags = 0; /* clear all flags */ media->cap_index = CC_MAX_MEDIA_CAP; /* max is invalid value */ @@ -427,6 +432,8 @@ gsmsdp_init_media (fsmdef_media_t *media) media->rtcp_mux = FALSE; media->protocol = NULL; media->payloads = NULL; + media->local_dpt_list = NULL; + media->remote_dpt_list = NULL; media->num_payloads = 0; } @@ -1661,7 +1668,7 @@ gsmsdp_set_local_sdp_direction (fsmdef_dcb_t *dcb_p, if (media->direction_set) { media->previous_sdp.direction = media->direction; gsmsdp_remove_sdp_direction(media, media->direction, - dcb_p->sdp ? dcb_p->sdp->src_sdp: NULL ); + dcb_p->sdp ? dcb_p->sdp->src_sdp : NULL ); media->direction_set = FALSE; } gsmsdp_set_sdp_direction(media, direction, dcb_p->sdp ? dcb_p->sdp->src_sdp : NULL); @@ -1926,18 +1933,30 @@ gsmsdp_add_default_audio_formats_to_local_sdp (fsmdef_dcb_t *dcb_p, (rtp_ptype *) local_media_types, CC_MAX_MEDIA_TYPES); /* - * If the media payload type is set to RTP_NONE, its because we are making an + * If there are no media payload types, it's because we are making an * initial offer. We will be opening our receive port so we need to specify * the media payload type to be used initially. We set the media payload * type in the dcb to do this. Until we receive an answer from the far * end, we will use our first choice payload type. i.e. the first payload * type sent in our AUDIO media line. */ - if (dcb_p && media && media->payload == RTP_NONE) { - media->payload = local_media_types[0]; + + /* TODO -- Eventually, the structure in previous_sdp needs to be + updated to match the payloads[] array so that we can act on + a codec other than the first one changing. */ + if (dcb_p && media && media->num_payloads == 0) { + if (!media->payloads) { + media->payloads = cpr_calloc(1, sizeof(vcm_media_payload_type_t)); + media->local_dpt_list = cpr_calloc(1, sizeof(uint8_t)); + media->remote_dpt_list = cpr_calloc(1, sizeof(uint8_t)); + } + media->payloads[0] = local_media_types[0]; media->previous_sdp.payload_type = local_media_types[0]; media->previous_sdp.local_payload_type = local_media_types[0]; + media->num_payloads = 1; } + + /* reset the local dynamic payload to NONE */ if (media) { media->local_dynamic_payload_type_value = RTP_NONE; @@ -2061,10 +2080,22 @@ gsmsdp_add_default_video_formats_to_local_sdp (fsmdef_dcb_t *dcb_p, * end, we will use our first choice payload type. i.e. the first payload * type sent in our video media line. */ - if (dcb_p && media && media->payload == RTP_NONE) { - media->payload = video_media_types[0]; + + /* TODO -- Eventually, the structure in previous_sdp needs to be + updated to match the payloads[] array so that we can act on + a codec other than the first one changing. */ + + if (dcb_p && media && media->num_payloads == 0) { + if (!media->payloads) { + media->payloads = (vcm_media_payload_type_t*) + cpr_calloc(1, sizeof(vcm_media_payload_type_t)); + media->local_dpt_list = cpr_calloc(1, sizeof(uint8_t)); + media->remote_dpt_list = cpr_calloc(1, sizeof(uint8_t)); + } + media->payloads[0] = video_media_types[0]; media->previous_sdp.payload_type = video_media_types[0]; media->previous_sdp.local_payload_type = video_media_types[0]; + media->num_payloads = 1; } /* reset the local dynamic payload to NONE */ if (media) { @@ -2241,6 +2272,7 @@ gsmsdp_update_local_sdp_media (fsmdef_dcb_t *dcb_p, cc_sdp_t *cc_sdp_p, uint16_t level; void *sdp_p; int sdpmode = 0; + int i = 0; if (!dcb_p || !media) { GSM_ERR_MSG(get_debug_string(FSMDEF_DBG_INVALID_DCB), fname); @@ -2317,42 +2349,52 @@ gsmsdp_update_local_sdp_media (fsmdef_dcb_t *dcb_p, cc_sdp_t *cc_sdp_p, break; } } else { - /* - * add the single negotiated media format - * answer with the same dynamic payload type value, if a dynamic payload is choosen. + * Add negotiated codec list to the sdp */ - if (media->remote_dynamic_payload_type_value > 0) { - dynamic_payload_type = media->remote_dynamic_payload_type_value; - } else { - dynamic_payload_type = media->payload; - } - result = + for(i=0; i < media->num_payloads; i++) { + // Get the PT value. If the remote party has + // specified a dynamic value for this payload, + // we re-use the same value they do. Otherwise, + // we use our own selected dynamic payload type. + if (media->remote_dpt_list[i] > 0) { + dynamic_payload_type = media->remote_dpt_list[i]; + } else { + dynamic_payload_type = media->local_dpt_list[i]; + } + + result = sdp_add_media_payload_type(sdp_p, level, (uint16_t)dynamic_payload_type, SDP_PAYLOAD_NUMERIC); - if (result != SDP_SUCCESS) { + + if (result != SDP_SUCCESS) { GSM_ERR_MSG(GSM_L_C_F_PREFIX"Adding dynamic payload type failed\n", dcb_p->line, dcb_p->call_id, fname); - } - switch (media->type) { - case SDP_MEDIA_AUDIO: - gsmsdp_set_media_attributes(media->payload, sdp_p, level, - (uint16_t)dynamic_payload_type); - break; - case SDP_MEDIA_VIDEO: - gsmsdp_set_video_media_attributes(media->payload, cc_sdp_p, level, - (uint16_t)dynamic_payload_type); - break; - case SDP_MEDIA_APPLICATION: - gsmsdp_set_sctp_attributes (sdp_p, level, media); - break; - default: - GSM_ERR_MSG(GSM_L_C_F_PREFIX"SDP ERROR media %d for level %d is not" - " supported\n", + } + + switch (media->type) { + case SDP_MEDIA_AUDIO: + gsmsdp_set_media_attributes(mediaPayloadToVcmRtp( + media->payloads[i]), sdp_p, level, + (uint16_t)dynamic_payload_type); + break; + case SDP_MEDIA_VIDEO: + gsmsdp_set_video_media_attributes(mediaPayloadToVcmRtp( + media->payloads[i]), cc_sdp_p, level, + (uint16_t)dynamic_payload_type); + break; + case SDP_MEDIA_APPLICATION: + gsmsdp_set_sctp_attributes (sdp_p, level, media); + break; + default: + GSM_ERR_MSG(GSM_L_C_F_PREFIX"SDP ERROR media %d for level %d is" + " not supported\n", dcb_p->line, dcb_p->call_id, fname, media->level); - break; - } + break; + } + + }//end for /* * add the avt media type @@ -2808,7 +2850,8 @@ gsmsdp_negotiate_codec (fsmdef_dcb_t *dcb_p, cc_sdp_t *sdp_p, /* * Save pay load type for use by gsmspd_compare_to_previous_sdp */ - media->previous_sdp.payload_type = media->payload; + media->previous_sdp.payload_type = + media->num_payloads ? media->payloads[0] : RTP_NONE; media->previous_sdp.local_payload_type = media->local_dynamic_payload_type_value; /* @@ -2821,13 +2864,19 @@ gsmsdp_negotiate_codec (fsmdef_dcb_t *dcb_p, cc_sdp_t *sdp_p, payload_types_count = num_slave_types; } - media->payloads = (vcm_media_payload_type_t*) cpr_malloc(payload_types_count * sizeof(vcm_media_payload_type_t)); - if (!(media->payloads)) - { - GSM_ERR_MSG(GSM_L_C_F_PREFIX"Memory Allocation failed for payloads\n", - DEB_L_C_F_PREFIX_ARGS(GSM, dcb_p->line, dcb_p->call_id, fname)); - return RTP_NONE; + /* Remove any previously allocated lists */ + if (media->payloads) { + cpr_free(media->payloads); + cpr_free(media->local_dpt_list); + cpr_free(media->remote_dpt_list); } + /* Allocate memory for PT value, local PT and remote PT. */ + media->payloads = cpr_calloc(payload_types_count, + sizeof(vcm_media_payload_type_t)); + media->local_dpt_list = cpr_calloc(payload_types_count, + sizeof(uint8_t)); + media->remote_dpt_list = cpr_calloc(payload_types_count, + sizeof(uint8_t)); for (i = 0; i < num_master_types; i++) { for (j = 0; j < num_slave_types; j++) { @@ -2842,7 +2891,7 @@ gsmsdp_negotiate_codec (fsmdef_dcb_t *dcb_p, cc_sdp_t *sdp_p, } } else { //if remote SDP is an answer if (media->local_dynamic_payload_type_value == RTP_NONE || - media->payload != media->previous_sdp.payload_type) { + !media->num_payloads || media->payloads[0] != media->previous_sdp.payload_type) { /* If the the negotiated payload type is different from previous, set it the local dynamic to payload type as this is what we offered*/ } @@ -2858,7 +2907,7 @@ gsmsdp_negotiate_codec (fsmdef_dcb_t *dcb_p, cc_sdp_t *sdp_p, if (payload == RTP_ILBC) { media->mode = (uint16_t)sdp_attr_get_fmtp_mode_for_payload_type (sdp_p->dest_sdp, level, 0, - media->remote_dynamic_payload_type_value); + remote_dynamic_payload_type_value); } if (payload == RTP_OPUS) { u16 a_inst; @@ -2908,14 +2957,13 @@ gsmsdp_negotiate_codec (fsmdef_dcb_t *dcb_p, cc_sdp_t *sdp_p, vcmFreeMediaPtr(media->video); media->video = NULL; } - - if ( vcmCheckAttribs(media->payload, sdp_p, level, - &media->video) == FALSE ) { - GSM_DEBUG(DEB_L_C_F_PREFIX"codec= %d ignored - attribs not accepted\n", - DEB_L_C_F_PREFIX_ARGS(GSM, dcb_p->line, - dcb_p->call_id, fname), media->payload); - explicit_reject = TRUE; - continue; // keep looking + if ( vcmCheckAttribs(payload, sdp_p, level, + &media->video) == FALSE ) { + GSM_DEBUG(DEB_L_C_F_PREFIX"codec= %d ignored - attribs not accepted\n", + DEB_L_C_F_PREFIX_ARGS(GSM, dcb_p->line, + dcb_p->call_id, fname), payload); + explicit_reject = TRUE; + continue; // keep looking } // cache the negotiated profile_level and bandwidth @@ -2929,7 +2977,7 @@ gsmsdp_negotiate_codec (fsmdef_dcb_t *dcb_p, cc_sdp_t *sdp_p, GSM_DEBUG(DEB_L_C_F_PREFIX"codec= %d\n", DEB_L_C_F_PREFIX_ARGS(GSM, dcb_p->line, - dcb_p->call_id, fname), media->payload); + dcb_p->call_id, fname), payload); } found_codec = TRUE; @@ -2937,12 +2985,17 @@ gsmsdp_negotiate_codec (fsmdef_dcb_t *dcb_p, cc_sdp_t *sdp_p, //we maxed our allocated memory. ignore and return; return payload; } + //update media structure with the negotiated value media->payloads[media->num_payloads] = vcmRtpToMediaPayload(payload, remote_dynamic_payload_type_value, media->mode); + media->local_dpt_list[media->num_payloads] = payload; + media->remote_dpt_list[media->num_payloads] = remote_dynamic_payload_type_value; media->num_payloads++; if(offer) { //return on the first match + // TODO -- Eventually, we'll (probably) want to answer with all + // the codecs we can receive. See bug 814227. return payload; } } @@ -2962,20 +3015,20 @@ gsmsdp_negotiate_codec (fsmdef_dcb_t *dcb_p, cc_sdp_t *sdp_p, */ if (!initial_offer && explicit_reject == FALSE) { for (i = 0; i < num_types; i++) { - if (media->payload == GET_CODEC_TYPE(remote_media_types[i])) { + if (media->num_payloads != 0 && media->payloads[0] == GET_CODEC_TYPE(remote_media_types[i])) { /* * CSCta40560 - DSP runs out of bandwidth as the current API's do not consider the request is for * the same call. Need to update dynamic payload types for dynamic PT codecs. Would need to possibly * add other video codecs as support is added here. */ - if ( (media->payload == RTP_H264_P1 || media->payload == RTP_H264_P0) && offer == TRUE ) { + if ( (media->payloads[0] == RTP_H264_P1 || media->payloads[0] == RTP_H264_P0) && offer == TRUE ) { media->remote_dynamic_payload_type_value = GET_DYN_PAYLOAD_TYPE_VALUE(master_list_p[i]); media->local_dynamic_payload_type_value = GET_DYN_PAYLOAD_TYPE_VALUE(master_list_p[i]); } GSM_DEBUG(DEB_L_C_F_PREFIX"local codec list was empty codec= %d local=%d remote =%d\n", DEB_L_C_F_PREFIX_ARGS(GSM, dcb_p->line, dcb_p->call_id, fname), - media->payload, media->local_dynamic_payload_type_value, media->remote_dynamic_payload_type_value); - return (media->payload); + media->payloads[0], media->local_dynamic_payload_type_value, media->remote_dynamic_payload_type_value); + return (media->payloads[0]); } } } @@ -4141,7 +4194,7 @@ gsmsdp_negotiate_media_lines (fsm_fcb_t *fcb_p, cc_sdp_t *sdp_p, boolean initial GSM_DEBUG(DEB_L_C_F_PREFIX"remote_direction: %d global match %sfound\n", DEB_L_C_F_PREFIX_ARGS(GSM, dcb_p->line, dcb_p->call_id, fname), - remote_direction, (cap_index != CC_MAX_MEDIA_CAP)?"":"not "); + remote_direction, (cap_index != CC_MAX_MEDIA_CAP) ? "" : "not "); if ( cap_index != CC_MAX_MEDIA_CAP && remote_direction != SDP_DIRECTION_INACTIVE ) { // this is an offer and platform can support video @@ -4756,11 +4809,17 @@ gsmsdp_add_media_line (fsmdef_dcb_t *dcb_p, const cc_media_cap_t *media_cap, * Since we are initiating an initial offer and opening a * receive port, store initial media settings. */ + + /* TODO -- Eventually, the structure in previous_sdp needs to be + updated to match the payloads[] array so that we can act on + a codec other than the first one changing. */ media->previous_sdp.avt_payload_type = media->avt_payload_type; media->previous_sdp.direction = media->direction; media->previous_sdp.packetization_period = media->packetization_period; - media->previous_sdp.payload_type = media->payload; - media->previous_sdp.local_payload_type = media->payload; + media->previous_sdp.payload_type = + media->num_payloads ? media->payloads[0] : RTP_NONE; + media->previous_sdp.local_payload_type = + media->num_payloads ? media->payloads[0] : RTP_NONE; break; } @@ -6231,11 +6290,14 @@ gsmsdp_sdp_differs_from_previous_sdp (boolean rcv_only, fsmdef_media_t *media) * Consider attributes of interest common to all active directions. */ if ((media->previous_sdp.avt_payload_type != media->avt_payload_type) || - (media->previous_sdp.payload_type != media->payload)) { + (0 == media->num_payloads) || + (media->previous_sdp.payload_type != media->payloads[0])) { GSM_DEBUG(DEB_F_PREFIX"previous payload: %d new payload: %d\n", - DEB_F_PREFIX_ARGS(GSM, fname), media->previous_sdp.payload_type, media->payload); + DEB_F_PREFIX_ARGS(GSM, fname), + media->previous_sdp.payload_type, media->payloads[0]); GSM_DEBUG(DEB_F_PREFIX"previous avt payload: %d new avt payload: %d\n", - DEB_F_PREFIX_ARGS(GSM, fname), media->previous_sdp.avt_payload_type, + DEB_F_PREFIX_ARGS(GSM, fname), + media->previous_sdp.avt_payload_type, media->avt_payload_type); return TRUE; } diff --git a/media/webrtc/signaling/src/sipcc/core/gsm/h/fsm.h b/media/webrtc/signaling/src/sipcc/core/gsm/h/fsm.h index d3ba16c2a9a8..958f35a488aa 100755 --- a/media/webrtc/signaling/src/sipcc/core/gsm/h/fsm.h +++ b/media/webrtc/signaling/src/sipcc/core/gsm/h/fsm.h @@ -146,7 +146,6 @@ typedef struct fsmdef_media_t_ { * We answer with the same payload type value that the remote offers. * If remote choses to answer with different value than we offer, we support asymmetric. */ - int32_t payload; //payload type - one of rtp_ptype enumerations int32_t local_dynamic_payload_type_value; // dynamic payload type value offered/answered by us int32_t remote_dynamic_payload_type_value; // dynamic payload type value offered/answered by remote int32_t avt_payload_type; @@ -237,11 +236,30 @@ typedef struct fsmdef_media_t_ { uint32 streams; char *protocol; + /* + * This field contains the size of the payloads, + * local_dpt_list, and remote_dpt_list fields. + */ + int32_t num_payloads; + /* * List of active lists of payloads negotiated + * There needs to be 1-1 mapping between the 3 arrays + * TODO:crypt: Move these to a aggregate structure per payload + * since we need to include codec type, local rtp type, remote rty type, + * codec specific parameters as generated by any SDP action. */ vcm_media_payload_type_t* payloads; - int32_t num_payloads; + + /* + * dynamic payload type value offered/answered by us + */ + uint8_t* local_dpt_list; + + /* + * dynamic payload type value offered/answered by remote + */ + uint8_t* remote_dpt_list; } fsmdef_media_t; struct fsm_fcb_t_; diff --git a/media/webrtc/signaling/src/sipcc/core/gsm/lsm.c b/media/webrtc/signaling/src/sipcc/core/gsm/lsm.c index 791ee701e0a7..3ca597f48751 100755 --- a/media/webrtc/signaling/src/sipcc/core/gsm/lsm.c +++ b/media/webrtc/signaling/src/sipcc/core/gsm/lsm.c @@ -995,7 +995,8 @@ lsm_rx_start (lsm_lcb_t *lcb, const char *fname, fsmdef_media_t *media) pc_track_id = 0; dcb->cur_video_avail &= ~CC_ATTRIB_CAST; if (media->local_dynamic_payload_type_value == RTP_NONE) { - media->local_dynamic_payload_type_value = media->payload; + media->local_dynamic_payload_type_value = + media->num_payloads ? media->payloads[0] : RTP_NONE; } config_get_value(CFGID_SDPMODE, &sdpmode, sizeof(sdpmode)); @@ -1014,9 +1015,15 @@ lsm_rx_start (lsm_lcb_t *lcb, const char *fname, fsmdef_media_t *media) } else if (!sdpmode) { ret_val = vcmRxStart(media->cap_index, group_id, media->refid, lsm_get_ms_ui_call_handle(dcb->line, call_id, CC_NO_CALL_ID), - vcmRtpToMediaPayload(media->payload, - media->local_dynamic_payload_type_value, - media->mode), + /* RTP_NONE is not technically a valid + value for vcm_media_payload_type_t. + However, we really should never + reach this part of the code with + num_payloads < 1, so the RTP_NONE + shouldn't ever be used. It's only + here are extra protection against + an invalid pointer deref.*/ + media->num_payloads ? media->payloads[0] : RTP_NONE, media->is_multicast ? &media->dest_addr:&media->src_addr, port, FSM_NEGOTIATED_CRYPTO_ALGORITHM_ID(media), @@ -1235,9 +1242,7 @@ lsm_tx_start (lsm_lcb_t *lcb, const char *fname, fsmdef_media_t *media) if (vcmTxStart(media->cap_index, group_id, media->refid, lsm_get_ms_ui_call_handle(dcb->line, call_id, CC_NO_CALL_ID), - vcmRtpToMediaPayload(media->payload, - media->remote_dynamic_payload_type_value, - media->mode), + media->num_payloads ? media->payloads[0] : RTP_NONE, (short)dscp, &media->src_addr, media->src_port, @@ -1263,9 +1268,7 @@ lsm_tx_start (lsm_lcb_t *lcb, const char *fname, fsmdef_media_t *media) dcb->media_cap_tbl->cap[media->cap_index].pc_track, lsm_get_ms_ui_call_handle(dcb->line, call_id, CC_NO_CALL_ID), dcb->peerconnection, - vcmRtpToMediaPayload(media->payload, - media->remote_dynamic_payload_type_value, - media->mode), + media->num_payloads ? media->payloads[0] : RTP_NONE, (short)dscp, FSM_NEGOTIATED_CRYPTO_DIGEST_ALGORITHM(media), FSM_NEGOTIATED_CRYPTO_DIGEST(media), @@ -1949,7 +1952,7 @@ lsm_get_free_lcb (callid_t call_id, line_t line, fsmdef_dcb_t *dcb) lcb->mru = mru; lcb->dcb = dcb; // start unmuted if txPref is true - lcb->vid_mute = cc_media_getVideoAutoTxPref()?FALSE:TRUE; + lcb->vid_mute = cc_media_getVideoAutoTxPref() ? FALSE : TRUE; lcb->ui_id = call_id; /* default UI ID is the same as call_id */ break; @@ -3857,12 +3860,16 @@ lsm_update_media (lsm_lcb_t *lcb, const char *caller_fname) if (LSMDebug) { /* debug is enabled, format the dest addr into string */ ipaddr2dotted(addr_str, &media->dest_addr); + for (int i = 0; i < media->num_payloads; i++) + { + LSM_DEBUG(DEB_L_C_F_PREFIX"%d rx, tx refresh's are %d %d" + ", dir=%d, payload=%d addr=%s, multicast=%d\n", + DEB_L_C_F_PREFIX_ARGS(LSM, dcb->line, + dcb->call_id, fname), media->refid, rx_refresh, + tx_refresh, media->direction, + media->payloads[i], addr_str, media->is_multicast ); + } } - LSM_DEBUG(DEB_L_C_F_PREFIX"%d rx, tx refresh's are %d %d" - ", dir=%d, payload=%d addr=%s, multicast=%d\n", - DEB_L_C_F_PREFIX_ARGS(LSM, dcb->line, dcb->call_id, fname), - media->refid, rx_refresh, tx_refresh, media->direction, - media->payload, addr_str, media->is_multicast ); if (rx_refresh || (media->is_multicast && media->direction_set && @@ -5289,9 +5296,9 @@ void lsm_add_remote_stream (line_t line, callid_t call_id, fsmdef_media_t *media return; } - vcmCreateRemoteStream(media->cap_index, dcb->peerconnection, pc_stream_id, - vcmRtpToMediaPayload(media->payload, - media->local_dynamic_payload_type_value,media->mode)); + vcmCreateRemoteStream(media->cap_index, dcb->peerconnection, + pc_stream_id, + media->num_payloads ? media->payloads[0] : RTP_NONE); } } diff --git a/media/webrtc/signaling/src/sipcc/core/includes/vcm_util.h b/media/webrtc/signaling/src/sipcc/core/includes/vcm_util.h index e3b11ed89af6..0428df64786d 100644 --- a/media/webrtc/signaling/src/sipcc/core/includes/vcm_util.h +++ b/media/webrtc/signaling/src/sipcc/core/includes/vcm_util.h @@ -19,4 +19,5 @@ vcm_media_payload_type_t vcm_rtp_to_media_payload (int32_t ptype, uint16_t mode); +int32_t mediaPayloadToVcmRtp (vcm_media_payload_type_t payload_in); #endif /* VCM_UTIL_H_ */ diff --git a/media/webrtc/signaling/test/signaling_unittests.cpp b/media/webrtc/signaling/test/signaling_unittests.cpp index e2388a4cb145..d053a184388f 100644 --- a/media/webrtc/signaling/test/signaling_unittests.cpp +++ b/media/webrtc/signaling/test/signaling_unittests.cpp @@ -575,7 +575,8 @@ class SignalingAgent { } void CreateAnswer(sipcc::MediaConstraints& constraints, std::string offer, - uint32_t offerAnswerFlags, uint32_t sdpCheck) { + uint32_t offerAnswerFlags, + uint32_t sdpCheck = DONT_CHECK_AUDIO|DONT_CHECK_VIDEO) { // Create a media stream as if it came from GUM nsRefPtr domMediaStream = new nsDOMMediaStream(); @@ -1217,7 +1218,8 @@ TEST_F(SignalingTest, FullCallTrickle) ASSERT_GE(a2_.GetPacketsReceived(0), 40); } -TEST_F(SignalingTest, Bug810220) +// This test comes from Bug 810220 +TEST_F(SignalingTest, AudioOnlyG711Call) { sipcc::MediaConstraints constraints; std::string offer = @@ -1228,10 +1230,11 @@ TEST_F(SignalingTest, Bug810220) "t=0 0\r\n" "a=fingerprint:sha-256 F3:FA:20:C0:CD:48:C4:5F:02:5F:A5:D3:21:D0:2D:48:" "7B:31:60:5C:5A:D8:0D:CD:78:78:6C:6D:CE:CC:0C:67\r\n" - "m=audio 9000 RTP/AVP 0 126\r\n" + "m=audio 9000 RTP/AVP 0 8 126\r\n" "c=IN IP4 148.147.200.251\r\n" "b=TIAS:64000\r\n" "a=rtpmap:0 PCMU/8000\r\n" + "a=rtpmap:8 PCMA/8000\r\n" "a=rtpmap:126 telephone-event/8000\r\n" "a=candidate:0 1 udp 2130706432 148.147.200.251 9000 typ host\r\n" "a=candidate:0 2 udp 2130706432 148.147.200.251 9005 typ host\r\n" @@ -1247,12 +1250,29 @@ TEST_F(SignalingTest, Bug810220) DONT_CHECK_AUDIO | DONT_CHECK_VIDEO); std::string answer = a2_.answer(); - /* TODO -- make sure the answer looks right */ + + // They didn't offer opus, so our answer shouldn't include it. + ASSERT_EQ(answer.find(" opus/"), std::string::npos); + + // They also didn't offer video or application + ASSERT_EQ(answer.find("video"), std::string::npos); + ASSERT_EQ(answer.find("application"), std::string::npos); + + // We should answer with PCMU and telephone-event + ASSERT_NE(answer.find(" PCMU/8000"), std::string::npos); + ASSERT_NE(answer.find(" telephone-event/8000"), std::string::npos); + + // Double-check the directionality + ASSERT_NE(answer.find("\r\na=sendrecv"), std::string::npos); + } -TEST_F(SignalingTest, Bug814038) +// This test comes from Bug814038 +TEST_F(SignalingTest, ChromeOfferAnswer) { sipcc::MediaConstraints constraints; + + // This is captured SDP from an early interop attempt with Chrome. std::string offer = "v=0\r\n" "o=- 1713781661 2 IN IP4 127.0.0.1\r\n" @@ -1275,7 +1295,12 @@ TEST_F(SignalingTest, Bug814038) "RzrYlzpkTsvgYFD1hQqNCzQ7y4emNLKI1tODsjim\r\n" "a=rtpmap:103 ISAC/16000\r\n" "a=rtpmap:104 ISAC/32000\r\n" - "a=rtpmap:111 opus/48000\r\n" + // NOTE: the actual SDP that Chrome sends at the moment + // doesn't indicate two channels. I've amended their SDP + // here, under the assumption that the constraints + // described in draft-spittka-payload-rtp-opus will + // eventually be implemented by Google. + "a=rtpmap:111 opus/48000/2\r\n" "a=rtpmap:0 PCMU/8000\r\n" "a=rtpmap:8 PCMA/8000\r\n" "a=rtpmap:107 CN/48000\r\n" @@ -1314,13 +1339,13 @@ TEST_F(SignalingTest, Bug814038) a2_.SetRemote(TestObserver::OFFER, offer); std::cout << "Creating answer:" << std::endl; - a2_.CreateAnswer(constraints, offer, OFFER_AUDIO | ANSWER_AUDIO, - SHOULD_INACTIVE_AUDIO | SHOULD_INACTIVE_VIDEO); + a2_.CreateAnswer(constraints, offer, OFFER_AUDIO | ANSWER_AUDIO); std::string answer = a2_.answer(); } + } // End namespace test. int main(int argc, char **argv) { From 62a965cb1275b31a8e2f2cf1739f8e75b417f60b Mon Sep 17 00:00:00 2001 From: Randell Jesup Date: Sun, 2 Dec 2012 02:56:21 -0500 Subject: [PATCH 12/12] Backed out changeset 483a8a9702a6 --- .../src/sipcc/core/common/vcm_util.c | 69 ------ .../signaling/src/sipcc/core/gsm/gsm_sdp.c | 196 ++++++------------ .../signaling/src/sipcc/core/gsm/h/fsm.h | 22 +- .../webrtc/signaling/src/sipcc/core/gsm/lsm.c | 45 ++-- .../src/sipcc/core/includes/vcm_util.h | 1 - .../signaling/test/signaling_unittests.cpp | 41 +--- 6 files changed, 96 insertions(+), 278 deletions(-) diff --git a/media/webrtc/signaling/src/sipcc/core/common/vcm_util.c b/media/webrtc/signaling/src/sipcc/core/common/vcm_util.c index 1a2c29fc1af7..f4daf87ce8ce 100644 --- a/media/webrtc/signaling/src/sipcc/core/common/vcm_util.c +++ b/media/webrtc/signaling/src/sipcc/core/common/vcm_util.c @@ -82,72 +82,3 @@ vcm_media_payload_type_t vcmRtpToMediaPayload (int32_t ptype, } return type; } - -#define EXTRACT_DYNAMIC_PAYLOAD_TYPE(PTYPE) ((PTYPE)>>16) -#define CLEAR_DYNAMIC_PAYLOAD_TYPE(PTYPE) (PTYPE & 0x0000FFFF) -#define CHECK_DYNAMIC_PAYLOAD_TYPE(PTYPE) (PTYPE & 0xFFFF0000) -/* TODO -- This function is temporary, to deal with the fact - that we currently have two different codec representations. - It should be removed when the media types are unified. */ -int32_t mediaPayloadToVcmRtp (vcm_media_payload_type_t payload_in) -{ - int vcmPayload = -1; - int rtp_payload = -1; - - if (CHECK_DYNAMIC_PAYLOAD_TYPE(payload_in)) { - vcmPayload = CLEAR_DYNAMIC_PAYLOAD_TYPE(payload_in); - } else { - //static payload type - vcmPayload = payload_in; - } - - switch(vcmPayload) { - case VCM_Media_Payload_G711Ulaw64k: - rtp_payload = RTP_PCMU; - break; - - case VCM_Media_Payload_G729: - rtp_payload = RTP_G729; - break; - - case VCM_Media_Payload_Wide_Band_256k: - rtp_payload = RTP_L16; - break; - - case VCM_Media_Payload_G722_64k: - rtp_payload = RTP_G722; - break; - - case VCM_Media_Payload_ILBC20: - case VCM_Media_Payload_ILBC30: - rtp_payload = RTP_ILBC; - break; - - case VCM_Media_Payload_ISAC: - rtp_payload = RTP_ISAC; - break; - - case VCM_Media_Payload_H263: - rtp_payload = RTP_H263; - break; - - case VCM_Media_Payload_H264: - rtp_payload = RTP_H264_P0; - break; - - case VCM_Media_Payload_VP8: - rtp_payload = RTP_VP8; - break; - - case VCM_Media_Payload_OPUS: - rtp_payload = RTP_OPUS; - break; - - default: - rtp_payload = RTP_NONE; - break; - } - - return rtp_payload; - -} diff --git a/media/webrtc/signaling/src/sipcc/core/gsm/gsm_sdp.c b/media/webrtc/signaling/src/sipcc/core/gsm/gsm_sdp.c index 9eba893268df..a2fa101c5a53 100644 --- a/media/webrtc/signaling/src/sipcc/core/gsm/gsm_sdp.c +++ b/media/webrtc/signaling/src/sipcc/core/gsm/gsm_sdp.c @@ -24,7 +24,6 @@ #include "prlog.h" #include "plstr.h" #include "sdp_private.h" -#include "vcm_util.h" //TODO Need to place this in a portable location #define MULTICAST_START_ADDRESS 0xe1000000 @@ -349,8 +348,6 @@ gsmsdp_free_media (fsmdef_media_t *media) if(media->payloads != NULL) { cpr_free(media->payloads); - cpr_free(media->local_dpt_list); - cpr_free(media->remote_dpt_list); media->num_payloads = 0; } /* @@ -391,6 +388,7 @@ gsmsdp_init_media (fsmdef_media_t *media) media->mode = (uint16_t)vcmGetILBCMode(); media->vad = VCM_VAD_OFF; /* Default to audio codec */ + media->payload = RTP_NONE; media->level = 0; media->dest_port = 0; media->dest_addr = ip_addr_invalid; @@ -416,14 +414,11 @@ gsmsdp_init_media (fsmdef_media_t *media) media->previous_sdp.direction = SDP_DIRECTION_INACTIVE; media->previous_sdp.packetization_period = media->packetization_period; media->previous_sdp.max_packetization_period = media->max_packetization_period; - media->previous_sdp.payload_type = - media->num_payloads ? media->payloads[0] : RTP_NONE; - media->previous_sdp.local_payload_type = - media->num_payloads ? media->payloads[0] : RTP_NONE; + media->previous_sdp.payload_type = media->payload; + media->previous_sdp.local_payload_type = media->payload; media->previous_sdp.tias_bw = SDP_INVALID_VALUE; media->previous_sdp.profile_level = 0; - media->local_dynamic_payload_type_value = - media->num_payloads ? media->payloads[0] : RTP_NONE; + media->local_dynamic_payload_type_value = media->payload; media->hold = FSM_HOLD_NONE; media->flags = 0; /* clear all flags */ media->cap_index = CC_MAX_MEDIA_CAP; /* max is invalid value */ @@ -432,8 +427,6 @@ gsmsdp_init_media (fsmdef_media_t *media) media->rtcp_mux = FALSE; media->protocol = NULL; media->payloads = NULL; - media->local_dpt_list = NULL; - media->remote_dpt_list = NULL; media->num_payloads = 0; } @@ -1668,7 +1661,7 @@ gsmsdp_set_local_sdp_direction (fsmdef_dcb_t *dcb_p, if (media->direction_set) { media->previous_sdp.direction = media->direction; gsmsdp_remove_sdp_direction(media, media->direction, - dcb_p->sdp ? dcb_p->sdp->src_sdp : NULL ); + dcb_p->sdp ? dcb_p->sdp->src_sdp: NULL ); media->direction_set = FALSE; } gsmsdp_set_sdp_direction(media, direction, dcb_p->sdp ? dcb_p->sdp->src_sdp : NULL); @@ -1933,30 +1926,18 @@ gsmsdp_add_default_audio_formats_to_local_sdp (fsmdef_dcb_t *dcb_p, (rtp_ptype *) local_media_types, CC_MAX_MEDIA_TYPES); /* - * If there are no media payload types, it's because we are making an + * If the media payload type is set to RTP_NONE, its because we are making an * initial offer. We will be opening our receive port so we need to specify * the media payload type to be used initially. We set the media payload * type in the dcb to do this. Until we receive an answer from the far * end, we will use our first choice payload type. i.e. the first payload * type sent in our AUDIO media line. */ - - /* TODO -- Eventually, the structure in previous_sdp needs to be - updated to match the payloads[] array so that we can act on - a codec other than the first one changing. */ - if (dcb_p && media && media->num_payloads == 0) { - if (!media->payloads) { - media->payloads = cpr_calloc(1, sizeof(vcm_media_payload_type_t)); - media->local_dpt_list = cpr_calloc(1, sizeof(uint8_t)); - media->remote_dpt_list = cpr_calloc(1, sizeof(uint8_t)); - } - media->payloads[0] = local_media_types[0]; + if (dcb_p && media && media->payload == RTP_NONE) { + media->payload = local_media_types[0]; media->previous_sdp.payload_type = local_media_types[0]; media->previous_sdp.local_payload_type = local_media_types[0]; - media->num_payloads = 1; } - - /* reset the local dynamic payload to NONE */ if (media) { media->local_dynamic_payload_type_value = RTP_NONE; @@ -2080,22 +2061,10 @@ gsmsdp_add_default_video_formats_to_local_sdp (fsmdef_dcb_t *dcb_p, * end, we will use our first choice payload type. i.e. the first payload * type sent in our video media line. */ - - /* TODO -- Eventually, the structure in previous_sdp needs to be - updated to match the payloads[] array so that we can act on - a codec other than the first one changing. */ - - if (dcb_p && media && media->num_payloads == 0) { - if (!media->payloads) { - media->payloads = (vcm_media_payload_type_t*) - cpr_calloc(1, sizeof(vcm_media_payload_type_t)); - media->local_dpt_list = cpr_calloc(1, sizeof(uint8_t)); - media->remote_dpt_list = cpr_calloc(1, sizeof(uint8_t)); - } - media->payloads[0] = video_media_types[0]; + if (dcb_p && media && media->payload == RTP_NONE) { + media->payload = video_media_types[0]; media->previous_sdp.payload_type = video_media_types[0]; media->previous_sdp.local_payload_type = video_media_types[0]; - media->num_payloads = 1; } /* reset the local dynamic payload to NONE */ if (media) { @@ -2272,7 +2241,6 @@ gsmsdp_update_local_sdp_media (fsmdef_dcb_t *dcb_p, cc_sdp_t *cc_sdp_p, uint16_t level; void *sdp_p; int sdpmode = 0; - int i = 0; if (!dcb_p || !media) { GSM_ERR_MSG(get_debug_string(FSMDEF_DBG_INVALID_DCB), fname); @@ -2349,52 +2317,42 @@ gsmsdp_update_local_sdp_media (fsmdef_dcb_t *dcb_p, cc_sdp_t *cc_sdp_p, break; } } else { - /* - * Add negotiated codec list to the sdp - */ - for(i=0; i < media->num_payloads; i++) { - // Get the PT value. If the remote party has - // specified a dynamic value for this payload, - // we re-use the same value they do. Otherwise, - // we use our own selected dynamic payload type. - if (media->remote_dpt_list[i] > 0) { - dynamic_payload_type = media->remote_dpt_list[i]; - } else { - dynamic_payload_type = media->local_dpt_list[i]; - } - result = + /* + * add the single negotiated media format + * answer with the same dynamic payload type value, if a dynamic payload is choosen. + */ + if (media->remote_dynamic_payload_type_value > 0) { + dynamic_payload_type = media->remote_dynamic_payload_type_value; + } else { + dynamic_payload_type = media->payload; + } + result = sdp_add_media_payload_type(sdp_p, level, (uint16_t)dynamic_payload_type, SDP_PAYLOAD_NUMERIC); - - if (result != SDP_SUCCESS) { + if (result != SDP_SUCCESS) { GSM_ERR_MSG(GSM_L_C_F_PREFIX"Adding dynamic payload type failed\n", dcb_p->line, dcb_p->call_id, fname); - } - - switch (media->type) { - case SDP_MEDIA_AUDIO: - gsmsdp_set_media_attributes(mediaPayloadToVcmRtp( - media->payloads[i]), sdp_p, level, - (uint16_t)dynamic_payload_type); - break; - case SDP_MEDIA_VIDEO: - gsmsdp_set_video_media_attributes(mediaPayloadToVcmRtp( - media->payloads[i]), cc_sdp_p, level, - (uint16_t)dynamic_payload_type); - break; - case SDP_MEDIA_APPLICATION: - gsmsdp_set_sctp_attributes (sdp_p, level, media); - break; - default: - GSM_ERR_MSG(GSM_L_C_F_PREFIX"SDP ERROR media %d for level %d is" - " not supported\n", + } + switch (media->type) { + case SDP_MEDIA_AUDIO: + gsmsdp_set_media_attributes(media->payload, sdp_p, level, + (uint16_t)dynamic_payload_type); + break; + case SDP_MEDIA_VIDEO: + gsmsdp_set_video_media_attributes(media->payload, cc_sdp_p, level, + (uint16_t)dynamic_payload_type); + break; + case SDP_MEDIA_APPLICATION: + gsmsdp_set_sctp_attributes (sdp_p, level, media); + break; + default: + GSM_ERR_MSG(GSM_L_C_F_PREFIX"SDP ERROR media %d for level %d is not" + " supported\n", dcb_p->line, dcb_p->call_id, fname, media->level); - break; - } - - }//end for + break; + } /* * add the avt media type @@ -2850,8 +2808,7 @@ gsmsdp_negotiate_codec (fsmdef_dcb_t *dcb_p, cc_sdp_t *sdp_p, /* * Save pay load type for use by gsmspd_compare_to_previous_sdp */ - media->previous_sdp.payload_type = - media->num_payloads ? media->payloads[0] : RTP_NONE; + media->previous_sdp.payload_type = media->payload; media->previous_sdp.local_payload_type = media->local_dynamic_payload_type_value; /* @@ -2864,19 +2821,13 @@ gsmsdp_negotiate_codec (fsmdef_dcb_t *dcb_p, cc_sdp_t *sdp_p, payload_types_count = num_slave_types; } - /* Remove any previously allocated lists */ - if (media->payloads) { - cpr_free(media->payloads); - cpr_free(media->local_dpt_list); - cpr_free(media->remote_dpt_list); + media->payloads = (vcm_media_payload_type_t*) cpr_malloc(payload_types_count * sizeof(vcm_media_payload_type_t)); + if (!(media->payloads)) + { + GSM_ERR_MSG(GSM_L_C_F_PREFIX"Memory Allocation failed for payloads\n", + DEB_L_C_F_PREFIX_ARGS(GSM, dcb_p->line, dcb_p->call_id, fname)); + return RTP_NONE; } - /* Allocate memory for PT value, local PT and remote PT. */ - media->payloads = cpr_calloc(payload_types_count, - sizeof(vcm_media_payload_type_t)); - media->local_dpt_list = cpr_calloc(payload_types_count, - sizeof(uint8_t)); - media->remote_dpt_list = cpr_calloc(payload_types_count, - sizeof(uint8_t)); for (i = 0; i < num_master_types; i++) { for (j = 0; j < num_slave_types; j++) { @@ -2891,7 +2842,7 @@ gsmsdp_negotiate_codec (fsmdef_dcb_t *dcb_p, cc_sdp_t *sdp_p, } } else { //if remote SDP is an answer if (media->local_dynamic_payload_type_value == RTP_NONE || - !media->num_payloads || media->payloads[0] != media->previous_sdp.payload_type) { + media->payload != media->previous_sdp.payload_type) { /* If the the negotiated payload type is different from previous, set it the local dynamic to payload type as this is what we offered*/ } @@ -2907,7 +2858,7 @@ gsmsdp_negotiate_codec (fsmdef_dcb_t *dcb_p, cc_sdp_t *sdp_p, if (payload == RTP_ILBC) { media->mode = (uint16_t)sdp_attr_get_fmtp_mode_for_payload_type (sdp_p->dest_sdp, level, 0, - remote_dynamic_payload_type_value); + media->remote_dynamic_payload_type_value); } if (payload == RTP_OPUS) { u16 a_inst; @@ -2957,13 +2908,14 @@ gsmsdp_negotiate_codec (fsmdef_dcb_t *dcb_p, cc_sdp_t *sdp_p, vcmFreeMediaPtr(media->video); media->video = NULL; } - if ( vcmCheckAttribs(payload, sdp_p, level, - &media->video) == FALSE ) { - GSM_DEBUG(DEB_L_C_F_PREFIX"codec= %d ignored - attribs not accepted\n", - DEB_L_C_F_PREFIX_ARGS(GSM, dcb_p->line, - dcb_p->call_id, fname), payload); - explicit_reject = TRUE; - continue; // keep looking + + if ( vcmCheckAttribs(media->payload, sdp_p, level, + &media->video) == FALSE ) { + GSM_DEBUG(DEB_L_C_F_PREFIX"codec= %d ignored - attribs not accepted\n", + DEB_L_C_F_PREFIX_ARGS(GSM, dcb_p->line, + dcb_p->call_id, fname), media->payload); + explicit_reject = TRUE; + continue; // keep looking } // cache the negotiated profile_level and bandwidth @@ -2977,7 +2929,7 @@ gsmsdp_negotiate_codec (fsmdef_dcb_t *dcb_p, cc_sdp_t *sdp_p, GSM_DEBUG(DEB_L_C_F_PREFIX"codec= %d\n", DEB_L_C_F_PREFIX_ARGS(GSM, dcb_p->line, - dcb_p->call_id, fname), payload); + dcb_p->call_id, fname), media->payload); } found_codec = TRUE; @@ -2985,17 +2937,12 @@ gsmsdp_negotiate_codec (fsmdef_dcb_t *dcb_p, cc_sdp_t *sdp_p, //we maxed our allocated memory. ignore and return; return payload; } - //update media structure with the negotiated value media->payloads[media->num_payloads] = vcmRtpToMediaPayload(payload, remote_dynamic_payload_type_value, media->mode); - media->local_dpt_list[media->num_payloads] = payload; - media->remote_dpt_list[media->num_payloads] = remote_dynamic_payload_type_value; media->num_payloads++; if(offer) { //return on the first match - // TODO -- Eventually, we'll (probably) want to answer with all - // the codecs we can receive. See bug 814227. return payload; } } @@ -3015,20 +2962,20 @@ gsmsdp_negotiate_codec (fsmdef_dcb_t *dcb_p, cc_sdp_t *sdp_p, */ if (!initial_offer && explicit_reject == FALSE) { for (i = 0; i < num_types; i++) { - if (media->num_payloads != 0 && media->payloads[0] == GET_CODEC_TYPE(remote_media_types[i])) { + if (media->payload == GET_CODEC_TYPE(remote_media_types[i])) { /* * CSCta40560 - DSP runs out of bandwidth as the current API's do not consider the request is for * the same call. Need to update dynamic payload types for dynamic PT codecs. Would need to possibly * add other video codecs as support is added here. */ - if ( (media->payloads[0] == RTP_H264_P1 || media->payloads[0] == RTP_H264_P0) && offer == TRUE ) { + if ( (media->payload == RTP_H264_P1 || media->payload == RTP_H264_P0) && offer == TRUE ) { media->remote_dynamic_payload_type_value = GET_DYN_PAYLOAD_TYPE_VALUE(master_list_p[i]); media->local_dynamic_payload_type_value = GET_DYN_PAYLOAD_TYPE_VALUE(master_list_p[i]); } GSM_DEBUG(DEB_L_C_F_PREFIX"local codec list was empty codec= %d local=%d remote =%d\n", DEB_L_C_F_PREFIX_ARGS(GSM, dcb_p->line, dcb_p->call_id, fname), - media->payloads[0], media->local_dynamic_payload_type_value, media->remote_dynamic_payload_type_value); - return (media->payloads[0]); + media->payload, media->local_dynamic_payload_type_value, media->remote_dynamic_payload_type_value); + return (media->payload); } } } @@ -4194,7 +4141,7 @@ gsmsdp_negotiate_media_lines (fsm_fcb_t *fcb_p, cc_sdp_t *sdp_p, boolean initial GSM_DEBUG(DEB_L_C_F_PREFIX"remote_direction: %d global match %sfound\n", DEB_L_C_F_PREFIX_ARGS(GSM, dcb_p->line, dcb_p->call_id, fname), - remote_direction, (cap_index != CC_MAX_MEDIA_CAP) ? "" : "not "); + remote_direction, (cap_index != CC_MAX_MEDIA_CAP)?"":"not "); if ( cap_index != CC_MAX_MEDIA_CAP && remote_direction != SDP_DIRECTION_INACTIVE ) { // this is an offer and platform can support video @@ -4809,17 +4756,11 @@ gsmsdp_add_media_line (fsmdef_dcb_t *dcb_p, const cc_media_cap_t *media_cap, * Since we are initiating an initial offer and opening a * receive port, store initial media settings. */ - - /* TODO -- Eventually, the structure in previous_sdp needs to be - updated to match the payloads[] array so that we can act on - a codec other than the first one changing. */ media->previous_sdp.avt_payload_type = media->avt_payload_type; media->previous_sdp.direction = media->direction; media->previous_sdp.packetization_period = media->packetization_period; - media->previous_sdp.payload_type = - media->num_payloads ? media->payloads[0] : RTP_NONE; - media->previous_sdp.local_payload_type = - media->num_payloads ? media->payloads[0] : RTP_NONE; + media->previous_sdp.payload_type = media->payload; + media->previous_sdp.local_payload_type = media->payload; break; } @@ -6290,14 +6231,11 @@ gsmsdp_sdp_differs_from_previous_sdp (boolean rcv_only, fsmdef_media_t *media) * Consider attributes of interest common to all active directions. */ if ((media->previous_sdp.avt_payload_type != media->avt_payload_type) || - (0 == media->num_payloads) || - (media->previous_sdp.payload_type != media->payloads[0])) { + (media->previous_sdp.payload_type != media->payload)) { GSM_DEBUG(DEB_F_PREFIX"previous payload: %d new payload: %d\n", - DEB_F_PREFIX_ARGS(GSM, fname), - media->previous_sdp.payload_type, media->payloads[0]); + DEB_F_PREFIX_ARGS(GSM, fname), media->previous_sdp.payload_type, media->payload); GSM_DEBUG(DEB_F_PREFIX"previous avt payload: %d new avt payload: %d\n", - DEB_F_PREFIX_ARGS(GSM, fname), - media->previous_sdp.avt_payload_type, + DEB_F_PREFIX_ARGS(GSM, fname), media->previous_sdp.avt_payload_type, media->avt_payload_type); return TRUE; } diff --git a/media/webrtc/signaling/src/sipcc/core/gsm/h/fsm.h b/media/webrtc/signaling/src/sipcc/core/gsm/h/fsm.h index 958f35a488aa..d3ba16c2a9a8 100755 --- a/media/webrtc/signaling/src/sipcc/core/gsm/h/fsm.h +++ b/media/webrtc/signaling/src/sipcc/core/gsm/h/fsm.h @@ -146,6 +146,7 @@ typedef struct fsmdef_media_t_ { * We answer with the same payload type value that the remote offers. * If remote choses to answer with different value than we offer, we support asymmetric. */ + int32_t payload; //payload type - one of rtp_ptype enumerations int32_t local_dynamic_payload_type_value; // dynamic payload type value offered/answered by us int32_t remote_dynamic_payload_type_value; // dynamic payload type value offered/answered by remote int32_t avt_payload_type; @@ -236,30 +237,11 @@ typedef struct fsmdef_media_t_ { uint32 streams; char *protocol; - /* - * This field contains the size of the payloads, - * local_dpt_list, and remote_dpt_list fields. - */ - int32_t num_payloads; - /* * List of active lists of payloads negotiated - * There needs to be 1-1 mapping between the 3 arrays - * TODO:crypt: Move these to a aggregate structure per payload - * since we need to include codec type, local rtp type, remote rty type, - * codec specific parameters as generated by any SDP action. */ vcm_media_payload_type_t* payloads; - - /* - * dynamic payload type value offered/answered by us - */ - uint8_t* local_dpt_list; - - /* - * dynamic payload type value offered/answered by remote - */ - uint8_t* remote_dpt_list; + int32_t num_payloads; } fsmdef_media_t; struct fsm_fcb_t_; diff --git a/media/webrtc/signaling/src/sipcc/core/gsm/lsm.c b/media/webrtc/signaling/src/sipcc/core/gsm/lsm.c index 3ca597f48751..791ee701e0a7 100755 --- a/media/webrtc/signaling/src/sipcc/core/gsm/lsm.c +++ b/media/webrtc/signaling/src/sipcc/core/gsm/lsm.c @@ -995,8 +995,7 @@ lsm_rx_start (lsm_lcb_t *lcb, const char *fname, fsmdef_media_t *media) pc_track_id = 0; dcb->cur_video_avail &= ~CC_ATTRIB_CAST; if (media->local_dynamic_payload_type_value == RTP_NONE) { - media->local_dynamic_payload_type_value = - media->num_payloads ? media->payloads[0] : RTP_NONE; + media->local_dynamic_payload_type_value = media->payload; } config_get_value(CFGID_SDPMODE, &sdpmode, sizeof(sdpmode)); @@ -1015,15 +1014,9 @@ lsm_rx_start (lsm_lcb_t *lcb, const char *fname, fsmdef_media_t *media) } else if (!sdpmode) { ret_val = vcmRxStart(media->cap_index, group_id, media->refid, lsm_get_ms_ui_call_handle(dcb->line, call_id, CC_NO_CALL_ID), - /* RTP_NONE is not technically a valid - value for vcm_media_payload_type_t. - However, we really should never - reach this part of the code with - num_payloads < 1, so the RTP_NONE - shouldn't ever be used. It's only - here are extra protection against - an invalid pointer deref.*/ - media->num_payloads ? media->payloads[0] : RTP_NONE, + vcmRtpToMediaPayload(media->payload, + media->local_dynamic_payload_type_value, + media->mode), media->is_multicast ? &media->dest_addr:&media->src_addr, port, FSM_NEGOTIATED_CRYPTO_ALGORITHM_ID(media), @@ -1242,7 +1235,9 @@ lsm_tx_start (lsm_lcb_t *lcb, const char *fname, fsmdef_media_t *media) if (vcmTxStart(media->cap_index, group_id, media->refid, lsm_get_ms_ui_call_handle(dcb->line, call_id, CC_NO_CALL_ID), - media->num_payloads ? media->payloads[0] : RTP_NONE, + vcmRtpToMediaPayload(media->payload, + media->remote_dynamic_payload_type_value, + media->mode), (short)dscp, &media->src_addr, media->src_port, @@ -1268,7 +1263,9 @@ lsm_tx_start (lsm_lcb_t *lcb, const char *fname, fsmdef_media_t *media) dcb->media_cap_tbl->cap[media->cap_index].pc_track, lsm_get_ms_ui_call_handle(dcb->line, call_id, CC_NO_CALL_ID), dcb->peerconnection, - media->num_payloads ? media->payloads[0] : RTP_NONE, + vcmRtpToMediaPayload(media->payload, + media->remote_dynamic_payload_type_value, + media->mode), (short)dscp, FSM_NEGOTIATED_CRYPTO_DIGEST_ALGORITHM(media), FSM_NEGOTIATED_CRYPTO_DIGEST(media), @@ -1952,7 +1949,7 @@ lsm_get_free_lcb (callid_t call_id, line_t line, fsmdef_dcb_t *dcb) lcb->mru = mru; lcb->dcb = dcb; // start unmuted if txPref is true - lcb->vid_mute = cc_media_getVideoAutoTxPref() ? FALSE : TRUE; + lcb->vid_mute = cc_media_getVideoAutoTxPref()?FALSE:TRUE; lcb->ui_id = call_id; /* default UI ID is the same as call_id */ break; @@ -3860,16 +3857,12 @@ lsm_update_media (lsm_lcb_t *lcb, const char *caller_fname) if (LSMDebug) { /* debug is enabled, format the dest addr into string */ ipaddr2dotted(addr_str, &media->dest_addr); - for (int i = 0; i < media->num_payloads; i++) - { - LSM_DEBUG(DEB_L_C_F_PREFIX"%d rx, tx refresh's are %d %d" - ", dir=%d, payload=%d addr=%s, multicast=%d\n", - DEB_L_C_F_PREFIX_ARGS(LSM, dcb->line, - dcb->call_id, fname), media->refid, rx_refresh, - tx_refresh, media->direction, - media->payloads[i], addr_str, media->is_multicast ); - } } + LSM_DEBUG(DEB_L_C_F_PREFIX"%d rx, tx refresh's are %d %d" + ", dir=%d, payload=%d addr=%s, multicast=%d\n", + DEB_L_C_F_PREFIX_ARGS(LSM, dcb->line, dcb->call_id, fname), + media->refid, rx_refresh, tx_refresh, media->direction, + media->payload, addr_str, media->is_multicast ); if (rx_refresh || (media->is_multicast && media->direction_set && @@ -5296,9 +5289,9 @@ void lsm_add_remote_stream (line_t line, callid_t call_id, fsmdef_media_t *media return; } - vcmCreateRemoteStream(media->cap_index, dcb->peerconnection, - pc_stream_id, - media->num_payloads ? media->payloads[0] : RTP_NONE); + vcmCreateRemoteStream(media->cap_index, dcb->peerconnection, pc_stream_id, + vcmRtpToMediaPayload(media->payload, + media->local_dynamic_payload_type_value,media->mode)); } } diff --git a/media/webrtc/signaling/src/sipcc/core/includes/vcm_util.h b/media/webrtc/signaling/src/sipcc/core/includes/vcm_util.h index 0428df64786d..e3b11ed89af6 100644 --- a/media/webrtc/signaling/src/sipcc/core/includes/vcm_util.h +++ b/media/webrtc/signaling/src/sipcc/core/includes/vcm_util.h @@ -19,5 +19,4 @@ vcm_media_payload_type_t vcm_rtp_to_media_payload (int32_t ptype, uint16_t mode); -int32_t mediaPayloadToVcmRtp (vcm_media_payload_type_t payload_in); #endif /* VCM_UTIL_H_ */ diff --git a/media/webrtc/signaling/test/signaling_unittests.cpp b/media/webrtc/signaling/test/signaling_unittests.cpp index d053a184388f..e2388a4cb145 100644 --- a/media/webrtc/signaling/test/signaling_unittests.cpp +++ b/media/webrtc/signaling/test/signaling_unittests.cpp @@ -575,8 +575,7 @@ class SignalingAgent { } void CreateAnswer(sipcc::MediaConstraints& constraints, std::string offer, - uint32_t offerAnswerFlags, - uint32_t sdpCheck = DONT_CHECK_AUDIO|DONT_CHECK_VIDEO) { + uint32_t offerAnswerFlags, uint32_t sdpCheck) { // Create a media stream as if it came from GUM nsRefPtr domMediaStream = new nsDOMMediaStream(); @@ -1218,8 +1217,7 @@ TEST_F(SignalingTest, FullCallTrickle) ASSERT_GE(a2_.GetPacketsReceived(0), 40); } -// This test comes from Bug 810220 -TEST_F(SignalingTest, AudioOnlyG711Call) +TEST_F(SignalingTest, Bug810220) { sipcc::MediaConstraints constraints; std::string offer = @@ -1230,11 +1228,10 @@ TEST_F(SignalingTest, AudioOnlyG711Call) "t=0 0\r\n" "a=fingerprint:sha-256 F3:FA:20:C0:CD:48:C4:5F:02:5F:A5:D3:21:D0:2D:48:" "7B:31:60:5C:5A:D8:0D:CD:78:78:6C:6D:CE:CC:0C:67\r\n" - "m=audio 9000 RTP/AVP 0 8 126\r\n" + "m=audio 9000 RTP/AVP 0 126\r\n" "c=IN IP4 148.147.200.251\r\n" "b=TIAS:64000\r\n" "a=rtpmap:0 PCMU/8000\r\n" - "a=rtpmap:8 PCMA/8000\r\n" "a=rtpmap:126 telephone-event/8000\r\n" "a=candidate:0 1 udp 2130706432 148.147.200.251 9000 typ host\r\n" "a=candidate:0 2 udp 2130706432 148.147.200.251 9005 typ host\r\n" @@ -1250,29 +1247,12 @@ TEST_F(SignalingTest, AudioOnlyG711Call) DONT_CHECK_AUDIO | DONT_CHECK_VIDEO); std::string answer = a2_.answer(); - - // They didn't offer opus, so our answer shouldn't include it. - ASSERT_EQ(answer.find(" opus/"), std::string::npos); - - // They also didn't offer video or application - ASSERT_EQ(answer.find("video"), std::string::npos); - ASSERT_EQ(answer.find("application"), std::string::npos); - - // We should answer with PCMU and telephone-event - ASSERT_NE(answer.find(" PCMU/8000"), std::string::npos); - ASSERT_NE(answer.find(" telephone-event/8000"), std::string::npos); - - // Double-check the directionality - ASSERT_NE(answer.find("\r\na=sendrecv"), std::string::npos); - + /* TODO -- make sure the answer looks right */ } -// This test comes from Bug814038 -TEST_F(SignalingTest, ChromeOfferAnswer) +TEST_F(SignalingTest, Bug814038) { sipcc::MediaConstraints constraints; - - // This is captured SDP from an early interop attempt with Chrome. std::string offer = "v=0\r\n" "o=- 1713781661 2 IN IP4 127.0.0.1\r\n" @@ -1295,12 +1275,7 @@ TEST_F(SignalingTest, ChromeOfferAnswer) "RzrYlzpkTsvgYFD1hQqNCzQ7y4emNLKI1tODsjim\r\n" "a=rtpmap:103 ISAC/16000\r\n" "a=rtpmap:104 ISAC/32000\r\n" - // NOTE: the actual SDP that Chrome sends at the moment - // doesn't indicate two channels. I've amended their SDP - // here, under the assumption that the constraints - // described in draft-spittka-payload-rtp-opus will - // eventually be implemented by Google. - "a=rtpmap:111 opus/48000/2\r\n" + "a=rtpmap:111 opus/48000\r\n" "a=rtpmap:0 PCMU/8000\r\n" "a=rtpmap:8 PCMA/8000\r\n" "a=rtpmap:107 CN/48000\r\n" @@ -1339,13 +1314,13 @@ TEST_F(SignalingTest, ChromeOfferAnswer) a2_.SetRemote(TestObserver::OFFER, offer); std::cout << "Creating answer:" << std::endl; - a2_.CreateAnswer(constraints, offer, OFFER_AUDIO | ANSWER_AUDIO); + a2_.CreateAnswer(constraints, offer, OFFER_AUDIO | ANSWER_AUDIO, + SHOULD_INACTIVE_AUDIO | SHOULD_INACTIVE_VIDEO); std::string answer = a2_.answer(); } - } // End namespace test. int main(int argc, char **argv) {