Permalink
Browse files

[Perf] Better class constants

Summary:
If a class constant was defined in terms of something that was not itself a
scalar (eg another class constant), we immediately marked it dynamic, even
though the initializer might eventually be resolved to a scalar.

During optimization, we would only replace a class constant if the value was
scalar, when we can get better results by always replacing.

Type inference allways assumed Variant for dynamic constants, but thats not
necessary. eg "class X { const FOO=1; } class Y { const BAR=X::FOO; }". If X is
volatile, then Y::BAR is dynamic. But it is known to be of type int, and to have
the value 1. Its only dynamic because it might fatal when accessed (if X doesnt
exist).

Any class which contained at least one dynamic constant was marked as
lazy-init, which affected every use of every constant/static in the class.

Test Plan: fast_tests slow_tests

Reviewers: qigao, myang

Reviewed By: qigao

CC: ps, mwilliams, qigao

Differential Revision: 336136
  • Loading branch information...
1 parent e67357d commit 31e24e5ee6d1e0595181370b6417aed71ff846ae @markw65 markw65 committed with scottmac Sep 30, 2011
@@ -79,7 +79,7 @@ TypePtr ConstantTable::add(const std::string &name, TypePtr type,
}
void ConstantTable::setDynamic(AnalysisResultConstPtr ar,
- const std::string &name) {
+ const std::string &name, bool forceVariant) {
Symbol *sym = genSymbol(name, true);
if (!sym->isDynamic()) {
Lock lock(BlockScope::s_constMutex);
@@ -89,7 +89,9 @@ void ConstantTable::setDynamic(AnalysisResultConstPtr ar,
addUpdates(BlockScope::UseKindConstRef);
}
m_hasDynamic = true;
- setType(ar, sym, Type::Variant, true);
+ if (forceVariant) {
+ setType(ar, sym, Type::Variant, true);
+ }
}
}
@@ -256,8 +258,14 @@ void ConstantTable::getCPPDynamicDecl(CodeGenerator &cg,
BOOST_FOREACH(Symbol *sym, m_symbolVec) {
if (sym->declarationSet() && sym->isDynamic() &&
system == sym->isSystem()) {
- symbols.insert(string(prefix) + classId + fmt +
- CodeGenerator::FormatLabel(sym->getName()));
+ string tmp = string(prefix) + classId + fmt +
+ CodeGenerator::FormatLabel(sym->getName());
+ if (Type::IsMappedToVariant(sym->getFinalType())) {
+ symbols.insert(tmp);
+ } else {
+ type2names[sym->getFinalType()->getCPPDecl(ar, BlockScopeRawPtr())].
+ insert(tmp);
+ }
}
}
}
@@ -53,7 +53,8 @@ class ConstantTable : public SymbolTable {
/**
* Explicitly setting a constant to be dynamic, mainly for "Dynamic" note.
*/
- void setDynamic(AnalysisResultConstPtr ar, const std::string &name);
+ void setDynamic(AnalysisResultConstPtr ar, const std::string &name,
+ bool forceVariant);
/**
* Called when a constant is declared (l-value).
@@ -459,7 +459,7 @@ bool BuiltinSymbols::Load(AnalysisResultPtr ar, bool extOnly /* = false */) {
s_constants = ConstantTablePtr(new ConstantTable(*ar2.get()));
NoSuperGlobals = true;
}
- s_constants->setDynamic(ar, "SID");
+ s_constants->setDynamic(ar, "SID", true);
// load extension constants, classes and dynamics
ParseExtConsts(ar, ExtensionConsts, false);
@@ -123,15 +123,16 @@ void AssignmentExpression::analyzeProgram(AnalysisResultPtr ar) {
VariableTablePtr variables = getScope()->getVariables();
variables->addUsed(name);
}
+ } else if (ar->getPhase() == AnalysisResult::AnalyzeFinal) {
if (m_variable->is(Expression::KindOfConstantExpression)) {
ConstantExpressionPtr exp =
dynamic_pointer_cast<ConstantExpression>(m_variable);
if (!m_value->isScalar()) {
- getScope()->getConstants()->setDynamic(ar, exp->getName());
+ getScope()->getConstants()->setDynamic(ar, exp->getName(), false);
}
+ } else {
+ CheckNeeded(m_variable, m_value);
}
- } else if (ar->getPhase() == AnalysisResult::AnalyzeFinal) {
- CheckNeeded(m_variable, m_value);
}
}
@@ -40,13 +40,14 @@ ClassConstantExpression::ClassConstantExpression
: Expression(
EXPRESSION_CONSTRUCTOR_PARAMETER_VALUES(ClassConstantExpression)),
StaticClassName(classExp), m_varName(varName), m_defScope(NULL),
- m_valid(false) {
+ m_valid(false), m_depsSet(false) {
}
ExpressionPtr ClassConstantExpression::clone() {
ClassConstantExpressionPtr exp(new ClassConstantExpression(*this));
Expression::deepCopy(exp);
exp->m_class = Clone(m_class);
+ exp->m_depsSet = false;
return exp;
}
@@ -72,6 +73,13 @@ void ClassConstantExpression::analyzeProgram(AnalysisResultPtr ar) {
ConstructPtr decl = cls->getConstants()->
getValueRecur(ar, m_varName, cls);
cls->addUse(getScope(), BlockScope::UseKindConstRef);
+ m_depsSet = true;
+ if (ar->getPhase() == AnalysisResult::AnalyzeFinal) {
+ if (!isPresent()) {
+ getScope()->getVariables()->
+ setAttribute(VariableTable::NeedGlobalPointer);
+ }
+ }
}
addUserClass(ar, m_className);
}
@@ -122,7 +130,13 @@ ExpressionPtr ClassConstantExpression::preOptimize(AnalysisResultConstPtr ar) {
}
ClassScopePtr cls = resolveClass();
- if (!cls || (cls->isVolatile() && !isPresent())) return ExpressionPtr();
+ if (!cls || (cls->isVolatile() && !isPresent())) {
+ if (cls && !m_depsSet) {
+ cls->addUse(getScope(), BlockScope::UseKindConstRef);
+ m_depsSet = true;
+ }
+ return ExpressionPtr();
+ }
ConstantTablePtr constants = cls->getConstants();
ClassScopePtr defClass = cls;
@@ -131,18 +145,14 @@ ExpressionPtr ClassConstantExpression::preOptimize(AnalysisResultConstPtr ar) {
BlockScope::s_constMutex.lock();
ExpressionPtr value = dynamic_pointer_cast<Expression>(decl);
BlockScope::s_constMutex.unlock();
- if (value->isScalar()) {
- ExpressionPtr rep = Clone(value, getScope());
- bool annotate = Option::FlAnnotate;
- Option::FlAnnotate = false; // avoid nested comments on getText
- rep->setComment(getText());
- Option::FlAnnotate = annotate;
- rep->setLocation(getLocation());
- if (!value->is(KindOfScalarExpression)) {
- value->getScope()->addUse(getScope(), BlockScope::UseKindConstRef);
- }
- return replaceValue(rep);
- }
+
+ ExpressionPtr rep = Clone(value, getScope());
+ bool annotate = Option::FlAnnotate;
+ Option::FlAnnotate = false; // avoid nested comments on getText
+ rep->setComment(getText());
+ Option::FlAnnotate = annotate;
+ rep->setLocation(getLocation());
+ return replaceValue(rep);
}
return ExpressionPtr();
}
@@ -170,24 +180,9 @@ TypePtr ClassConstantExpression::inferTypes(AnalysisResultPtr ar,
ASSERT(cls);
ClassScopePtr defClass = cls;
- bool isDynamic = false;
- ConstructPtr decl;
- // we want to read dynamic-ness and declaration atomically
- if (getScope()->is(BlockScope::FunctionScope)) {
- GET_LOCK(cls);
- isDynamic = cls->getConstants()->isDynamic(m_varName);
- decl = cls->getConstants()->getDeclarationRecur(ar, m_varName, defClass);
- } else {
- ASSERT(getScope()->is(BlockScope::ClassScope));
- TRY_LOCK(cls);
- isDynamic = cls->getConstants()->isDynamic(m_varName);
- decl = cls->getConstants()->getDeclarationRecur(ar, m_varName, defClass);
- }
+ ConstructPtr decl =
+ cls->getConstants()->getDeclarationRecur(ar, m_varName, defClass);
- if (isDynamic || !isPresent()) {
- getScope()->getVariables()->
- setAttribute(VariableTable::NeedGlobalPointer);
- }
if (decl) { // No decl means an extension class or derived from redeclaring
cls = defClass;
m_valid = true;
@@ -50,6 +50,7 @@ class ClassConstantExpression : public Expression, public StaticClassName {
std::string m_varName;
BlockScope *m_defScope;
bool m_valid;
+ bool m_depsSet;
};
///////////////////////////////////////////////////////////////////////////////
@@ -42,12 +42,13 @@ ConstantExpression::ConstantExpression
const string &name, const string &docComment)
: Expression(EXPRESSION_CONSTRUCTOR_PARAMETER_VALUES(ConstantExpression)),
m_name(name), m_docComment(docComment),
- m_valid(false), m_dynamic(false), m_visited(false) {
+ m_valid(false), m_dynamic(false), m_visited(false), m_depsSet(false) {
}
ExpressionPtr ConstantExpression::clone() {
ConstantExpressionPtr exp(new ConstantExpression(*this));
Expression::deepCopy(exp);
+ m_depsSet = false;
return exp;
}
@@ -134,20 +135,9 @@ void ConstantExpression::analyzeProgram(AnalysisResultPtr ar) {
} else {
ConstructPtr decl = sym->getDeclaration();
if (decl) {
- if (!decl->getScope()) {
- /*
- this only happens if a define is parsed, but a
- later syntax error in the same file prevents
- completeScope being called on the scope containing
- the define.
- Might be better to catch this in the parser...
- */
- sym->setDeclaration(ExpressionPtr());
- sym->setValue(ExpressionPtr());
- } else {
- decl->getScope()->addUse(
- getScope(), BlockScope::UseKindConstRef);
- }
+ decl->getScope()->addUse(
+ getScope(), BlockScope::UseKindConstRef);
+ m_depsSet = true;
}
}
}
@@ -176,7 +166,14 @@ ExpressionPtr ConstantExpression::preOptimize(AnalysisResultConstPtr ar) {
ExpressionPtr value = dynamic_pointer_cast<Expression>(sym->getValue());
if (!sym->isSystem()) BlockScope::s_constMutex.unlock();
- if (!value || !value->isScalar()) break;
+ if (!value || !value->isScalar()) {
+ if (!m_depsSet && sym->getDeclaration()) {
+ sym->getDeclaration()->getScope()->addUse(
+ getScope(), BlockScope::UseKindConstRef);
+ m_depsSet = true;
+ }
+ break;
+ }
Variant scalarValue;
if (value->getScalarValue(scalarValue) &&
@@ -198,9 +195,6 @@ ExpressionPtr ConstantExpression::preOptimize(AnalysisResultConstPtr ar) {
rep->setComment(getText());
Option::FlAnnotate = annotate;
rep->setLocation(getLocation());
- if (!sym->isSystem() && !value->is(KindOfScalarExpression)) {
- value->getScope()->addUse(getScope(), BlockScope::UseKindConstRef);
- }
return replaceValue(rep);
}
@@ -63,10 +63,11 @@ class ConstantExpression : public Expression {
Symbol *resolveNS(AnalysisResultConstPtr ar);
std::string m_name;
std::string m_docComment;
+ std::string m_comment; // for inlined constant name
bool m_valid;
bool m_dynamic;
bool m_visited;
- std::string m_comment; // for inlined constant name
+ bool m_depsSet;
};
///////////////////////////////////////////////////////////////////////////////
@@ -154,7 +154,9 @@ class Expression : public Construct {
ExpressionPtr replaceValue(ExpressionPtr rep);
void clearContext();
int getContext() const { return m_context;}
- bool hasContext(Context context) const { return (m_context & context) == context; }
+ bool hasContext(Context context) const {
+ return (m_context & context) == context;
+ }
bool hasAnyContext(int context) const {
if ((context & Declaration) == Declaration) {
// special case Declaration because it is 2 bit fields
@@ -164,7 +166,9 @@ class Expression : public Construct {
}
return m_context & context;
}
- bool hasAllContext(int context) const { return (m_context & context) == context; }
+ bool hasAllContext(int context) const {
+ return (m_context & context) == context;
+ }
bool hasSubExpr(ExpressionPtr sub) const;
virtual void setComment(const std::string &) {}
/**
@@ -197,7 +201,8 @@ class Expression : public Construct {
void collectCPPTemps(ExpressionPtrVec &collection);
void disableCSE();
bool hasChainRoots();
- bool preOutputCPPTemp(CodeGenerator &cg, AnalysisResultPtr ar, bool emitTemps);
+ bool preOutputCPPTemp(CodeGenerator &cg, AnalysisResultPtr ar,
+ bool emitTemps);
virtual bool preOutputCPP(CodeGenerator &cg, AnalysisResultPtr ar,
int state);
bool preOutputOffsetLHS(CodeGenerator &cg, AnalysisResultPtr ar,
@@ -341,7 +341,7 @@ void SimpleFunctionCall::analyzeProgram(AnalysisResultPtr ar) {
constants = block->getConstants();
// set to be dynamic
if (m_type == DefinedFunction) {
- constants->setDynamic(ar, symbol);
+ constants->setDynamic(ar, symbol, true);
}
}
}
@@ -978,7 +978,7 @@ TypePtr SimpleFunctionCall::inferAndCheck(AnalysisResultPtr ar, TypePtr type,
if (constants != ar->getConstants()) {
TRY_LOCK(block);
if (value && !value->isScalar()) {
- constants->setDynamic(ar, varName);
+ constants->setDynamic(ar, varName, true);
varType = Type::Variant;
}
if (constants->isDynamic(varName)) {

0 comments on commit 31e24e5

Please sign in to comment.