Skip to content

[isolation-regions] Eliminate unnecessary heap allocations used for temporary values #68712

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

Merged
merged 3 commits into from
Sep 28, 2023
Merged

[isolation-regions] Eliminate unnecessary heap allocations used for temporary values #68712

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
63db040
[isolation-regions] Eliminate all heap allocations from the main tran…
gottesmm Sep 18, 2023
a9e8baa
[isolation-regions] Eliminate last temporary std::vector usage from t…
gottesmm Sep 19, 2023
8d46cc8
[isolation-regions] Rename Consume to Transfer.
gottesmm Sep 21, 2023
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
36 changes: 17 additions & 19 deletions include/swift/SILOptimizer/Utils/PartitionUtils.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,25 +44,25 @@ struct Region {

using namespace PartitionPrimitives;

// PartitionOpKind represents the different kinds of PartitionOps that
// SILInstructions can be translated to
/// PartitionOpKind represents the different kinds of PartitionOps that
/// SILInstructions can be translated to
Copy link
Contributor

Choose a reason for hiding this comment

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

It might help if the comments for each case gave an example of an actual instruction that lowers to this operation?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'll add that in a follow on.

enum class PartitionOpKind : uint8_t {
// Assign one value to the region of another, takes two args, second arg
// must already be tracked with a non-consumed region
/// Assign one value to the region of another, takes two args, second arg
/// must already be tracked with a non-consumed region
Assign,

// Assign one value to a fresh region, takes one arg.
/// Assign one value to a fresh region, takes one arg.
AssignFresh,

// Consume the region of a value if not already consumed, takes one arg.
Consume,

// Merge the regions of two values, takes two args, both must be from
// non-consumed regions.
/// Merge the regions of two values, takes two args, both must be from
/// non-consumed regions.
Merge,

// Require the region of a value to be non-consumed, takes one arg.
Require
/// Consume the region of a value if not already consumed, takes one arg.
Transfer,

/// Require the region of a value to be non-consumed, takes one arg.
Require,
};

// PartitionOp represents a primitive operation that can be performed on
Expand Down Expand Up @@ -108,11 +108,9 @@ class PartitionOp {
return PartitionOp(PartitionOpKind::AssignFresh, tgt, sourceInst);
}

static PartitionOp Consume(Element tgt,
SILInstruction *sourceInst = nullptr,
Expr *sourceExpr = nullptr) {
return PartitionOp(PartitionOpKind::Consume, tgt,
sourceInst, sourceExpr);
static PartitionOp Transfer(Element tgt, SILInstruction *sourceInst = nullptr,
Expr *sourceExpr = nullptr) {
return PartitionOp(PartitionOpKind::Transfer, tgt, sourceInst, sourceExpr);
}

static PartitionOp Merge(Element tgt1, Element tgt2,
Expand Down Expand Up @@ -164,7 +162,7 @@ class PartitionOp {
case PartitionOpKind::AssignFresh:
os << "assign_fresh %%" << OpArgs[0] << "\n";
break;
case PartitionOpKind::Consume:
case PartitionOpKind::Transfer:
os << "consume %%" << OpArgs[0] << "\n";
break;
case PartitionOpKind::Merge:
Expand Down Expand Up @@ -425,7 +423,7 @@ class Partition {
fresh_label = Region(fresh_label + 1);
canonical = false;
break;
case PartitionOpKind::Consume:
case PartitionOpKind::Transfer:
assert(op.OpArgs.size() == 1 &&
"Consume PartitionOp should be passed 1 argument");
assert(labels.count(op.OpArgs[0]) &&
Expand Down
Loading