Bug 1282541 - remove postfix increment on register set iterators; r=nbp

A surprising number of places use postfix increment on register set
iterators, which is theoretically less efficient due to requiring a copy
of the iterator.  A sufficiently smart compiler may be able to optimize
that copy out...but seeing postfix iterators is arguably surprising and
none of the current call sites actually need the copy.  So let's convert
all the postfix increments into prefix increments and remove the postfix
version.  That way, nobody else makes the same mistake and the code
reads more idiomatically.
This commit is contained in:
Nathan Froyd 2016-06-28 20:02:48 -04:00
Родитель f181ac8328
Коммит fae35df0c6
14 изменённых файлов: 39 добавлений и 55 удалений

Просмотреть файл

@ -560,7 +560,7 @@ BacktrackingAllocator::buildLivenessInfo()
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_.asLiveSet()); iter.more(); iter++) {
for (AnyRegisterIterator iter(allRegisters_.asLiveSet()); iter.more(); ++iter) {
bool found = false;
for (size_t i = 0; i < ins->numDefs(); i++) {
if (ins->getDef(i)->isFixed() &&

Просмотреть файл

@ -298,7 +298,7 @@ JitFrameIterator::machineState() const
uintptr_t* spill = spillBase();
MachineState machine;
for (GeneralRegisterBackwardIterator iter(reader.allGprSpills()); iter.more(); iter++)
for (GeneralRegisterBackwardIterator iter(reader.allGprSpills()); iter.more(); ++iter)
machine.setRegisterLocation(*iter, --spill);
uint8_t* spillAlign = alignDoubleSpillWithOffset(reinterpret_cast<uint8_t*>(spill), 0);
@ -306,7 +306,7 @@ JitFrameIterator::machineState() const
char* floatSpill = reinterpret_cast<char*>(spillAlign);
FloatRegisterSet fregs = reader.allFloatSpills().set();
fregs = fregs.reduceSetForPush();
for (FloatRegisterBackwardIterator iter(fregs); iter.more(); iter++) {
for (FloatRegisterBackwardIterator iter(fregs); iter.more(); ++iter) {
floatSpill -= (*iter).size();
for (uint32_t a = 0; a < (*iter).numAlignedAliased(); a++) {
// Only say that registers that actually start here start here.
@ -1049,7 +1049,7 @@ MarkIonJSFrame(JSTracer* trc, const JitFrameIterator& frame)
uintptr_t* spill = frame.spillBase();
LiveGeneralRegisterSet gcRegs = safepoint.gcSpills();
LiveGeneralRegisterSet valueRegs = safepoint.valueSpills();
for (GeneralRegisterBackwardIterator iter(safepoint.allGprSpills()); iter.more(); iter++) {
for (GeneralRegisterBackwardIterator iter(safepoint.allGprSpills()); iter.more(); ++iter) {
--spill;
if (gcRegs.has(*iter))
TraceGenericPointerRoot(trc, reinterpret_cast<gc::Cell**>(spill), "ion-gc-spill");
@ -1136,7 +1136,7 @@ UpdateIonJSFrameForMinorGC(JSTracer* trc, const JitFrameIterator& frame)
LiveGeneralRegisterSet slotsRegs = safepoint.slotsOrElementsSpills();
uintptr_t* spill = frame.spillBase();
for (GeneralRegisterBackwardIterator iter(safepoint.allGprSpills()); iter.more(); iter++) {
for (GeneralRegisterBackwardIterator iter(safepoint.allGprSpills()); iter.more(); ++iter) {
--spill;
if (slotsRegs.has(*iter))
nursery.forwardBufferPointer(reinterpret_cast<HeapSlot**>(spill));

Просмотреть файл

@ -1096,11 +1096,6 @@ class TypedRegisterIterator
bool more() const {
return !regset_.empty();
}
TypedRegisterIterator<T> operator ++(int) {
TypedRegisterIterator<T> old(*this);
regset_.takeAny();
return old;
}
TypedRegisterIterator<T>& operator ++() {
regset_.takeAny();
return *this;
@ -1128,11 +1123,6 @@ class TypedRegisterBackwardIterator
bool more() const {
return !regset_.empty();
}
TypedRegisterBackwardIterator<T> operator ++(int) {
TypedRegisterBackwardIterator<T> old(*this);
regset_.takeLast();
return old;
}
TypedRegisterBackwardIterator<T>& operator ++() {
regset_.takeLast();
return *this;
@ -1159,11 +1149,6 @@ class TypedRegisterForwardIterator
bool more() const {
return !regset_.empty();
}
TypedRegisterForwardIterator<T> operator ++(int) {
TypedRegisterForwardIterator<T> old(*this);
regset_.takeFirst();
return old;
}
TypedRegisterForwardIterator<T>& operator ++() {
regset_.takeFirst();
return *this;
@ -1204,13 +1189,12 @@ class AnyRegisterIterator
bool more() const {
return geniter_.more() || floatiter_.more();
}
AnyRegisterIterator operator ++(int) {
AnyRegisterIterator old(*this);
AnyRegisterIterator& operator ++() {
if (geniter_.more())
geniter_++;
++geniter_;
else
floatiter_++;
return old;
++floatiter_;
return *this;
}
AnyRegister operator*() const {
if (geniter_.more())
@ -1297,7 +1281,7 @@ SavedNonVolatileRegisters(AllocatableGeneralRegisterSet unused)
{
LiveGeneralRegisterSet result;
for (GeneralRegisterIterator iter(GeneralRegisterSet::NonVolatile()); iter.more(); iter++) {
for (GeneralRegisterIterator iter(GeneralRegisterSet::NonVolatile()); iter.more(); ++iter) {
Register reg = *iter;
if (!unused.has(reg))
result.add(reg);

Просмотреть файл

@ -113,7 +113,7 @@ SafepointWriter::writeGcRegs(LSafepoint* safepoint)
#ifdef JS_JITSPEW
if (JitSpewEnabled(JitSpew_Safepoints)) {
for (GeneralRegisterForwardIterator iter(spilledGpr); iter.more(); iter++) {
for (GeneralRegisterForwardIterator iter(spilledGpr); iter.more(); ++iter) {
const char* type = gc.has(*iter)
? "gc"
: slots.has(*iter)
@ -123,7 +123,7 @@ SafepointWriter::writeGcRegs(LSafepoint* safepoint)
: "any";
JitSpew(JitSpew_Safepoints, " %s reg: %s", type, (*iter).name());
}
for (FloatRegisterForwardIterator iter(spilledFloat); iter.more(); iter++)
for (FloatRegisterForwardIterator iter(spilledFloat); iter.more(); ++iter)
JitSpew(JitSpew_Safepoints, " float reg: %s", (*iter).name());
}
#endif

Просмотреть файл

@ -357,7 +357,7 @@ FloatRegisterSet
VFPRegister::ReduceSetForPush(const FloatRegisterSet& s)
{
LiveFloatRegisterSet mod;
for (FloatRegisterIterator iter(s); iter.more(); iter++) {
for (FloatRegisterIterator iter(s); iter.more(); ++iter) {
if ((*iter).isSingle()) {
// Add in just this float.
mod.addUnchecked(*iter);

Просмотреть файл

@ -4558,14 +4558,14 @@ MacroAssembler::PushRegsInMask(LiveRegisterSet set)
if (set.gprs().size() > 1) {
adjustFrame(diffG);
startDataTransferM(IsStore, StackPointer, DB, WriteBack);
for (GeneralRegisterBackwardIterator iter(set.gprs()); iter.more(); iter++) {
for (GeneralRegisterBackwardIterator iter(set.gprs()); iter.more(); ++iter) {
diffG -= sizeof(intptr_t);
transferReg(*iter);
}
finishDataTransfer();
} else {
reserveStack(diffG);
for (GeneralRegisterBackwardIterator iter(set.gprs()); iter.more(); iter++) {
for (GeneralRegisterBackwardIterator iter(set.gprs()); iter.more(); ++iter) {
diffG -= sizeof(intptr_t);
storePtr(*iter, Address(StackPointer, diffG));
}
@ -4593,7 +4593,7 @@ MacroAssembler::PopRegsInMaskIgnore(LiveRegisterSet set, LiveRegisterSet ignore)
} else {
LiveFloatRegisterSet fpset(set.fpus().reduceSetForPush());
LiveFloatRegisterSet fpignore(ignore.fpus().reduceSetForPush());
for (FloatRegisterBackwardIterator iter(fpset); iter.more(); iter++) {
for (FloatRegisterBackwardIterator iter(fpset); iter.more(); ++iter) {
diffF -= (*iter).size();
if (!fpignore.has(*iter))
loadDouble(Address(StackPointer, diffF), *iter);
@ -4604,14 +4604,14 @@ MacroAssembler::PopRegsInMaskIgnore(LiveRegisterSet set, LiveRegisterSet ignore)
if (set.gprs().size() > 1 && ignore.emptyGeneral()) {
startDataTransferM(IsLoad, StackPointer, IA, WriteBack);
for (GeneralRegisterBackwardIterator iter(set.gprs()); iter.more(); iter++) {
for (GeneralRegisterBackwardIterator iter(set.gprs()); iter.more(); ++iter) {
diffG -= sizeof(intptr_t);
transferReg(*iter);
}
finishDataTransfer();
adjustFrame(-reservedG);
} else {
for (GeneralRegisterBackwardIterator iter(set.gprs()); iter.more(); iter++) {
for (GeneralRegisterBackwardIterator iter(set.gprs()); iter.more(); ++iter) {
diffG -= sizeof(intptr_t);
if (!ignore.has(*iter))
loadPtr(Address(StackPointer, diffG), *iter);

Просмотреть файл

@ -66,7 +66,7 @@ FloatRegisterSet
FloatRegister::ReduceSetForPush(const FloatRegisterSet& s)
{
LiveFloatRegisterSet mod;
for (FloatRegisterIterator iter(s); iter.more(); iter++) {
for (FloatRegisterIterator iter(s); iter.more(); ++iter) {
if ((*iter).isSingle()) {
// Even for single size registers save complete double register.
mod.addUnchecked((*iter).doubleOverlay());

Просмотреть файл

@ -2003,7 +2003,7 @@ MacroAssembler::PushRegsInMask(LiveRegisterSet set)
int32_t diffG = set.gprs().size() * sizeof(intptr_t);
reserveStack(diffG);
for (GeneralRegisterBackwardIterator iter(set.gprs()); iter.more(); iter++) {
for (GeneralRegisterBackwardIterator iter(set.gprs()); iter.more(); ++iter) {
diffG -= sizeof(intptr_t);
storePtr(*iter, Address(StackPointer, diffG));
}
@ -2015,7 +2015,7 @@ MacroAssembler::PushRegsInMask(LiveRegisterSet set)
ma_and(SecondScratchReg, sp, Imm32(~(ABIStackAlignment - 1)));
reserveStack(diffF + sizeof(double));
for (FloatRegisterForwardIterator iter(set.fpus().reduceSetForPush()); iter.more(); iter++) {
for (FloatRegisterForwardIterator iter(set.fpus().reduceSetForPush()); iter.more(); ++iter) {
if ((*iter).code() % 2 == 0)
as_sd(*iter, SecondScratchReg, -diffF);
diffF -= sizeof(double);
@ -2035,7 +2035,7 @@ MacroAssembler::PopRegsInMaskIgnore(LiveRegisterSet set, LiveRegisterSet ignore)
ma_addu(SecondScratchReg, sp, Imm32(reservedF + sizeof(double)));
ma_and(SecondScratchReg, SecondScratchReg, Imm32(~(ABIStackAlignment - 1)));
for (FloatRegisterForwardIterator iter(set.fpus().reduceSetForPush()); iter.more(); iter++) {
for (FloatRegisterForwardIterator iter(set.fpus().reduceSetForPush()); iter.more(); ++iter) {
if (!ignore.has(*iter) && ((*iter).code() % 2 == 0))
// Use assembly l.d because we have alligned the stack.
as_ld(*iter, SecondScratchReg, -diffF);
@ -2044,7 +2044,7 @@ MacroAssembler::PopRegsInMaskIgnore(LiveRegisterSet set, LiveRegisterSet ignore)
freeStack(reservedF + sizeof(double));
MOZ_ASSERT(diffF == 0);
for (GeneralRegisterBackwardIterator iter(set.gprs()); iter.more(); iter++) {
for (GeneralRegisterBackwardIterator iter(set.gprs()); iter.more(); ++iter) {
diffG -= sizeof(intptr_t);
if (!ignore.has(*iter))
loadPtr(Address(StackPointer, diffG), *iter);

Просмотреть файл

@ -61,7 +61,7 @@ FloatRegisterSet
FloatRegister::ReduceSetForPush(const FloatRegisterSet& s)
{
LiveFloatRegisterSet mod;
for (FloatRegisterIterator iter(s); iter.more(); iter++) {
for (FloatRegisterIterator iter(s); iter.more(); ++iter) {
if ((*iter).isSingle()) {
// Even for single size registers save complete double register.
mod.addUnchecked((*iter).doubleOverlay());

Просмотреть файл

@ -2125,11 +2125,11 @@ MacroAssembler::PushRegsInMask(LiveRegisterSet set)
const int32_t reserved = diff;
reserveStack(reserved);
for (GeneralRegisterBackwardIterator iter(set.gprs()); iter.more(); iter++) {
for (GeneralRegisterBackwardIterator iter(set.gprs()); iter.more(); ++iter) {
diff -= sizeof(intptr_t);
storePtr(*iter, Address(StackPointer, diff));
}
for (FloatRegisterBackwardIterator iter(set.fpus().reduceSetForPush()); iter.more(); iter++) {
for (FloatRegisterBackwardIterator iter(set.fpus().reduceSetForPush()); iter.more(); ++iter) {
diff -= sizeof(double);
storeDouble(*iter, Address(StackPointer, diff));
}
@ -2143,12 +2143,12 @@ MacroAssembler::PopRegsInMaskIgnore(LiveRegisterSet set, LiveRegisterSet ignore)
set.fpus().getPushSizeInBytes();
const int32_t reserved = diff;
for (GeneralRegisterBackwardIterator iter(set.gprs()); iter.more(); iter++) {
for (GeneralRegisterBackwardIterator iter(set.gprs()); iter.more(); ++iter) {
diff -= sizeof(intptr_t);
if (!ignore.has(*iter))
loadPtr(Address(StackPointer, diff), *iter);
}
for (FloatRegisterBackwardIterator iter(set.fpus().reduceSetForPush()); iter.more(); iter++) {
for (FloatRegisterBackwardIterator iter(set.fpus().reduceSetForPush()); iter.more(); ++iter) {
diff -= sizeof(double);
if (!ignore.has(*iter))
loadDouble(Address(StackPointer, diff), *iter);

Просмотреть файл

@ -1137,7 +1137,7 @@ HandleRegisterDump(Op op, MacroAssembler& masm, LiveRegisterSet liveRegs, Regist
const size_t baseOffset = JitActivation::offsetOfRegs();
// Handle live GPRs.
for (GeneralRegisterIterator iter(liveRegs.gprs()); iter.more(); iter++) {
for (GeneralRegisterIterator iter(liveRegs.gprs()); iter.more(); ++iter) {
Register reg = *iter;
Address dump(activation, baseOffset + RegisterDump::offsetOfRegister(reg));
@ -1154,7 +1154,7 @@ HandleRegisterDump(Op op, MacroAssembler& masm, LiveRegisterSet liveRegs, Regist
}
// Handle live FPRs.
for (FloatRegisterIterator iter(liveRegs.fpus()); iter.more(); iter++) {
for (FloatRegisterIterator iter(liveRegs.fpus()); iter.more(); ++iter) {
FloatRegister reg = *iter;
Address dump(activation, baseOffset + RegisterDump::offsetOfRegister(reg));
op(reg, dump);

Просмотреть файл

@ -563,11 +563,11 @@ PushBailoutFrame(MacroAssembler& masm, Register spArg)
// the float registers to have the maximal possible size
// (Simd128DataSize). To work around this, we just spill the double
// registers by hand here, using the register dump offset directly.
for (GeneralRegisterBackwardIterator iter(AllRegs.gprs()); iter.more(); iter++)
for (GeneralRegisterBackwardIterator iter(AllRegs.gprs()); iter.more(); ++iter)
masm.Push(*iter);
masm.reserveStack(sizeof(RegisterDump::FPUArray));
for (FloatRegisterBackwardIterator iter(AllRegs.fpus()); iter.more(); iter++) {
for (FloatRegisterBackwardIterator iter(AllRegs.fpus()); iter.more(); ++iter) {
FloatRegister reg = *iter;
Address spillAddress(StackPointer, reg.getRegisterDumpOffsetInBytes());
masm.storeDouble(reg, spillAddress);

Просмотреть файл

@ -489,14 +489,14 @@ MacroAssembler::PushRegsInMask(LiveRegisterSet set)
// On x86, always use push to push the integer registers, as it's fast
// on modern hardware and it's a small instruction.
for (GeneralRegisterBackwardIterator iter(set.gprs()); iter.more(); iter++) {
for (GeneralRegisterBackwardIterator iter(set.gprs()); iter.more(); ++iter) {
diffG -= sizeof(intptr_t);
Push(*iter);
}
MOZ_ASSERT(diffG == 0);
reserveStack(diffF);
for (FloatRegisterBackwardIterator iter(fpuSet); iter.more(); iter++) {
for (FloatRegisterBackwardIterator iter(fpuSet); iter.more(); ++iter) {
FloatRegister reg = *iter;
diffF -= reg.size();
numFpu -= 1;
@ -527,7 +527,7 @@ MacroAssembler::PopRegsInMaskIgnore(LiveRegisterSet set, LiveRegisterSet ignore)
const int32_t reservedG = diffG;
const int32_t reservedF = diffF;
for (FloatRegisterBackwardIterator iter(fpuSet); iter.more(); iter++) {
for (FloatRegisterBackwardIterator iter(fpuSet); iter.more(); ++iter) {
FloatRegister reg = *iter;
diffF -= reg.size();
numFpu -= 1;
@ -555,12 +555,12 @@ MacroAssembler::PopRegsInMaskIgnore(LiveRegisterSet set, LiveRegisterSet ignore)
// ignore any slots, as it's fast on modern hardware and it's a small
// instruction.
if (ignore.emptyGeneral()) {
for (GeneralRegisterForwardIterator iter(set.gprs()); iter.more(); iter++) {
for (GeneralRegisterForwardIterator iter(set.gprs()); iter.more(); ++iter) {
diffG -= sizeof(intptr_t);
Pop(*iter);
}
} else {
for (GeneralRegisterBackwardIterator iter(set.gprs()); iter.more(); iter++) {
for (GeneralRegisterBackwardIterator iter(set.gprs()); iter.more(); ++iter) {
diffG -= sizeof(intptr_t);
if (!ignore.has(*iter))
loadPtr(Address(StackPointer, diffG), *iter);

Просмотреть файл

@ -559,11 +559,11 @@ PushBailoutFrame(MacroAssembler& masm, uint32_t frameClass, Register spArg)
// the float registers to have the maximal possible size
// (Simd128DataSize). To work around this, we just spill the double
// registers by hand here, using the register dump offset directly.
for (GeneralRegisterBackwardIterator iter(AllRegs.gprs()); iter.more(); iter++)
for (GeneralRegisterBackwardIterator iter(AllRegs.gprs()); iter.more(); ++iter)
masm.Push(*iter);
masm.reserveStack(sizeof(RegisterDump::FPUArray));
for (FloatRegisterBackwardIterator iter(AllRegs.fpus()); iter.more(); iter++) {
for (FloatRegisterBackwardIterator iter(AllRegs.fpus()); iter.more(); ++iter) {
FloatRegister reg = *iter;
Address spillAddress(StackPointer, reg.getRegisterDumpOffsetInBytes());
masm.storeDouble(reg, spillAddress);