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

[VAST] ExternPackageType & tighten space invariants. #1720

Merged
merged 2 commits into from
Nov 18, 2024
+339 −43
Merged

[VAST] ExternPackageType & tighten space invariants. #1720

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
ccbb8cc
[VAST] `ExternPackageType` & tighten space invariants.
cdleary Nov 15, 2024
e56d590
Handle missing test case for integer underlying an enum.
cdleary Nov 15, 2024
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
95 changes: 79 additions & 16 deletions xls/codegen/vast/vast.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,40 @@ std::string EmitNothing(const VastNode* node, LineInfo* line_info) {
return "";
}

// Helper for validating that there is not leading/trailing whitespace that can
// be stripped from string view `s` -- this helps us maintain our invariants
// that our Emit() calls generally do not return things with leading or
// trailing whitespace, so callers uniformly know what to expect in terms of
// spacing.
//
// This routine is made without performing string copies or performing a strcmp
// so should be usable in CHECKs instead of resorting to DCHECKs everywhere.
//
// i.e. callers are expected to call `CHECK(CannotStripWhitespace(s));`
bool CannotStripWhitespace(std::string_view s) {
std::string_view stripped = absl::StripAsciiWhitespace(s);
return stripped.size() == s.size();
}

// Helper that combines DataKind and DataType strings -- since either of these
// can be empty under certain circumstances we consolidate the handling of
// inserting spaces appropriately in this routine.
std::string CombineKindAndDataType(std::string_view kind_str,
std::string_view data_type_str) {
CHECK(CannotStripWhitespace(kind_str));
CHECK(CannotStripWhitespace(data_type_str));

std::string result;
if (kind_str.empty()) {
result = data_type_str;
} else if (data_type_str.empty()) {
result = kind_str;
} else {
result = absl::StrCat(kind_str, " ", data_type_str);
}
return result;
}

} // namespace

int Precedence(OperatorKind kind) {
Expand Down Expand Up @@ -301,14 +335,24 @@ std::string ToString(Direction direction) {
}
}

std::string DataType::EmitWithIdentifier(LineInfo* line_info,
std::string_view identifier) const {
std::string base = Emit(line_info);
if (base.empty()) {
return std::string{identifier};
}
CHECK(CannotStripWhitespace(base));
return absl::StrCat(base, " ", identifier);
}

std::string ScalarType::Emit(LineInfo* line_info) const {
// The `DataKind` preceding the type is enough.
if (!is_signed_) {
return EmitNothing(this, line_info);
}
LineInfoStart(line_info, this);
LineInfoEnd(line_info, this);
return " signed";
return "signed";
}

std::string IntegerType::Emit(LineInfo* line_info) const {
Expand All @@ -319,7 +363,7 @@ std::string IntegerType::Emit(LineInfo* line_info) const {
}
LineInfoStart(line_info, this);
LineInfoEnd(line_info, this);
return " unsigned";
return "unsigned";
}

