Permalink
Browse files

Optimize IterInit[K] and IterNext[K] in HHBBC better

Summary:
Do a better analysis of things being iterated over to enable better
optimizations of IterInit[K] and IterNext[K]. Namely, if we definitely know we
won't enter the iteration loop (for example, if the input array is known to be
empty, or something that isn't an array), we can mark the loop body as
unreachable and not consider its effects. Likewise, if we know we'll always
enter the iteration loop (for example, if the input array is known to be
non-empty), we can consider IterInit's fallthrough case to be unreachable. If we
know the thing being iterated over is an array that's not bigger than one
element, we can optimize away an IterNext into just an IterFree with
fall-through (because it will never loop).

Reviewed By: markw65

Differential Revision: D5801565

fbshipit-source-id: ae703dc883d29cb1a15435a5541011fb421f9bf1
  • Loading branch information...
ricklavoie authored and hhvm-bot committed Sep 11, 2017
1 parent 0b8780f commit 311faed7f02dc173d4b0c3ad9593cb54341536e6
@@ -45,11 +45,20 @@ bool merge_into(Iter& dst, const Iter& src, JoinOp join) {
return true;
},
[&] (const TrackedIter& siter) {
auto k1 = join(diter.kv.first, siter.kv.first);
auto k2 = join(diter.kv.second, siter.kv.second);
auto const changed = k1 != diter.kv.first || k2 != diter.kv.second;
diter.kv = std::make_pair(std::move(k1), std::move(k2));
return changed;
if (std::tie(diter.types.key,
diter.types.value,
diter.types.count,
diter.types.mayThrowOnInit,
diter.types.mayThrowOnNext) !=
std::tie(siter.types.key,
siter.types.value,
siter.types.count,
siter.types.mayThrowOnInit,
siter.types.mayThrowOnNext)) {
dst = UnknownIter {};
return true;
}
return false;
}
);
}
@@ -61,8 +70,8 @@ std::string show(const Iter& iter) {
iter,
[&] (UnknownIter) { return "unk"; },
[&] (const TrackedIter& ti) {
return folly::sformat("{}, {}", show(ti.kv.first),
show(ti.kv.second));
return folly::sformat("{}, {}", show(ti.types.key),
show(ti.types.value));
}
);
}
@@ -81,7 +81,7 @@ struct ActRec {
* State of an iterator in the program.
*/
struct UnknownIter {};
struct TrackedIter { std::pair<Type,Type> kv; };
struct TrackedIter { IterTypes types; };
using Iter = boost::variant< UnknownIter
, TrackedIter
>;
View
@@ -2530,19 +2530,39 @@ void in(ISS& env, const bc::DecodeCufIter& op) {
void in(ISS& env, const bc::IterInit& op) {
auto const t1 = popC(env);
// Take the branch before setting locals if the iter is already
// empty, but after popping. Similar for the other IterInits
// below.
freeIter(env, op.iter1);
env.propagate(op.target, env.state);
if (t1.subtypeOfAny(TArrE, TVecE, TDictE, TKeysetE)) {
nothrow(env);
jmp_nofallthrough(env);
return;
}
auto ity = iter_types(t1);
setLoc(env, op.loc3, ity.second);
setIter(env, op.iter1, TrackedIter { std::move(ity) });
if (!ity.mayThrowOnInit) nothrow(env);
auto const taken = [&]{
// Take the branch before setting locals if the iter is already
// empty, but after popping. Similar for the other IterInits
// below.
freeIter(env, op.iter1);
env.propagate(op.target, env.state);
};
auto const fallthrough = [&]{
setLoc(env, op.loc3, ity.value);
setIter(env, op.iter1, TrackedIter { std::move(ity) });
};
switch (ity.count) {
case IterTypes::Count::Empty:
taken();
mayReadLocal(env, op.loc3);
jmp_nofallthrough(env);
break;
case IterTypes::Count::Single:
case IterTypes::Count::NonEmpty:
fallthrough();
jmp_nevertaken(env);
break;
case IterTypes::Count::ZeroOrOne:
case IterTypes::Count::Any:
taken();
fallthrough();
break;
}
}
void in(ISS& env, const bc::MIterInit& op) {
@@ -2554,17 +2574,38 @@ void in(ISS& env, const bc::MIterInit& op) {
void in(ISS& env, const bc::IterInitK& op) {
auto const t1 = popC(env);
freeIter(env, op.iter1);
env.propagate(op.target, env.state);
if (t1.subtypeOfAny(TArrE, TVecE, TDictE, TKeysetE)) {
nothrow(env);
jmp_nofallthrough(env);
return;
}
auto ity = iter_types(t1);
setLoc(env, op.loc3, ity.second);
setLoc(env, op.loc4, ity.first);
setIter(env, op.iter1, TrackedIter { std::move(ity) });
if (!ity.mayThrowOnInit) nothrow(env);
auto const taken = [&]{
freeIter(env, op.iter1);
env.propagate(op.target, env.state);
};
auto const fallthrough = [&]{
setLoc(env, op.loc3, ity.value);
setLoc(env, op.loc4, ity.key);
setIter(env, op.iter1, TrackedIter { std::move(ity) });
};
switch (ity.count) {
case IterTypes::Count::Empty:
taken();
mayReadLocal(env, op.loc3);
mayReadLocal(env, op.loc4);
jmp_nofallthrough(env);
break;
case IterTypes::Count::Single:
case IterTypes::Count::NonEmpty:
fallthrough();
jmp_nevertaken(env);
break;
case IterTypes::Count::ZeroOrOne:
case IterTypes::Count::Any:
taken();
fallthrough();
break;
}
}
void in(ISS& env, const bc::MIterInitK& op) {
@@ -2594,19 +2635,38 @@ void in(ISS& env, const bc::WIterInitK& op) {
void in(ISS& env, const bc::IterNext& op) {
auto const curLoc3 = locRaw(env, op.loc3);
match<void>(
auto const noTaken = match<bool>(
env.state.iters[op.iter1],
[&] (UnknownIter) {
setLoc(env, op.loc3, TInitCell);
return false;
},
[&] (const TrackedIter& ti) {
setLoc(env, op.loc3, ti.kv.second);
if (!ti.types.mayThrowOnNext) nothrow(env);
switch (ti.types.count) {
case IterTypes::Count::Single:
case IterTypes::Count::ZeroOrOne:
return true;
case IterTypes::Count::NonEmpty:
case IterTypes::Count::Any:
setLoc(env, op.loc3, ti.types.value);
return false;
case IterTypes::Count::Empty:
always_assert(false);
}
not_reached();
}
);
if (noTaken) {
jmp_nevertaken(env);
freeIter(env, op.iter1);
return;
}
env.propagate(op.target, env.state);
freeIter(env, op.iter1);
if (curLoc3.subtypeOf(TInitCell)) setLocRaw(env, op.loc3, curLoc3);
setLocRaw(env, op.loc3, curLoc3);
}
void in(ISS& env, const bc::MIterNext& op) {
@@ -2619,22 +2679,41 @@ void in(ISS& env, const bc::IterNextK& op) {
auto const curLoc3 = locRaw(env, op.loc3);
auto const curLoc4 = locRaw(env, op.loc4);
match<void>(
auto const noTaken = match<bool>(
env.state.iters[op.iter1],
[&] (UnknownIter) {
[&] (UnknownIter) {
setLoc(env, op.loc3, TInitCell);
setLoc(env, op.loc4, TInitCell);
return false;
},
[&] (const TrackedIter& ti) {
setLoc(env, op.loc3, ti.kv.second);
setLoc(env, op.loc4, ti.kv.first);
if (!ti.types.mayThrowOnNext) nothrow(env);
switch (ti.types.count) {
case IterTypes::Count::Single:
case IterTypes::Count::ZeroOrOne:
return true;
case IterTypes::Count::NonEmpty:
case IterTypes::Count::Any:
setLoc(env, op.loc3, ti.types.value);
setLoc(env, op.loc4, ti.types.key);
return false;
case IterTypes::Count::Empty:
always_assert(false);
}
not_reached();
}
);
if (noTaken) {
jmp_nevertaken(env);
freeIter(env, op.iter1);
return;
}
env.propagate(op.target, env.state);
freeIter(env, op.iter1);
if (curLoc3.subtypeOf(TInitCell)) setLocRaw(env, op.loc3, curLoc3);
if (curLoc4.subtypeOf(TInitCell)) setLocRaw(env, op.loc4, curLoc4);
setLocRaw(env, op.loc3, curLoc3);
setLocRaw(env, op.loc4, curLoc4);
}
void in(ISS& env, const bc::MIterNextK& op) {
View
@@ -484,6 +484,20 @@ borrowed_ptr<php::Block> make_block(FuncAnalysis& ainfo,
return blk;
}
borrowed_ptr<php::Block> make_fatal_block(FuncAnalysis& ainfo,
borrowed_ptr<const php::Block> srcBlk,
const State& state) {
auto blk = make_block(ainfo, srcBlk, state);
auto const srcLoc = srcBlk->hhbcs.back().srcLoc;
blk->hhbcs = {
bc_with_loc(srcLoc, bc::String { s_unreachable.get() }),
bc_with_loc(srcLoc, bc::Fatal { FatalOp::Runtime })
};
blk->fallthrough = NoBlockId;
blk->exnNode = nullptr;
return blk;
}
void first_pass(const Index& index,
FuncAnalysis& ainfo,
borrowed_ptr<php::Block> const blk,
@@ -568,32 +582,109 @@ void first_pass(const Index& index,
break;
}
if (options.RemoveDeadBlocks &&
flags.jmpFlag != StepFlags::JmpFlags::Either) {
always_assert(!flags.wasPEI);
if (options.RemoveDeadBlocks) {
switch (flags.jmpFlag) {
case StepFlags::JmpFlags::Taken:
switch (op.op) {
case Op::JmpNZ: blk->fallthrough = op.JmpNZ.target; break;
case Op::JmpZ: blk->fallthrough = op.JmpZ.target; break;
case Op::IterInit: blk->fallthrough = op.IterInit.target; break;
case Op::IterInitK: blk->fallthrough = op.IterInitK.target; break;
/*
* For jumps, we need to pop the cell that was on the stack for the
* conditional jump. Note: for jumps this also conceptually
* needs to execute any side effects a conversion to bool can
* have. (Currently that is none.)
*/
case Op::JmpNZ:
always_assert(!flags.wasPEI);
blk->fallthrough = op.JmpNZ.target;
gen(bc::PopC {});
continue;
case Op::JmpZ:
always_assert(!flags.wasPEI);
blk->fallthrough = op.JmpZ.target;
gen(bc::PopC {});
continue;
case Op::IterInit:
/*
* For iterators, if we'll always take the taken branch (which means
* there's nothing to iterate over), and the op cannot raise an
* exception, we can just pop the input and set the fall-through to
* the taken branch. If not, we have to keep the op, but we can make
* sure we'll fatal if we ever actually take the fall-through.
*/
if (!flags.wasPEI) {
blk->fallthrough = op.IterInit.target;
gen(bc::PopC {});
continue;
} else {
blk->fallthrough = make_fatal_block(ainfo, blk, state)->id;
break;
}
case Op::IterInitK:
if (!flags.wasPEI) {
blk->fallthrough = op.IterInitK.target;
gen(bc::PopC {});
continue;
} else {
blk->fallthrough = make_fatal_block(ainfo, blk, state)->id;
break;
}
default:
// No support for switch, etc, right now.
always_assert(0 && "unsupported tookBranch case");
}
// fall through to PopC
break;
case StepFlags::JmpFlags::Fallthrough:
/*
* We need to pop the cell that was on the stack for the
* conditional jump. Note: for jumps this also conceptually
* needs to execute any side effects a conversion to bool can
* have. (Currently that is none.)
*/
gen(bc::PopC {});
continue;
switch (op.op) {
case Op::JmpNZ:
case Op::JmpZ:
/*
* Same as the taken case for jumps, except the fall-through is
* already set, so we can just turn them into pops.
*/
always_assert(!flags.wasPEI);
gen(bc::PopC {});
continue;
case Op::IterInit:
/*
* We can't ever optimize away iteration initialization if we know
* we'll always fall-through (which means we enter the loop) because
* we need to initialize the iterator. We can ensure, however, that
* the taken branch is a fatal.
*/
op.IterInit.target = make_fatal_block(ainfo, blk, state)->id;
break;
case Op::IterInitK:
op.IterInitK.target = make_fatal_block(ainfo, blk, state)->id;
break;
case Op::IterNext:
/*
* If we're nexting an iterator and we know we'll always fall-through
* (which means the iteration is over), and we can't raise an
* exception when nexting the iterator, we can just free the iterator
* and let it fall-through. If not, we can at least ensure the taken
* branch is a fatal.
*/
if (!flags.wasPEI) {
gen(bc::IterFree { op.IterNext.iter1 });
continue;
} else {
op.IterNext.target = make_fatal_block(ainfo, blk, state)->id;
break;
}
case Op::IterNextK:
if (!flags.wasPEI) {
gen(bc::IterFree { op.IterNextK.iter1 });
continue;
} else {
op.IterNextK.target = make_fatal_block(ainfo, blk, state)->id;
break;
}
default:
// No support for switch, etc, right now.
always_assert(0 && "unsupported fallthrough case");
}
break;
case StepFlags::JmpFlags::Either:
not_reached();
break;
}
}
Oops, something went wrong.

0 comments on commit 311faed

Please sign in to comment.