Skip to content

Commit

Permalink
Remove some trivial leaks in codebase (#5012)
Browse files Browse the repository at this point in the history 
* Remove trivial map leaks in LocalCopyPropagation

Signed-off-by: Anton Korobeynikov <[email protected]>

* Ensure we do not leak globals in control flow visitors

Signed-off-by: Anton Korobeynikov <[email protected]>

* Do not explicitly leak ID during parsing

Signed-off-by: Anton Korobeynikov <[email protected]>

* Do not leak cached locs in def_use

Signed-off-by: Anton Korobeynikov <[email protected]>

* Get rid of trivially dead code that only leaks

Signed-off-by: Anton Korobeynikov <[email protected]>

* Remove trivial leaks in StructInitializers

Signed-off-by: Anton Korobeynikov <[email protected]>

* Remove yet another set of useless leaks in frontend passes

Signed-off-by: Anton Korobeynikov <[email protected]>

* Remove some leaks from midend

Signed-off-by: Anton Korobeynikov <[email protected]>

* Remove more leaks from frontend

Signed-off-by: Anton Korobeynikov <[email protected]>

* Apply suggestions from code review

Co-authored-by: Vladimír Štill <[email protected]>
Signed-off-by: Anton Korobeynikov <[email protected]>

* Address review comments

Signed-off-by: Anton Korobeynikov <[email protected]>

---------

Signed-off-by: Anton Korobeynikov <[email protected]>
Co-authored-by: Vladimír Štill <[email protected]>
  • Loading branch information
@asl @vlstill
asl and vlstill authored Nov 13, 2024
1 parent 65b4daa commit 7348ad9
Show file tree
Hide file tree
Showing 30 changed files with 301 additions and 306 deletions.
30 changes: 14 additions & 16 deletions frontends/p4/def_use.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -779,15 +779,15 @@ std::size_t P4::loc_t::hash() const {
// In this case parentLoc is the loc of n's direct parent.
const P4::loc_t *ComputeWriteSet::getLoc(const IR::Node *n, const loc_t *parentLoc) {
loc_t tmp{n, parentLoc};
return &*cached_locs.insert(tmp).first;
return &*cached_locs->insert(tmp).first;
}

// Returns program location given the context of the currently being visited node.
// Use to get loc of currently being visited node.
const P4::loc_t *ComputeWriteSet::getLoc(const Visitor::Context *ctxt) {
if (!ctxt) return nullptr;
loc_t tmp{ctxt->node, getLoc(ctxt->parent)};
return &*cached_locs.insert(tmp).first;
return &*cached_locs->insert(tmp).first;
}

// Returns program location of a child node n, given the context of the
Expand All @@ -798,7 +798,7 @@ const P4::loc_t *ComputeWriteSet::getLoc(const IR::Node *n, const Visitor::Conte
if (p->node == n) return getLoc(p);
auto rv = getLoc(ctxt);
loc_t tmp{n, rv};
return &*cached_locs.insert(tmp).first;
return &*cached_locs->insert(tmp).first;
}

// Symbolic execution of the parser
Expand Down Expand Up @@ -1032,39 +1032,37 @@ bool ComputeWriteSet::preorder(const IR::P4Action *action) {
auto saveReturned = returnedDefinitions;
returnedDefinitions = new Definitions();

auto decls = new IR::IndexedVector<IR::Declaration>();
IR::IndexedVector<IR::Declaration> decls;
// We assume that there are no declarations in inner scopes
// inside the action body.
for (auto s : action->body->components) {
if (s->is<IR::Declaration>()) decls->push_back(s->to<IR::Declaration>());
if (s->is<IR::Declaration>()) decls.push_back(s->to<IR::Declaration>());
}
ProgramPoint pt(callingContext, action);
enterScope(action->parameters, decls, pt, false);
enterScope(action->parameters, &decls, pt, false);
visit(action->body);
currentDefinitions = currentDefinitions->joinDefinitions(returnedDefinitions);
setDefinitions(currentDefinitions, action->body, true); // overwrite
exitScope(action->parameters, decls, pt);
exitScope(action->parameters, &decls, pt);
returnedDefinitions = saveReturned;
return false;
}

namespace {
class GetDeclarations : public Inspector {
IR::IndexedVector<IR::Declaration> *declarations;
IR::IndexedVector<IR::Declaration> declarations;
bool preorder(const IR::Declaration_Variable *declaration) override {
declarations->push_back(declaration);
declarations.push_back(declaration);
return true;
}
bool preorder(const IR::Declaration_Instance *declaration) override {
declarations->push_back(declaration);
declarations.push_back(declaration);
return true;
}

public:
GetDeclarations() : declarations(new IR::IndexedVector<IR::Declaration>()) {
setName("GetDeclarations");
}
static IR::IndexedVector<IR::Declaration> *get(const IR::Node *node, const Visitor *calledBy) {
GetDeclarations() { setName("GetDeclarations"); }
static IR::IndexedVector<IR::Declaration> get(const IR::Node *node, const Visitor *calledBy) {
GetDeclarations gd;
gd.setCalledBy(calledBy);
(void)node->apply(gd);
Expand All @@ -1085,7 +1083,7 @@ bool ComputeWriteSet::preorder(const IR::Function *function) {
auto point = ProgramPoint(callingContext, function);
auto locals = GetDeclarations::get(function->body, this);
auto saveReturned = returnedDefinitions;
enterScope(function->type->parameters, locals, point, false);
enterScope(function->type->parameters, &locals, point, false);

returnedDefinitions = new Definitions();
visit(function->body);
Expand All @@ -1095,7 +1093,7 @@ bool ComputeWriteSet::preorder(const IR::Function *function) {
else
LOG3("CWS @" << point.after() << " with " << currentDefinitions->size() << " defs");
allDefinitions->setDefinitionsAt(point.after(), currentDefinitions, false);
exitScope(function->type->parameters, locals, point);
exitScope(function->type->parameters, &locals, point);

returnedDefinitions = saveReturned;
LOG3("Done " << dbp(function));
Expand Down
16 changes: 12 additions & 4 deletions frontends/p4/def_use.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ limitations under the License.

#include "absl/container/flat_hash_set.h"
#include "absl/container/inlined_vector.h"
#include "absl/container/node_hash_set.h"
#include "frontends/common/resolveReferences/referenceMap.h"
#include "ir/ir.h"
#include "lib/alloc_trace.h"
Expand Down Expand Up @@ -463,6 +464,11 @@ template <>
struct Hasher<P4::ProgramPoint> {
size_t operator()(const P4::ProgramPoint &p) const { return p.hash(); }
};

template <>
struct Hasher<P4::loc_t> {
size_t operator()(const P4::loc_t &loc) const { return loc.hash(); }
};
} // namespace P4::Util

namespace P4 {
Expand Down Expand Up @@ -605,6 +611,9 @@ class AllDefinitions : public IHasDbPrint {
*/

class ComputeWriteSet : public Inspector, public IHasDbPrint {
// We are using node_hash_set here as we need pointers to entries (loc_t) to be stable
using CachedLocs = absl::node_hash_set<loc_t, Util::Hash>;

public:
explicit ComputeWriteSet(AllDefinitions *allDefinitions, ReferenceMap *refMap, TypeMap *typeMap)
: refMap(refMap),
Expand All @@ -615,7 +624,7 @@ class ComputeWriteSet : public Inspector, public IHasDbPrint {
exitDefinitions(new Definitions()),
lhs(false),
virtualMethod(false),
cached_locs(*new std::unordered_set<loc_t>) {
cached_locs(new CachedLocs) {
CHECK_NULL(allDefinitions);
CHECK_NULL(refMap);
CHECK_NULL(typeMap);
Expand Down Expand Up @@ -689,7 +698,7 @@ class ComputeWriteSet : public Inspector, public IHasDbPrint {
/// Creates new visitor, but with same underlying data structures.
/// Needed to visit some program fragments repeatedly.
ComputeWriteSet(const ComputeWriteSet *source, ProgramPoint context, Definitions *definitions,
std::unordered_set<loc_t> &cached_locs)
std::shared_ptr<CachedLocs> cached_locs)
: refMap(source->refMap),
typeMap(source->typeMap),

Expand Down Expand Up @@ -769,8 +778,7 @@ class ComputeWriteSet : public Inspector, public IHasDbPrint {
}

private:
// TODO: Make absl::node_hash_set instead?
std::unordered_set<loc_t> &cached_locs;
std::shared_ptr<CachedLocs> cached_locs;
};

} // namespace P4
Expand Down
42 changes: 21 additions & 21 deletions frontends/p4/moveDeclarations.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,46 +21,46 @@ namespace P4 {
const IR::Node *MoveDeclarations::postorder(IR::P4Action *action) {
if (findContext<IR::P4Control>() == nullptr) {
// Else let the parent control get these
auto body = new IR::IndexedVector<IR::StatOrDecl>();
auto m = getMoves();
body->insert(body->end(), m->begin(), m->end());
body->append(action->body->components);
action->body =
new IR::BlockStatement(action->body->srcInfo, action->body->annotations, *body);
IR::IndexedVector<IR::StatOrDecl> body;
const auto &m = getMoves();
body.insert(body.end(), m.begin(), m.end());
body.append(action->body->components);
action->body = new IR::BlockStatement(action->body->srcInfo, action->body->annotations,
std::move(body));
pop();
}
return action;
}

const IR::Node *MoveDeclarations::postorder(IR::P4Control *control) {
LOG1("Visiting " << control << " toMove " << toMove.size());
auto decls = new IR::IndexedVector<IR::Declaration>();
for (auto decl : *getMoves()) {
IR::IndexedVector<IR::Declaration> decls;
for (auto decl : getMoves()) {
LOG1("Moved " << decl);
decls->push_back(decl);
decls.push_back(decl);
}
decls->append(control->controlLocals);
control->controlLocals = *decls;
decls.append(control->controlLocals);
control->controlLocals = std::move(decls);
pop();
return control;
}

const IR::Node *MoveDeclarations::postorder(IR::P4Parser *parser) {
auto newStateful = new IR::IndexedVector<IR::Declaration>();
newStateful->append(*getMoves());
newStateful->append(parser->parserLocals);
parser->parserLocals = *newStateful;
IR::IndexedVector<IR::Declaration> newStateful;
newStateful.append(getMoves());
newStateful.append(parser->parserLocals);
parser->parserLocals = std::move(newStateful);
pop();
return parser;
}

const IR::Node *MoveDeclarations::postorder(IR::Function *function) {
auto body = new IR::IndexedVector<IR::StatOrDecl>();
auto m = getMoves();
body->insert(body->end(), m->begin(), m->end());
body->append(function->body->components);
function->body =
new IR::BlockStatement(function->body->srcInfo, function->body->annotations, *body);
IR::IndexedVector<IR::StatOrDecl> body;
const auto &m = getMoves();
body.insert(body.end(), m.begin(), m.end());
body.append(function->body->components);
function->body = new IR::BlockStatement(function->body->srcInfo, function->body->annotations,
std::move(body));
pop();
return function;
}
Expand Down
13 changes: 9 additions & 4 deletions frontends/p4/moveDeclarations.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -36,17 +36,22 @@ class MoveDeclarations : public Transform {

/// List of lists of declarations to move, one list per
/// control/parser/action.
std::vector<IR::Vector<IR::Declaration> *> toMove;
void push() { toMove.push_back(new IR::Vector<IR::Declaration>()); }
// FIXME: Do we really need IR::Vector here?
std::vector<IR::Vector<IR::Declaration>> toMove;
void push() { toMove.emplace_back(); }
void pop() {
BUG_CHECK(!toMove.empty(), "Empty move stack");
toMove.pop_back();
}
IR::Vector<IR::Declaration> *getMoves() const {
const IR::Vector<IR::Declaration> &getMoves() const {
BUG_CHECK(!toMove.empty(), "Empty move stack");
return toMove.back();
}
void addMove(const IR::Declaration *decl) { getMoves()->push_back(decl); }
IR::Vector<IR::Declaration> &getMoves() {
BUG_CHECK(!toMove.empty(), "Empty move stack");
return toMove.back();
}
void addMove(const IR::Declaration *decl) { getMoves().push_back(decl); }

public:
explicit MoveDeclarations(bool parsersOnly = false) : parsersOnly(parsersOnly) {
Expand Down
32 changes: 12 additions & 20 deletions frontends/p4/parserControlFlow.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ const IR::Node *DoRemoveParserControlFlow::postorder(IR::ParserState *state) {
auto states = new IR::IndexedVector<IR::ParserState>();
IR::ParserState *currentState = state;
// components of the currentState
auto currentComponents = new IR::IndexedVector<IR::StatOrDecl>();
IR::IndexedVector<IR::StatOrDecl> currentComponents;
auto origComponents = state->components;
auto origSelect = state->selectExpression;

Expand All @@ -42,44 +42,36 @@ const IR::Node *DoRemoveParserControlFlow::postorder(IR::ParserState *state) {

// s_true
cstring trueName = nameGen.newName(state->name.name + "_true");
auto trueComponents = new IR::IndexedVector<IR::StatOrDecl>();
trueComponents->push_back(ifstat->ifTrue);
auto trueState = new IR::ParserState(trueName, *trueComponents,
auto trueState = new IR::ParserState(trueName, {ifstat->ifTrue},
new IR::PathExpression(IR::ID(joinName, nullptr)));
states->push_back(trueState);

// s_false
cstring falseName = joinName;
if (ifstat->ifFalse != nullptr) {
falseName = nameGen.newName(state->name.name + "_false");
auto falseComponents = new IR::IndexedVector<IR::StatOrDecl>();
falseComponents->push_back(ifstat->ifFalse);
auto falseState = new IR::ParserState(
falseName, *falseComponents, new IR::PathExpression(IR::ID(joinName, nullptr)));
auto falseState =
new IR::ParserState(falseName, {ifstat->ifFalse},
new IR::PathExpression(IR::ID(joinName, nullptr)));
states->push_back(falseState);
}

// left-over
auto vec = new IR::Vector<IR::Expression>();
vec->push_back(ifstat->condition);
auto trueCase = new IR::SelectCase(new IR::BoolLiteral(true),
new IR::PathExpression(IR::ID(trueName, nullptr)));
auto falseCase = new IR::SelectCase(new IR::BoolLiteral(false),
new IR::PathExpression(IR::ID(falseName, nullptr)));
auto cases = new IR::Vector<IR::SelectCase>();
cases->push_back(trueCase);
cases->push_back(falseCase);
currentState->selectExpression =
new IR::SelectExpression(new IR::ListExpression(*vec), std::move(*cases));

currentState->components = *currentComponents;
currentComponents = new IR::IndexedVector<IR::StatOrDecl>();
currentState->selectExpression = new IR::SelectExpression(
new IR::ListExpression({ifstat->condition}), {trueCase, falseCase});

currentState->components = std::move(currentComponents);
currentComponents.clear();
currentState = new IR::ParserState(joinName, origSelect); // may be overriten
} else {
currentComponents->push_back(c);
currentComponents.push_back(c);
}
}
currentState->components = *currentComponents;
currentState->components = std::move(currentComponents);

if (states->empty()) return state;
states->push_back(currentState);
Expand Down
34 changes: 17 additions & 17 deletions frontends/p4/removeParameters.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -89,15 +89,15 @@ class InsertBeforeExits : public Transform {
setName("InsertBeforeExits");
}
const IR::Node *postorder(IR::ReturnStatement *statement) override {
auto vec = new IR::IndexedVector<IR::StatOrDecl>(*toInsert);
vec->push_back(statement);
return new IR::BlockStatement(statement->srcInfo, *vec);
IR::IndexedVector<IR::StatOrDecl> vec(*toInsert);
vec.push_back(statement);
return new IR::BlockStatement(statement->srcInfo, std::move(vec));
}

const IR::Node *postorder(IR::ExitStatement *statement) override {
auto vec = new IR::IndexedVector<IR::StatOrDecl>(*toInsert);
vec->push_back(statement);
return new IR::BlockStatement(statement->srcInfo, *vec);
IR::IndexedVector<IR::StatOrDecl> vec(*toInsert);
vec.push_back(statement);
return new IR::BlockStatement(statement->srcInfo, std::move(vec));
}
};

Expand All @@ -108,9 +108,9 @@ const IR::Node *DoRemoveActionParameters::postorder(IR::P4Action *action) {
BUG_CHECK(getParent<IR::P4Control>() || getParent<IR::P4Program>(),
"%1%: unexpected parent %2%", getOriginal(), getContext()->node);
auto result = new IR::IndexedVector<IR::Declaration>();
auto leftParams = new IR::IndexedVector<IR::Parameter>();
auto body = new IR::IndexedVector<IR::StatOrDecl>();
auto postamble = new IR::IndexedVector<IR::StatOrDecl>();
IR::IndexedVector<IR::Parameter> leftParams;
IR::IndexedVector<IR::StatOrDecl> body;
IR::IndexedVector<IR::StatOrDecl> postamble;
auto invocation = invocations->get(getOriginal<IR::P4Action>());
if (invocation == nullptr) return action;
auto args = invocation->arguments;
Expand All @@ -121,7 +121,7 @@ const IR::Node *DoRemoveActionParameters::postorder(IR::P4Action *action) {
bool removeAll = invocations->removeAllParameters(getOriginal<IR::P4Action>());
for (auto p : action->parameters->parameters) {
if (p->direction == IR::Direction::None && !removeAll) {
leftParams->push_back(p);
leftParams.push_back(p);
} else {
auto decl =
new IR::Declaration_Variable(p->srcInfo, p->name, p->annotations, p->type, nullptr);
Expand All @@ -138,25 +138,25 @@ const IR::Node *DoRemoveActionParameters::postorder(IR::P4Action *action) {
p->direction == IR::Direction::None) {
auto left = new IR::PathExpression(p->name);
auto assign = new IR::AssignmentStatement(arg->srcInfo, left, arg->expression);
body->push_back(assign);
body.push_back(assign);
}
if (p->direction == IR::Direction::Out || p->direction == IR::Direction::InOut) {
auto right = new IR::PathExpression(p->name);
auto assign = new IR::AssignmentStatement(arg->srcInfo, arg->expression, right);
postamble->push_back(assign);
postamble.push_back(assign);
}
}
}
if (result->empty()) return action;

InsertBeforeExits ibf(postamble);
InsertBeforeExits ibf(&postamble);
ibf.setCalledBy(this);
auto actionBody = action->body->apply(ibf)->to<IR::BlockStatement>();
body->append(actionBody->components);
body->append(*postamble);
body.append(actionBody->components);
body.append(postamble);

action->parameters = new IR::ParameterList(action->parameters->srcInfo, *leftParams);
action->body = new IR::BlockStatement(action->body->srcInfo, *body);
action->parameters = new IR::ParameterList(action->parameters->srcInfo, std::move(leftParams));
action->body = new IR::BlockStatement(action->body->srcInfo, std::move(body));
LOG1("To replace " << dbp(action));
result->push_back(action);
return result;
Expand Down
Loading

0 comments on commit 7348ad9

Please sign in to comment.