std::string MacroRef::Emit(LineInfo* line_info) const {
Expand Down Expand Up @@ -500,17 +544,21 @@ LogicRef* VerilogFunction::return_value_ref() {

std::string VerilogFunction::Emit(LineInfo* line_info) const {
LineInfoStart(line_info, this);

// Construct the return type for the function, sometimes we want to put
// "logic" in front when in SV mode.
std::string return_type =
return_value_def_->data_type()->EmitWithIdentifier(line_info, name());
if (!absl::StartsWith(return_type, " ")) {
return_type = absl::StrCat(" ", return_type);
}
if (return_value_def_->data_type()->IsScalar() &&
file()->use_system_verilog()) {
// Preface the return type with "logic", so there's always a type provided.
return_type =
absl::StrCat(" ", DataKindToString(DataKind::kLogic), return_type);
absl::StrCat(DataKindToString(DataKind::kLogic), " ", return_type);
}
if (!return_type.empty()) {
return_type = absl::StrCat(" ", return_type);
}

std::string parameters =
absl::StrJoin(argument_defs_, ", ", [=](std::string* out, Def* d) {
absl::StrAppend(out, "input ", d->EmitNoSemi(line_info));
Expand Down Expand Up @@ -757,10 +805,11 @@ absl::StatusOr<int64_t> BitVectorType::FlatBitCountAsInt64() const {

std::string BitVectorType::Emit(LineInfo* line_info) const {
LineInfoStart(line_info, this);
std::string result =
absl::StrFormat("%s [%s:0]", is_signed_ ? " signed" : "",
size_expr_is_max_ ? size_expr_->Emit(line_info)
: WidthToLimit(line_info, size_expr_));
std::string result = is_signed_ ? "signed " : "";
absl::StrAppendFormat(&result, "[%s:0]",
size_expr_is_max_
? size_expr_->Emit(line_info)
: WidthToLimit(line_info, size_expr_));
LineInfoEnd(line_info, this);
return result;
}
Expand Down Expand Up @@ -891,8 +940,10 @@ std::string Def::Emit(LineInfo* line_info) const {
std::string Def::EmitNoSemi(LineInfo* line_info) const {
LineInfoStart(line_info, this);
std::string kind_str = DataKindToString(data_kind());
std::string result = absl::StrCat(
kind_str, data_type()->EmitWithIdentifier(line_info, GetName()));
std::string data_type_str =
data_type()->EmitWithIdentifier(line_info, GetName());
std::string result = CombineKindAndDataType(kind_str, data_type_str);

LineInfoEnd(line_info, this);
return result;
}
Expand Down Expand Up @@ -1106,8 +1157,10 @@ std::string Module::Emit(LineInfo* line_info) const {
absl::StrAppend(
&result,
absl::StrJoin(ports_, ",\n ", [=](std::string* out, const Port& port) {
std::string wire_str = port.wire->EmitNoSemi(line_info);
CHECK(CannotStripWhitespace(wire_str));
absl::StrAppendFormat(out, "%s %s", ToString(port.direction),
port.wire->EmitNoSemi(line_info));
wire_str);
LineInfoIncrease(line_info, 1);
}));
absl::StrAppend(&result, "\n);\n");
Expand Down Expand Up @@ -1302,7 +1355,14 @@ std::string TypedefType::Emit(LineInfo* line_info) const {

std::string ExternType::Emit(LineInfo* line_info) const {
LineInfoStart(line_info, this);
std::string result = absl::StrCat(" ", name_);
std::string result = name_;
LineInfoEnd(line_info, this);
return result;
}

std::string ExternPackageType::Emit(LineInfo* line_info) const {
LineInfoStart(line_info, this);
std::string result = absl::StrCat(package_name_, "::", type_name_);
LineInfoEnd(line_info, this);
return result;
}
Expand All @@ -1311,8 +1371,11 @@ std::string Enum::Emit(LineInfo* line_info) const {
LineInfoStart(line_info, this);
std::string result = "enum {\n";
if (kind_ != DataKind::kUntypedEnum) {
result = absl::StrFormat("enum %s%s {\n", DataKindToString(kind_),
BaseType()->Emit(line_info));
std::string kind_str = DataKindToString(kind_);
std::string data_type_str = BaseType()->Emit(line_info);
std::string underlying_type_str =
CombineKindAndDataType(kind_str, data_type_str);
result = absl::StrFormat("enum %s {\n", underlying_type_str);
}
LineInfoIncrease(line_info, 1);
for (int i = 0; i < members_.size(); i++) {
Expand Down
52 changes: 43 additions & 9 deletions xls/codegen/vast/vast.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -199,8 +199,8 @@ class DataType : public VastNode {
// Returns whether this is a scalar signal type (for example, "wire foo").
virtual bool IsScalar() const { return false; }

// Returns whether this type represents to a typedef, struct, enum, or an
// array of a user-defined type.
// Returns whether this type represents a typedef, struct, enum, or an array
// of a user-defined type.
virtual bool IsUserDefined() const { return false; }

// Returns the width of the def (not counting packed or unpacked dimensions)
Expand Down Expand Up @@ -233,9 +233,7 @@ class DataType : public VastNode {
// This method is required rather than simply Emit because an identifier
// string is nested within the string describing the type.
virtual std::string EmitWithIdentifier(LineInfo* line_info,
std::string_view identifier) const {
return absl::StrFormat("%s %s", Emit(line_info), identifier);
}
std::string_view identifier) const;
};

// Represents a scalar type. Example:
Expand Down Expand Up @@ -507,7 +505,7 @@ class UserDefinedDef final : public Def {
: Def(name, DataKind::kUser, data_type, init, file, loc) {}
};

// Register variable definition.Example:
// Register variable definition. Example:
// reg [41:0] foo;
class RegDef final : public Def {
public:
Expand All @@ -519,7 +517,7 @@ class RegDef final : public Def {
: Def(name, DataKind::kReg, data_type, init, file, loc) {}
};

// Logic variable definition.Example:
// Logic variable definition. Example:
// logic [41:0] foo;
class LogicDef final : public Def {
public:
Expand All @@ -543,7 +541,7 @@ class UserDef final : public Def {
: Def(name, DataKind::kUser, data_type, init, file, loc) {}
};

// Integer variable definition.Example:
// Integer variable definition. Example:
// integer foo;
class IntegerDef final : public Def {
public:
Expand Down Expand Up @@ -1163,6 +1161,37 @@ class Typedef final : public VastNode {
Def* def_;
};

// A type that is defined in an external package, where we may not know the
// underlying bit vector count as we request in `ExternType`.
class ExternPackageType : public DataType {
public:
explicit ExternPackageType(std::string_view package_name,
std::string_view type_name, VerilogFile* file,
const SourceInfo& loc)
: DataType(file, loc),
package_name_(package_name),
type_name_(type_name) {}

std::string Emit(LineInfo* line_info) const final;

bool IsScalar() const final { return false; }
bool IsUserDefined() const final { return true; }
absl::StatusOr<int64_t> WidthAsInt64() const final {
return absl::UnimplementedError(
"WidthAsInt64 is not implemented for ExternPackageType.");
}
absl::StatusOr<int64_t> FlatBitCountAsInt64() const final {
return absl::UnimplementedError(
"FlatBitCountAsInt64 is not implemented for ExternPackageType.");
}
std::optional<Expression*> width() const final { return std::nullopt; }
bool is_signed() const final { return false; }

private:
std::string package_name_;
std::string type_name_;
};

// The type of an entity when its type is a typedef. This emits just the name of
// the typedef.
class TypedefType final : public UserDefinedAliasType {
Expand Down Expand Up @@ -2299,7 +2328,12 @@ class VerilogPackageSection final : public VastNode {
std::vector<VerilogPackageMember> members_;
};

// Represents a package definition.
// Represents a package definition. Emits as:
// ```
// package ${name};
// .. content ..
// endpackage
// ```
class VerilogPackage final : public VastNode {
public:
VerilogPackage(std::string_view name, VerilogFile* file,
Expand Down
Loading
Loading