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

Jump to bottom

Collection of minor tweaks to get approx. 10-15% compile time #4245

Merged
merged 1 commit into from
Aug 23, 2024
Merged

Collection of minor tweaks to get approx. 10-15% compile time #4245

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
7 changes: 4 additions & 3 deletions common/vlog.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,10 @@ namespace Carbon {
//
// For example:
// CARBON_VLOG() << "Verbose message";
#define CARBON_VLOG() \
(vlog_stream_ == nullptr) ? (void)0 \
: CARBON_VLOG_INTERNAL_STREAM(vlog_stream_)
#define CARBON_VLOG() \
__builtin_expect(vlog_stream_ == nullptr, true) \
Copy link
Contributor

Choose a reason for hiding this comment

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

Should CARBON_CHECK have a similar __builtin_expect((__VA_ARGS__), false) ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The noreturn provides some help there, but yeah, we can probably do more. Will send another PR.

? (void)0 \
: CARBON_VLOG_INTERNAL_STREAM(vlog_stream_)

} // namespace Carbon

Expand Down
4 changes: 2 additions & 2 deletions toolchain/base/value_store.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -165,13 +165,13 @@ class ValueStore

// Returns a mutable value for an ID.
auto Get(IdT id) -> RefType {
CARBON_CHECK(id.index >= 0) << id;
CARBON_DCHECK(id.index >= 0) << id;
return values_[id.index];
}

// Returns the value for an ID.
auto Get(IdT id) const -> ConstRefType {
CARBON_CHECK(id.index >= 0) << id;
CARBON_DCHECK(id.index >= 0) << id;
return values_[id.index];
}

Expand Down
10 changes: 0 additions & 10 deletions toolchain/parse/tree.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,16 +21,6 @@ auto Tree::postorder() const -> llvm::iterator_range<PostorderIterator> {
PostorderIterator(NodeId(node_impls_.size())));
}

auto Tree::node_has_error(NodeId n) const -> bool {
CARBON_CHECK(n.is_valid());
return node_impls_[n.index].has_error;
}

auto Tree::node_kind(NodeId n) const -> NodeKind {
CARBON_CHECK(n.is_valid());
return node_impls_[n.index].kind;
}

auto Tree::node_token(NodeId n) const -> Lex::TokenIndex {
CARBON_CHECK(n.is_valid());
return node_impls_[n.index].token;
Expand Down
10 changes: 8 additions & 2 deletions toolchain/parse/tree.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -115,10 +115,16 @@ class Tree : public Printable<Tree> {

// Tests whether a particular node contains an error and may not match the
// full expected structure of the grammar.
auto node_has_error(NodeId n) const -> bool;
auto node_has_error(NodeId n) const -> bool {
CARBON_DCHECK(n.is_valid());
return node_impls_[n.index].has_error;
}

// Returns the kind of the given parse tree node.
auto node_kind(NodeId n) const -> NodeKind;
auto node_kind(NodeId n) const -> NodeKind {
CARBON_DCHECK(n.is_valid());
return node_impls_[n.index].kind;
}

// Returns the token the given parse tree node models.
auto node_token(NodeId n) const -> Lex::TokenIndex;
Expand Down
4 changes: 2 additions & 2 deletions toolchain/sem_ir/constant.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ class ConstantValueStore {
// Returns the constant value of the requested instruction, which is default_
// if unallocated.
auto Get(InstId inst_id) const -> ConstantId {
CARBON_CHECK(inst_id.index >= 0);
CARBON_DCHECK(inst_id.index >= 0);
return static_cast<size_t>(inst_id.index) >= values_.size()
? default_
: values_[inst_id.index];
Expand All @@ -50,7 +50,7 @@ class ConstantValueStore {
// Sets the constant value of the given instruction, or sets that it is known
// to not be a constant.
auto Set(InstId inst_id, ConstantId const_id) -> void {
CARBON_CHECK(inst_id.index >= 0);
CARBON_DCHECK(inst_id.index >= 0);
if (static_cast<size_t>(inst_id.index) >= values_.size()) {
values_.resize(inst_id.index + 1, default_);
}
Expand Down
10 changes: 5 additions & 5 deletions toolchain/sem_ir/ids.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -127,17 +127,17 @@ struct ConstantId : public IdBase, public Printable<ConstantId> {

// Returns whether this represents a constant. Requires is_valid.
auto is_constant() const -> bool {
CARBON_CHECK(is_valid());
CARBON_DCHECK(is_valid());
return *this != ConstantId::NotConstant;
}
// Returns whether this represents a symbolic constant. Requires is_valid.
auto is_symbolic() const -> bool {
CARBON_CHECK(is_valid());
CARBON_DCHECK(is_valid());
return index <= FirstSymbolicIndex;
}
// Returns whether this represents a template constant. Requires is_valid.
auto is_template() const -> bool {
CARBON_CHECK(is_valid());
CARBON_DCHECK(is_valid());
return index >= 0;
}

Expand Down Expand Up @@ -174,14 +174,14 @@ struct ConstantId : public IdBase, public Printable<ConstantId> {
// Requires `is_template()`. Use `ConstantValueStore::GetInstId` to get the
// instruction ID of a `ConstantId`.
constexpr auto template_inst_id() const -> InstId {
CARBON_CHECK(is_template());
CARBON_DCHECK(is_template());
return InstId(index);
}

// Returns the symbolic constant index that describes this symbolic constant
// value. Requires `is_symbolic()`.
constexpr auto symbolic_index() const -> int32_t {
CARBON_CHECK(is_symbolic());
CARBON_DCHECK(is_symbolic());
return FirstSymbolicIndex - index;
}

Expand Down
Loading