Skip to content

S
swiftshader
Commit 80ee5b3f by Eric Holk
Options

Subzero WASM: avoid needless comparisons, add bounds check flag option

Introduces a new BooleanVariable type which represents zero-extended variables generated from an i1, saving a pointer to the original i1. The Wasm frontend uses this to avoid comparing against 0 if possible when translating branches. This led to about a 12% improvement on the bzip2 spec benchmark. This change also adds the -wasm-disable-bounds-check command line option which omits bounds checking code. BUG= https://bugs.chromium.org/p/nativeclient/issues/detail?id=4369 R=stichnot@chromium.org Review URL: https://codereview.chromium.org/1961583002 .
parent 7146e997
Showing with 84 additions and 21 deletions
src/IceClFlags.def
......@@ -345,7 +345,11 @@ struct dev_list_flag {};
\
X(VerboseFocusOnString, std::string, dev_opt_flag, "verbose-focus", \
cl::desc("Override with -verbose=none except for specified functions"), \
cl::init(":"))
cl::init(":")) \
\
X(WasmBoundsCheck, bool, dev_opt_flag, "wasm-bounds-check", \
cl::desc("Add bounds checking code in WASM frontend"), \
cl::init(true))
//#define X(Name, Type, ClType, ...)
} // end of namespace Ice
......
src/IceOperand.h
......@@ -51,6 +51,7 @@ public:
kConst_Max = kConst_Target + MaxTargetKinds,
kVariable,
kVariable64On32,
kVariableBoolean,
kVariable_Target, // leave space for target-specific variable kinds
kVariable_Max = kVariable_Target + MaxTargetKinds,
// Target-specific operand classes use kTarget as the starting point for
......@@ -95,6 +96,8 @@ public:
virtual ~Operand() = default;
virtual Variable *asBoolean() { return nullptr; }
protected:
Operand(OperandKind Kind, Type Ty) : Ty(Ty), Kind(Kind) {
// It is undefined behavior to have a larger value in the enum
......@@ -984,6 +987,35 @@ private:
const static InstDefList NoDefinitions;
};
/// BooleanVariable represents a variable that was the zero-extended result of a
/// comparison. It maintains a pointer to its original i1 source so that the
/// WASM frontend can avoid adding needless comparisons.
class BooleanVariable : public Variable {
BooleanVariable() = delete;
BooleanVariable(const BooleanVariable &) = delete;
BooleanVariable &operator=(const BooleanVariable &) = delete;
BooleanVariable(const Cfg *Func, OperandKind K, Type Ty, SizeT Index)
: Variable(Func, K, Ty, Index) {}
public:
static BooleanVariable *create(Cfg *Func, Type Ty, SizeT Index) {
return new (Func->allocate<BooleanVariable>())
BooleanVariable(Func, kVariable, Ty, Index);
}
virtual Variable *asBoolean() { return BoolSource; }
void setBoolSource(Variable *Src) { BoolSource = Src; }
static bool classof(const Operand *Operand) {
return Operand->getKind() == kVariableBoolean;
}
private:
Variable *BoolSource = nullptr;
};
} // end of namespace Ice
#endif // SUBZERO_SRC_ICEOPERAND_H
src/WasmTranslator.cpp
......@@ -49,7 +49,6 @@
using namespace std;
using namespace Ice;
using namespace v8;
using namespace v8::internal;
using namespace v8::internal::wasm;
using v8::internal::wasm::DecodeWasmModule;
......@@ -255,6 +254,7 @@ bool isComparison(wasm::WasmOpcode Opcode) {
class IceBuilder {
using Node = OperandNode;
using Variable = Ice::Variable;
IceBuilder() = delete;
IceBuilder(const IceBuilder &) = delete;
......@@ -393,8 +393,14 @@ public:
Node Binop(wasm::WasmOpcode Opcode, Node Left, Node Right) {
LOG(out << "Binop(" << WasmOpcodes::OpcodeName(Opcode) << ", " << Left
<< ", " << Right << ") = ");
auto *Dest = makeVariable(
isComparison(Opcode) ? IceType_i32 : Left.toOperand()->getType());
BooleanVariable *BoolDest = nullptr;
Variable *Dest = nullptr;
if (isComparison(Opcode)) {
BoolDest = makeVariable<BooleanVariable>(IceType_i32);
Dest = BoolDest;
} else {
Dest = makeVariable(Left.toOperand()->getType());
}
switch (Opcode) {
case kExprI32Add:
case kExprI64Add:
......@@ -538,6 +544,7 @@ public:
auto *TmpDest = makeVariable(IceType_i1);
Control()->appendInst(
InstIcmp::create(Func, InstIcmp::Ne, TmpDest, Left, Right));
BoolDest->setBoolSource(TmpDest);
Control()->appendInst(
InstCast::create(Func, InstCast::Zext, Dest, TmpDest));
break;
......@@ -547,6 +554,7 @@ public:
auto *TmpDest = makeVariable(IceType_i1);
Control()->appendInst(
InstIcmp::create(Func, InstIcmp::Eq, TmpDest, Left, Right));
BoolDest->setBoolSource(TmpDest);
Control()->appendInst(
InstCast::create(Func, InstCast::Zext, Dest, TmpDest));
break;
......@@ -556,6 +564,7 @@ public:
auto *TmpDest = makeVariable(IceType_i1);
Control()->appendInst(
InstIcmp::create(Func, InstIcmp::Slt, TmpDest, Left, Right));
BoolDest->setBoolSource(TmpDest);
Control()->appendInst(
InstCast::create(Func, InstCast::Zext, Dest, TmpDest));
break;
......@@ -565,6 +574,7 @@ public:
auto *TmpDest = makeVariable(IceType_i1);
Control()->appendInst(
InstIcmp::create(Func, InstIcmp::Sle, TmpDest, Left, Right));
BoolDest->setBoolSource(TmpDest);
Control()->appendInst(
InstCast::create(Func, InstCast::Zext, Dest, TmpDest));
break;
......@@ -574,6 +584,7 @@ public:
auto *TmpDest = makeVariable(IceType_i1);
Control()->appendInst(
InstIcmp::create(Func, InstIcmp::Uge, TmpDest, Left, Right));
BoolDest->setBoolSource(TmpDest);
Control()->appendInst(
InstCast::create(Func, InstCast::Zext, Dest, TmpDest));
break;
......@@ -583,6 +594,7 @@ public:
auto *TmpDest = makeVariable(IceType_i1);
Control()->appendInst(
InstIcmp::create(Func, InstIcmp::Ule, TmpDest, Left, Right));
BoolDest->setBoolSource(TmpDest);
Control()->appendInst(
InstCast::create(Func, InstCast::Zext, Dest, TmpDest));
break;
......@@ -592,6 +604,7 @@ public:
auto *TmpDest = makeVariable(IceType_i1);
Control()->appendInst(
InstIcmp::create(Func, InstIcmp::Ult, TmpDest, Left, Right));
BoolDest->setBoolSource(TmpDest);
Control()->appendInst(
InstCast::create(Func, InstCast::Zext, Dest, TmpDest));
break;
......@@ -601,6 +614,7 @@ public:
auto *TmpDest = makeVariable(IceType_i1);
Control()->appendInst(
InstIcmp::create(Func, InstIcmp::Sge, TmpDest, Left, Right));
BoolDest->setBoolSource(TmpDest);
Control()->appendInst(
InstCast::create(Func, InstCast::Zext, Dest, TmpDest));
break;
......@@ -610,6 +624,7 @@ public:
auto *TmpDest = makeVariable(IceType_i1);
Control()->appendInst(
InstIcmp::create(Func, InstIcmp::Sgt, TmpDest, Left, Right));
BoolDest->setBoolSource(TmpDest);
Control()->appendInst(
InstCast::create(Func, InstCast::Zext, Dest, TmpDest));
break;
......@@ -619,6 +634,7 @@ public:
auto *TmpDest = makeVariable(IceType_i1);
Control()->appendInst(
InstIcmp::create(Func, InstIcmp::Ugt, TmpDest, Left, Right));
BoolDest->setBoolSource(TmpDest);
Control()->appendInst(
InstCast::create(Func, InstCast::Zext, Dest, TmpDest));
break;
......@@ -628,6 +644,7 @@ public:
auto *TmpDest = makeVariable(IceType_i1);
Control()->appendInst(
InstFcmp::create(Func, InstFcmp::Ueq, TmpDest, Left, Right));
BoolDest->setBoolSource(TmpDest);
Control()->appendInst(
InstCast::create(Func, InstCast::Zext, Dest, TmpDest));
break;
......@@ -637,6 +654,7 @@ public:
auto *TmpDest = makeVariable(IceType_i1);
Control()->appendInst(
InstFcmp::create(Func, InstFcmp::Une, TmpDest, Left, Right));
BoolDest->setBoolSource(TmpDest);
Control()->appendInst(
InstCast::create(Func, InstCast::Zext, Dest, TmpDest));
break;
......@@ -646,6 +664,7 @@ public:
auto *TmpDest = makeVariable(IceType_i1);
Control()->appendInst(
InstFcmp::create(Func, InstFcmp::Ule, TmpDest, Left, Right));
BoolDest->setBoolSource(TmpDest);
Control()->appendInst(
InstCast::create(Func, InstCast::Zext, Dest, TmpDest));
break;
......@@ -655,6 +674,7 @@ public:
auto *TmpDest = makeVariable(IceType_i1);
Control()->appendInst(
InstFcmp::create(Func, InstFcmp::Ult, TmpDest, Left, Right));
BoolDest->setBoolSource(TmpDest);
Control()->appendInst(
InstCast::create(Func, InstCast::Zext, Dest, TmpDest));
break;
......@@ -664,6 +684,7 @@ public:
auto *TmpDest = makeVariable(IceType_i1);
Control()->appendInst(
InstFcmp::create(Func, InstFcmp::Uge, TmpDest, Left, Right));
BoolDest->setBoolSource(TmpDest);
Control()->appendInst(
InstCast::create(Func, InstCast::Zext, Dest, TmpDest));
break;
......@@ -673,6 +694,7 @@ public:
auto *TmpDest = makeVariable(IceType_i1);
Control()->appendInst(
InstFcmp::create(Func, InstFcmp::Ugt, TmpDest, Left, Right));
BoolDest->setBoolSource(TmpDest);
Control()->appendInst(
InstCast::create(Func, InstCast::Zext, Dest, TmpDest));
break;
......@@ -688,22 +710,26 @@ public:
Node Unop(wasm::WasmOpcode Opcode, Node Input) {
LOG(out << "Unop(" << WasmOpcodes::OpcodeName(Opcode) << ", " << Input
<< ") = ");
Ice::Variable *Dest = nullptr;
Variable *Dest = nullptr;
switch (Opcode) {
// TODO (eholk): merge these next two cases using getConstantInteger
case kExprI32Eqz: {
Dest = makeVariable(IceType_i32);
auto *BoolDest = makeVariable<BooleanVariable>(IceType_i32);
Dest = BoolDest;
auto *Tmp = makeVariable(IceType_i1);
Control()->appendInst(InstIcmp::create(Func, InstIcmp::Eq, Tmp, Input,
Ctx->getConstantInt32(0)));
BoolDest->setBoolSource(Tmp);
Control()->appendInst(InstCast::create(Func, InstCast::Zext, Dest, Tmp));
break;
}
case kExprI64Eqz: {
Dest = makeVariable(IceType_i32);
auto *BoolDest = makeVariable<BooleanVariable>(IceType_i32);
Dest = BoolDest;
auto *Tmp = makeVariable(IceType_i1);
Control()->appendInst(InstIcmp::create(Func, InstIcmp::Eq, Tmp, Input,
Ctx->getConstantInt64(0)));
BoolDest->setBoolSource(Tmp);
Control()->appendInst(InstCast::create(Func, InstCast::Zext, Dest, Tmp));
break;
}
......@@ -948,9 +974,12 @@ public:
LOG(out << *TrueNode << ", " << *FalseNode << ")"
<< "\n");
auto *CondBool = makeVariable(IceType_i1);
Ctrl->appendInst(InstIcmp::create(Func, InstIcmp::Ne, CondBool, Cond,
Ctx->getConstantInt32(0)));
auto *CondBool = Cond.toOperand()->asBoolean();
if (CondBool == nullptr) {
CondBool = makeVariable(IceType_i1);
Ctrl->appendInst(InstIcmp::create(Func, InstIcmp::Ne, CondBool, Cond,
Ctx->getConstantInt32(0)));
}
Ctrl->appendInst(InstBr::create(Func, CondBool, *TrueNode, *FalseNode));
return OperandNode(nullptr);
......@@ -1183,7 +1212,7 @@ public:
Control()->appendInst(InstLoad::create(Func, LoadResult, RealAddr));
// and cast, if needed
Ice::Variable *Result = nullptr;
Variable *Result = nullptr;
if (toIceType(Type) != toIceType(MemType)) {
auto DestType = toIceType(Type);
Result = makeVariable(DestType);
......@@ -1278,12 +1307,13 @@ private:
PhiMap.emplace(Op, Phi);
}
Ice::Variable *makeVariable(Ice::Type Type) {
return makeVariable(Type, Control());
template <typename T = Variable> T *makeVariable(Ice::Type Type) {
return makeVariable<T>(Type, Control());
}
Ice::Variable *makeVariable(Ice::Type Type, CfgNode *DefNode) {
auto *Var = Func->makeVariable(Type);
template <typename T = Variable>
T *makeVariable(Ice::Type Type, CfgNode *DefNode) {
auto *Var = Func->makeVariable<T>(Type);
DefNodeMap.emplace(Var, DefNode);
return Var;
}
......@@ -1360,12 +1390,9 @@ private:
Base = Addr;
}
// Do the bounds check.
//
// TODO (eholk): Add a command line argument to control whether bounds
// checks are inserted, and maybe add a way to duplicate bounds checks to
// get a better sense of the overhead.
if (!llvm::dyn_cast<ConstantInteger32>(Base)) {
// Do the bounds check if enabled
if (getFlags().getWasmBoundsCheck() &&
!llvm::isa<ConstantInteger32>(Base)) {
// TODO (eholk): creating a new basic block on every memory access is
// terrible (see https://goo.gl/Zj7DTr). Try adding a new instruction that
// encapsulates this "abort if false" pattern.
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment