Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

remove --backend-aggressive-opts and all the related code #7

Merged
merged 1 commit into from
Aug 3, 2024
Merged

remove --backend-aggressive-opts and all the related code #7

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 0 additions & 3 deletions include/retdec/config/parameters.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,6 @@ class Parameters
bool isBackendNoOpts() const;
bool isBackendEmitCfg() const;
bool isBackendEmitCg() const;
bool isBackendAggressiveOpts() const;
bool isBackendKeepAllBrackets() const;
bool isBackendKeepLibraryFuncs() const;
bool isBackendNoTimeVaryingInfo() const;
Expand Down Expand Up @@ -76,7 +75,6 @@ class Parameters
void setIsBackendNoOpts(bool b);
void setIsBackendEmitCfg(bool b);
void setIsBackendEmitCg(bool b);
void setIsBackendAggressiveOpts(bool b);
void setIsBackendKeepAllBrackets(bool b);
void setIsBackendKeepLibraryFuncs(bool b);
void setIsBackendNoTimeVaryingInfo(bool b);
Expand Down Expand Up @@ -176,7 +174,6 @@ class Parameters
bool _backendNoOpts = false;
bool _backendEmitCfg = false;
bool _backendEmitCg = false;
bool _backendAggressiveOpts = false;
bool _backendKeepAllBrackets = false;
bool _backendKeepLibraryFuncs = false;
bool _backendNoTimeVaryingInfo = false;
Expand Down
1 change: 0 additions & 1 deletion include/retdec/llvmir2hll/llvmir2hll.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -138,7 +138,6 @@ class LlvmIr2Hll: public llvm::ModulePass

llvmir2hll::StringSet parseListOfOpts(
const std::string &opts) const;
std::string getTypeOfRunOptimizations() const;
llvmir2hll::StringVector getIdsOfPatternFindersToBeRun() const;
llvmir2hll::PatternFinderRunner::PatternFinders instantiatePatternFinders(
const llvmir2hll::StringVector &pfsIds);
Expand Down
5 changes: 1 addition & 4 deletions include/retdec/llvmir2hll/optimizer/optimizer_manager.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ class OptimizerManager final: private retdec::utils::NonCopyable {
OptimizerManager(const StringSet &enabledOpts, const StringSet &disabledOpts,
ShPtr<HLLWriter> hllWriter, ShPtr<ValueAnalysis> va,
ShPtr<CallInfoObtainer> cio, ShPtr<ArithmExprEvaluator> arithmExprEvaluator,
bool enableAggressiveOpts, bool enableDebug = false);
bool enableDebug = false);

void optimize(ShPtr<Module> m);

Expand Down Expand Up @@ -64,9 +64,6 @@ class OptimizerManager final: private retdec::utils::NonCopyable {
/// Used evaluator of arithmetical expressions.
ShPtr<ArithmExprEvaluator> arithmExprEvaluator;

/// Enable aggressive optimizations?
bool enableAggressiveOpts;

/// Enable emission of debug messages?
bool enableDebug;

Expand Down
58 changes: 0 additions & 58 deletions include/retdec/llvmir2hll/optimizer/optimizers/aggressive_deref_optimizer.h
View file
Open in desktop

This file was deleted.

44 changes: 0 additions & 44 deletions include/retdec/llvmir2hll/optimizer/optimizers/aggressive_global_to_local_optimizer.h
View file
Open in desktop

This file was deleted.

13 changes: 0 additions & 13 deletions src/config/parameters.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,6 @@ const std::string JSON_backendVarRenamer = "backendVarRenamer";
const std::string JSON_backendNoOpts = "backendNoOpts";
const std::string JSON_backendEmitCfg = "backendEmitCfg";
const std::string JSON_backendEmitCg = "backendEmitCg";
const std::string JSON_backendAggressiveOpts = "backendAggressiveOpts";
const std::string JSON_backendKeepAllBrackets = "backendKeepAllBrackets";
const std::string JSON_backendKeepLibraryFuncs = "backendKeepLibraryFuncs";
const std::string JSON_backendNoTimeVaryingInfo = "backendNoTimeVaryingInfo";
Expand Down Expand Up @@ -122,11 +121,6 @@ bool Parameters::isBackendEmitCg() const
return _backendEmitCg;
}

bool Parameters::isBackendAggressiveOpts() const
{
return _backendAggressiveOpts;
}

bool Parameters::isBackendKeepAllBrackets() const
{
return _backendKeepAllBrackets;
Expand Down Expand Up @@ -306,11 +300,6 @@ void Parameters::setIsBackendEmitCg(bool b)
_backendEmitCg = b;
}

void Parameters::setIsBackendAggressiveOpts(bool b)
{
_backendAggressiveOpts = b;
}

void Parameters::setIsBackendKeepAllBrackets(bool b)
{
_backendKeepAllBrackets = b;
Expand Down Expand Up @@ -519,7 +508,6 @@ void Parameters::serialize(Writer& writer) const
serdes::serializeBool(writer, JSON_backendEmitCfg, isBackendEmitCfg());
serdes::serializeBool(writer, JSON_backendEmitCg, isBackendEmitCg());
serdes::serializeBool(writer, JSON_detectStaticCode, isDetectStaticCode());
serdes::serializeBool(writer, JSON_backendAggressiveOpts, isBackendAggressiveOpts());
serdes::serializeBool(writer, JSON_backendKeepAllBrackets, isBackendKeepAllBrackets());
serdes::serializeBool(writer, JSON_backendKeepLibraryFuncs, isBackendKeepLibraryFuncs());
serdes::serializeBool(writer, JSON_backendNoTimeVaryingInfo, isBackendNoTimeVaryingInfo());
Comment on lines 508 to 513
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The serdes::serializeBool function calls are repeated multiple times with similar parameters. This repetition can lead to code bloat and makes the code harder to maintain. Consider refactoring these calls into a loop or a helper function to improve maintainability and reduce redundancy.

Expand Down Expand Up @@ -588,7 +576,6 @@ void Parameters::deserialize(const rapidjson::Value& val)
setIsBackendNoOpts( serdes::deserializeBool(val, JSON_backendNoOpts, false) );
setIsBackendEmitCfg( serdes::deserializeBool(val, JSON_backendEmitCfg, false) );
setIsBackendEmitCg( serdes::deserializeBool(val, JSON_backendEmitCg, false) );
setIsBackendAggressiveOpts( serdes::deserializeBool(val, JSON_backendAggressiveOpts, false) );
setIsBackendKeepAllBrackets( serdes::deserializeBool(val, JSON_backendKeepAllBrackets, false) );
setIsBackendKeepLibraryFuncs( serdes::deserializeBool(val, JSON_backendKeepLibraryFuncs, false) );
setIsBackendNoTimeVaryingInfo( serdes::deserializeBool(val, JSON_backendNoTimeVaryingInfo, false) );
Comment on lines 576 to 581
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The serdes::deserializeBool function calls are repeated multiple times with similar parameters. This repetition can lead to code bloat and makes the code harder to maintain. Consider refactoring these calls into a loop or a helper function to improve maintainability and reduce redundancy.

Expand Down
2 changes: 0 additions & 2 deletions src/llvmir2hll/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -152,8 +152,6 @@ add_library(llvmir2hll STATIC
optimizer/func_optimizer.cpp
optimizer/optimizer.cpp
optimizer/optimizer_manager.cpp
optimizer/optimizers/aggressive_deref_optimizer.cpp
optimizer/optimizers/aggressive_global_to_local_optimizer.cpp
optimizer/optimizers/bit_op_to_log_op_optimizer.cpp
optimizer/optimizers/bit_shift_optimizer.cpp
optimizer/optimizers/break_continue_return_optimizer.cpp
Expand Down
13 changes: 1 addition & 12 deletions src/llvmir2hll/llvmir2hll.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -195,7 +195,7 @@ bool LlvmIr2Hll::runOnModule(llvm::Module &m)
Log::phase("alias analysis [" + aliasAnalysis->getId() + "]");
initAliasAnalysis();

Log::phase("optimizations [" + getTypeOfRunOptimizations() + "]");
Log::phase("optimizations");
runOptimizations();
}
Comment on lines 195 to 200
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The initAliasAnalysis and runOptimizations functions are called without any error handling. If these functions can fail or throw exceptions, it would be prudent to handle such cases to ensure the program can either recover gracefully or provide meaningful error messages.

Recommended Solution: Implement error handling for initAliasAnalysis and runOptimizations to manage potential failures or exceptions.


Expand Down Expand Up @@ -603,7 +603,6 @@ void LlvmIr2Hll::runOptimizations()
llvmir2hll::ValueAnalysis::create(aliasAnalysis, true),
cio,
arithmExprEvaluator,
globalConfig->parameters.isBackendAggressiveOpts(),
Debug
)
);
Expand Down Expand Up @@ -844,16 +843,6 @@ retdec::llvmir2hll::StringSet LlvmIr2Hll::parseListOfOpts(
return llvmir2hll::StringSet(parsedOpts.begin(), parsedOpts.end());
}

/**
* @brief Returns the type of optimizations that should be run (as a string).
*/
std::string LlvmIr2Hll::getTypeOfRunOptimizations() const
{
return globalConfig->parameters.isBackendAggressiveOpts()
? "aggressive"
: "normal";
}

/**
* @brief Returns the IDs of pattern finders to be run.
*/
Expand Down
25 changes: 2 additions & 23 deletions src/llvmir2hll/optimizer/optimizer_manager.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,6 @@
#include "retdec/llvmir2hll/hll/hll_writer.h"
#include "retdec/llvmir2hll/obtainer/call_info_obtainer.h"
#include "retdec/llvmir2hll/optimizer/optimizer_manager.h"
#include "retdec/llvmir2hll/optimizer/optimizers/aggressive_deref_optimizer.h"
#include "retdec/llvmir2hll/optimizer/optimizers/aggressive_global_to_local_optimizer.h"
#include "retdec/llvmir2hll/optimizer/optimizers/bit_op_to_log_op_optimizer.h"
#include "retdec/llvmir2hll/optimizer/optimizers/bit_shift_optimizer.h"
#include "retdec/llvmir2hll/optimizer/optimizers/break_continue_return_optimizer.h"
Expand Down Expand Up @@ -65,9 +63,6 @@ namespace {
/// Suffix of all optimizers.
const std::string OPT_SUFFIX = "Optimizer";

/// Prefix of aggressive optimizations.
const std::string AGGRESSIVE_OPTS_PREFIX = "Aggressive";

/**
* @brief Trims the optional suffix "Optimizer" from all optimization names in
* @a opts.
Expand Down Expand Up @@ -101,7 +96,6 @@ StringSet trimOptimizerSuffix(const StringSet &opts) {
* @param[in] va Value analysis.
* @param[in] cio Call info obtainer.
* @param[in] arithmExprEvaluator Used evaluator of arithmetical expressions.
* @param[in] enableAggressiveOpts Enables aggressive optimizations.
* @param[in] enableDebug Enables emission of debug messages.
*
* To perform the actual optimizations, call optimize(). To get a list of
Expand All @@ -113,9 +107,6 @@ StringSet trimOptimizerSuffix(const StringSet &opts) {
* empty, also all optimizations are run. If an optimization is in both @a
* enabledOpts and @a disabledOpts, it is not run.
*
* Aggressive optimizations are run only if @a enableAggressiveOpts is @c true, or
* they are specified in @a enabledOpts.
*
* @a hllWriter, @a va, and @a cio are needed in some optimizations, so they
* have to be provided.
*
Expand All @@ -125,13 +116,12 @@ StringSet trimOptimizerSuffix(const StringSet &opts) {
OptimizerManager::OptimizerManager(const StringSet &enabledOpts,
const StringSet &disabledOpts, ShPtr<HLLWriter> hllWriter,
ShPtr<ValueAnalysis> va, ShPtr<CallInfoObtainer> cio,
ShPtr<ArithmExprEvaluator> arithmExprEvaluator,
bool enableAggressiveOpts, bool enableDebug):
ShPtr<ArithmExprEvaluator> arithmExprEvaluator, bool enableDebug):
enabledOpts(trimOptimizerSuffix(enabledOpts)),
disabledOpts(trimOptimizerSuffix(disabledOpts)),
hllWriter(hllWriter), va(va), cio(cio),
arithmExprEvaluator(arithmExprEvaluator),
enableAggressiveOpts(enableAggressiveOpts), enableDebug(enableDebug),
enableDebug(enableDebug),
recoverFromOutOfMemory(true), backendRunOpts() {
Comment on lines 116 to 125
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The constructor of OptimizerManager initializes several shared pointers (hllWriter, va, cio, arithmExprEvaluator) without checking if they are valid (non-null). Although PRECONDITION_NON_NULL is used, it is better to handle these cases explicitly to avoid potential null pointer dereferences.

Recommendation: Add explicit checks for the validity of these shared pointers and handle the error cases appropriately, possibly by throwing an exception or using an assertion.

PRECONDITION_NON_NULL(hllWriter);
PRECONDITION_NON_NULL(va);
Expand Down Expand Up @@ -162,12 +152,6 @@ void OptimizerManager::optimize(ShPtr<Module> m) {
run<GotoStmtOptimizer>(m);
run<RemoveUselessCastsOptimizer>(m);

// The first part of removal of non-compound statements. The other part
// should be run after structure optimizations because they may introduce
// constructs that can be optimized.
run<AggressiveDerefOptimizer>(m);
run<AggressiveGlobalToLocalOptimizer>(m);

// Data-flow optimizations.
// The following optimizations should be run before CopyPropagation to
// speed it up.
Expand Down Expand Up @@ -281,11 +265,6 @@ bool OptimizerManager::optShouldBeRun(const std::string &optId) const {
return true;
}

if (enabledOpts.empty() && startsWith(optId, AGGRESSIVE_OPTS_PREFIX)) {
// It is an aggressive optimization.
return enableAggressiveOpts;
}

return enabledOpts.empty();
}
Comment on lines 268 to 269
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The function isEnabled() returns true if enabledOpts is empty. This could lead to unintended behavior if the caller expects specific optimizations to be enabled but none are provided.

Recommendation: Consider adding a warning or error message when enabledOpts is empty to inform the user that no optimizations will be run. This can help in debugging and ensuring that the function behaves as expected.


Expand Down
75 changes: 0 additions & 75 deletions src/llvmir2hll/optimizer/optimizers/aggressive_deref_optimizer.cpp
View file
Open in desktop

This file was deleted.

Loading
Loading