Skip to content

Commit

Permalink
Merge branch 'master' of https://github.com/MikePopoloski/slang into …
Browse files Browse the repository at this point in the history
…dev/slang-tidy

* 'master' of https://github.com/MikePopoloski/slang:
  Update CHANGELOG.md
  [slang] Fix PLA task concatenation ascending order (MikePopoloski#1047)
  [slang] Fix PATHPULSE limit values (MikePopoloski#1048)
  Handle recursive parameter definitions via hierarchical reference
  Mark fmt lib headers as system headers to suppress warnings
  Bump dependency versions: fmt and pybind11
  [slang] Make string simple type (MikePopoloski#1049)
  Update README.md (MikePopoloski#1046)
  [slang][port] Fix bugs (MikePopoloski#1043)
  Use [[likely]] / [[unlikely]] replace macro SLANG_LIKELY/SLANG_UNLIKELY (MikePopoloski#1038)
  Add multiple slang-tidy checks and minor fixes in existing ones (MikePopoloski#1040)
  chore: update pre-commit hooks (MikePopoloski#1041)
  • Loading branch information
JoelSole-Semidyn committed Jul 12, 2024
2 parents dae9363 + 4dee9aa commit 3b79e10
Show file tree
Hide file tree
Showing 19 changed files with 139 additions and 45 deletions.
2 changes: 1 addition & 1 deletion .pre-commit-config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ repos:

# clang-format
- repo: https://github.com/pre-commit/mirrors-clang-format
rev: v18.1.7
rev: v18.1.8
hooks:
- id: clang-format
exclude: ^external/.*
18 changes: 17 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,9 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
* The error for invalid state-dependent path conditions in specify blocks can now be downgraded with -Wspecify-condition-expr for compatibility with other tools
* Added support for the optional system tasks and functions from Annex D in the LRM: `$countdrivers`, `$list`, `$input`, `$key`, `$nokey`, `$reset`, `$reset_count`, `$reset_value`, `$save`, `$incsave`, `$restart`, `$scope`, `$scale`, `$showscopes`, `$showvars`, `$sreadmemb`, `$sreadmemh`, `$getpattern`
* Added support for the optional compiler directives from Annex E in the LRM: `` `default_decay_time ``, `` `default_trireg_strength ``, `` `delay_mode_distributed ``, `` `delay_mode_path ``, `` `delay_mode_unit ``, `` `delay_mode_zero ``
* Rules about nondegeneracy of sequences and properties are now enforced (thanks to @likeamahoney)
* Special case rules about how name resolution works in bind directives are now enforced
* Changed the definition of "simple types" to include `string` to allow using it as a target for assignment pattern fields (thanks to @likeamahoney)
#### Clarifications in IEEE 1800-2023
* Assertion clocking events can't reference automatic variables
* The `.*` token sequence is actually two separate tokens that can be separated by whitespace
Expand Down Expand Up @@ -40,20 +43,33 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
* Added [-Wunused-import](https://sv-lang.com/warning-ref.html#unused-import) and [-Wunused-wildcard-import](https://sv-lang.com/warning-ref.html#unused-wildcard-import) which warn about unused import directives
* Added [-Warith-op-mismatch](https://sv-lang.com/warning-ref.html#arith-op-mismatch), [-Wbitwise-op-mismatch](https://sv-lang.com/warning-ref.html#bitwise-op-mismatch), [-Wcomparison-mismatch](https://sv-lang.com/warning-ref.html#comparison-mismatch), and [-Wsign-compare](https://sv-lang.com/warning-ref.html#sign-compare) which all warn about different cases of mismatched types in binary expressions
* slang-netlist has experimental support for detecting combinatorial loops (thanks to @udif)
* Added `--allow-merging-ansi-ports` (included in "vcs" compat mode) which allows non-standard behavior in which ANSI module ports can duplicate net and variables declared within the module
* Added `--ast-json-source-info` which includes source line information when dumping an AST to JSON (thanks to @KennethDRoe)
* Added `--enable-legacy-protect` which enables support for nonstandard / legacy protected envelopes: Verilog-XL style `` `protect `` directives and Verilog-A style `// pragma protect` comments

### Improvements
* Default value expressions for parameters that are overridden are now checked for basic correctness and other parameters they reference will not warn for being "unused"
* Made several minor improvements to the locations reported for propagated type conversion warnings
* Sped up `Compilation` object construction by reorganizing how system subroutines are created and registered
* Improved the parser error reported when encountering an extraneous end delimiter in a member list
* Various fixes and improvements were made to slang-netlist (thanks to @jameshanlon, @udif)
* Added options to slang-tidy to better control what gets output (thanks to @Sustrak)
* Added a bunch of new checks to slang-tidy (thanks to @JoelSole-Semidyn)

### Fixes
* Fixed several AST serialization methods (thanks to @tdp2110)
* Fixed several AST serialization methods (thanks to @tdp2110, @likeamahoney, @Kitaev2003)
* Fixed the return type of DPI import tasks
* Fixed a bug that caused some `inout` ports to warn as "unused"
* Fixed the checking of the `extends` override specifier when the containing class has no base class
* Fixed a case where bracketed delay expressions in sequence concatenations were not checked for correctness
* Fixed the type of the iterators used in with-expressions for covergroup bins
* Fixed a bug that sometimes prevented printing the correct scope for type alias names in diagnostic messages
* Fixed the hierarchical path string created for symbols inside of unnamed generate blocks (thanks to @povik)
* Fixed spurious errors that could occur when using generic class instantiations inside uninstantiated generate blocks
* Correctly disallow passing expressions of `void` type to format style system functions (thanks to @tdp2110)
* Fixed a bug where parameters that referred to themselves via hierarchical reference would cause a crash instead of reporting a diagnostic
* Fixed `PATHPULSE$` specparam initializers to allow providing only one value instead of requiring two (thanks to @likeamahoney)
* Fixed PLA tasks to accept concatenation expressions as arguments without reporting an error about the direction of the argument bounds (thanks to @likeamahoney)


## [v6.0] - 2024-04-21
Expand Down
4 changes: 2 additions & 2 deletions bindings/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -5,13 +5,13 @@

set(find_pkg_args "")
if(${CMAKE_VERSION} VERSION_GREATER_EQUAL "3.24.0")
set(find_pkg_args "FIND_PACKAGE_ARGS" "2.12.0")
set(find_pkg_args "FIND_PACKAGE_ARGS" "2.13.1")
endif()

FetchContent_Declare(
pybind11
GIT_REPOSITORY https://github.com/pybind/pybind11.git
GIT_TAG v2.12.0
GIT_TAG v2.13.1
GIT_SHALLOW ON
${find_pkg_args})
FetchContent_MakeAvailable(pybind11)
Expand Down
4 changes: 2 additions & 2 deletions conanfile.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,8 @@ class CompressorRecipe(ConanFile):
def requirements(self):
self.requires("mimalloc/2.1.7")
self.requires("catch2/3.6.0")
self.requires("pybind11/2.12.0")
self.requires("fmt/10.2.1")
self.requires("pybind11/2.13.1")
self.requires("fmt/11.0.1")

def layout(self):
self.folders.build_folder_vars = [
Expand Down
8 changes: 6 additions & 2 deletions external/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
# ~~~

# Required minimum versions for dependencies
set(fmt_min_version "10.2")
set(fmt_min_version "11.0")
set(mimalloc_min_version "2.1")
set(catch2_min_version "3.6")

Expand All @@ -14,10 +14,14 @@ if(${CMAKE_VERSION} VERSION_GREATER_EQUAL "3.24.0")
set(find_pkg_args "FIND_PACKAGE_ARGS" "${fmt_min_version}")
endif()

set(FMT_SYSTEM_HEADERS
ON
CACHE INTERNAL "")

FetchContent_Declare(
fmt
GIT_REPOSITORY https://github.com/fmtlib/fmt.git
GIT_TAG 10.2.1
GIT_TAG 11.0.1
GIT_SHALLOW ON
${find_pkg_args})

Expand Down
26 changes: 14 additions & 12 deletions include/slang/util/Hash.h
Original file line number Diff line number Diff line change
Expand Up @@ -104,12 +104,12 @@ static inline void mum(uint64_t* a, uint64_t* b) {
uint64_t seed = secret[0];
uint64_t a{};
uint64_t b{};
if (SLANG_LIKELY(len <= 16)) {
if (SLANG_LIKELY(len >= 4)) {
if (len <= 16) [[likely]] {
if (len >= 4) [[likely]] {
a = (r4(p) << 32U) | r4(p + ((len >> 3U) << 2U));
b = (r4(p + len - 4) << 32U) | r4(p + len - 4 - ((len >> 3U) << 2U));
}
else if (SLANG_LIKELY(len > 0)) {
else if (len > 0) [[likely]] {
a = r3(p, len);
b = 0;
}
Expand All @@ -120,19 +120,21 @@ static inline void mum(uint64_t* a, uint64_t* b) {
}
else {
size_t i = len;
if (SLANG_UNLIKELY(i > 48)) {
if (i > 48) [[unlikely]] {
uint64_t see1 = seed;
uint64_t see2 = seed;
do {
seed = mix(r8(p) ^ secret[1], r8(p + 8) ^ seed);
see1 = mix(r8(p + 16) ^ secret[2], r8(p + 24) ^ see1);
see2 = mix(r8(p + 32) ^ secret[3], r8(p + 40) ^ see2);
p += 48;
i -= 48;
} while (SLANG_LIKELY(i > 48));
do
[[likely]] {
seed = mix(r8(p) ^ secret[1], r8(p + 8) ^ seed);
see1 = mix(r8(p + 16) ^ secret[2], r8(p + 24) ^ see1);
see2 = mix(r8(p + 32) ^ secret[3], r8(p + 40) ^ see2);
p += 48;
i -= 48;
}
while (i > 48);
seed ^= see1 ^ see2;
}
while (SLANG_UNLIKELY(i > 16)) {
while (i > 16) [[unlikely]] {
seed = mix(r8(p) ^ secret[1], r8(p + 8) ^ seed);
i -= 16;
p += 16;
Expand Down
8 changes: 0 additions & 8 deletions include/slang/util/Util.h
Original file line number Diff line number Diff line change
Expand Up @@ -16,14 +16,6 @@
#include "slang/slang_export.h"
#include "slang/util/Enum.h"

#if defined(__GNUC__) || defined(__INTEL_COMPILER) || defined(__clang__)
# define SLANG_LIKELY(x) __builtin_expect(x, 1)
# define SLANG_UNLIKELY(x) __builtin_expect(x, 0)
#else
# define SLANG_LIKELY(x) (x)
# define SLANG_UNLIKELY(x) (x)
#endif

#if defined(__GNUC__) || defined(__clang__)
# define SLANG_ASSERT_FUNCTION __PRETTY_FUNCTION__
#elif defined(_MSC_VER)
Expand Down
1 change: 0 additions & 1 deletion scripts/diagnostics.txt
Original file line number Diff line number Diff line change
Expand Up @@ -262,7 +262,6 @@ error WrongSpecifyDelayCount "expected one, two, three, six, or twelve delay val
error IfNoneEdgeSensitive "ifnone specify path cannot be an edge-sensitive path"
error PulseControlSpecifyParent "pulse control specparams can only be declared in specify blocks"
error PulseControlPATHPULSE "pulse control specparam names must start with 'PATHPULSE$'"
error PulseControlTwoValues "pulse control specparams must be assigned two values (reject limit and error limit)"
error TooManyEdgeDescriptors "cannot have more than six pairs of edge transitions in list"
error EdgeDescWrongKeyword "edge descriptor list cannot be used after '{}'"
error BindDirectiveInvalidName "bind target name must be a simple module or interface identifier when a list of instances is provided"
Expand Down
13 changes: 13 additions & 0 deletions source/ast/Lookup.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -837,6 +837,19 @@ void unwrapResult(const Scope& scope, std::optional<SourceRange> range, LookupRe
if (!result.found)
return;

if (result.flags.has(LookupResultFlags::IsHierarchical)) {
auto declaredType = result.found->getDeclaredType();
if (declaredType && declaredType->isEvaluating()) {
if (range) {
auto& diag = result.addDiag(scope, diag::RecursiveDefinition, *range);
diag << result.found->name;
diag.addNote(diag::NoteDeclarationHere, result.found->location);
}
result.found = nullptr;
return;
}
}

checkVisibility(*result.found, scope, range, result);

// Unwrap type parameters into their target type alias.
Expand Down
6 changes: 4 additions & 2 deletions source/ast/builtins/SystemTasks.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -749,7 +749,8 @@ class PlaTask : public SystemTaskBase {
return badArg(context, *args[i]);
}

if (elemType.hasFixedRange() && !isValidRange(elemType)) {
if (elemType.hasFixedRange() && args[i]->kind != ExpressionKind::Concatenation &&
!isValidRange(elemType)) {
return badRange(context, *args[i]);
}
}
Expand All @@ -759,7 +760,8 @@ class PlaTask : public SystemTaskBase {
}
}

if (type.hasFixedRange() && !isValidRange(type)) {
if (type.hasFixedRange() && args[i]->kind != ExpressionKind::Concatenation &&
!isValidRange(type)) {
return badRange(context, *args[i]);
}
}
Expand Down
6 changes: 2 additions & 4 deletions source/ast/symbols/PortSymbols.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1981,10 +1981,8 @@ void PortConnection::checkSimulatedNetTypes() const {
}
}

if (in == internal.end() || ex == external.end()) {
SLANG_ASSERT(in == internal.end() && ex == external.end());
if (in == internal.end() || ex == external.end())
break;
}

currBit += width;
}
Expand Down Expand Up @@ -2069,7 +2067,7 @@ void PortConnection::makeConnections(
}

void PortConnection::serializeTo(ASTSerializer& serializer) const {
serializer.writeLink("port", port);
serializer.write("port", port);
if (port.kind == SymbolKind::InterfacePort) {
if (connectedSymbol)
serializer.writeLink("ifaceInstance", *connectedSymbol);
Expand Down
1 change: 1 addition & 0 deletions source/ast/types/Type.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -317,6 +317,7 @@ bool Type::isSimpleType() const {
case SymbolKind::FloatingType:
case SymbolKind::TypeAlias:
case SymbolKind::ClassType:
case SymbolKind::StringType:
return true;
default:
return false;
Expand Down
11 changes: 4 additions & 7 deletions source/parsing/Parser_members.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3223,13 +3223,10 @@ SpecparamDeclaratorSyntax& Parser::parseSpecparamDeclarator(SyntaxKind parentKin
closeParen = expect(TokenKind::CloseParenthesis);

if (!name.isMissing()) {
if (isPathPulse) {
if (parentKind != SyntaxKind::SpecifyBlock)
addDiag(diag::PulseControlSpecifyParent, name.range());
else if (!expr2)
addDiag(diag::PulseControlTwoValues, expr1.sourceRange()) << name.range();
}
else if (expr2) {
if (isPathPulse && parentKind != SyntaxKind::SpecifyBlock)
addDiag(diag::PulseControlSpecifyParent, name.range());

if (!isPathPulse && expr2) {
auto last = expr2->getLastToken();
SourceRange range(expr1.getFirstToken().location(),
last.location() + last.rawText().length());
Expand Down
22 changes: 22 additions & 0 deletions tests/unittests/ast/MemberTests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2756,3 +2756,25 @@ endmodule
]
})");
}

TEST_CASE("IEEE 1800 Section 10.9.2 - structure assignment patterns") {
auto tree = SyntaxTree::fromText(R"(
module top;
typedef struct {
logic [7:0] a;
bit b;
bit signed [31:0] c;
string s;
} sa;
sa s2;
initial s2 = '{int:1, default:0, string:""}; // set all to 0 except the
// array of bits to 1 and
// string to ""
endmodule
)");

Compilation compilation;
compilation.addSyntaxTree(tree);
NO_COMPILATION_ERRORS;
}
19 changes: 19 additions & 0 deletions tests/unittests/ast/ParameterTests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1211,3 +1211,22 @@ endmodule
compilation.addSyntaxTree(tree);
NO_COMPILATION_ERRORS;
}

TEST_CASE("Hierarchical recursive parameter initialization") {
auto tree = SyntaxTree::fromText(R"(
module M ();
parameter P0 = M.P1 + 1;
parameter P1 = M.P1 + 1;
parameter P2 = M.P1 + 1;
parameter P3 = M.P1 + 1;
initial $display(P0, P1, P2, P3);
endmodule
)");

Compilation compilation;
compilation.addSyntaxTree(tree);

auto& diags = compilation.getAllDiagnostics();
REQUIRE(diags.size() == 1);
CHECK(diags[0].code == diag::RecursiveDefinition);
}
14 changes: 14 additions & 0 deletions tests/unittests/ast/SystemFuncTests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1041,6 +1041,20 @@ module m;
$async$and$plane(mem, in, outBadOrder); // Bad output bit ordering
end
endmodule
module async_array(a1,a2,a3,a4,a5,a6,a7,b1,b2,b3);
input a1, a2, a3, a4, a5, a6, a7;
output b1, b2, b3;
logic [1:7] mem[1:3]; // memory declaration for array personality
logic b1, b2, b3;
initial begin
// set up the personality from the file array.dat
$readmemb("array.dat", mem);
// set up an asynchronous logic array with the input
// and output terms expressed as concatenations
$async$and$array(mem,{a1,a2,a3,a4,a5,a6,a7},{b1,b2,b3});
end
endmodule
)");

Compilation compilation;
Expand Down
19 changes: 16 additions & 3 deletions tests/unittests/parsing/MemberParsingTests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -691,6 +691,20 @@ module m;
specparam PATHPULSE$a$b = (1:2:3, 4:5:6);
endspecify
endmodule
module m1;
specify
specparam PATHPULSE$ = 1;
specparam PATHPULSE$a$b = 1;
endspecify
endmodule
module m2;
specify
specparam PATHPULSE$ = (1);
specparam PATHPULSE$a$b = (1);
endspecify
endmodule
)";

parseCompilationUnit(text);
Expand All @@ -710,10 +724,9 @@ endmodule

parseCompilationUnit(text);

REQUIRE(diagnostics.size() == 3);
REQUIRE(diagnostics.size() == 2);
CHECK(diagnostics[0].code == diag::PulseControlSpecifyParent);
CHECK(diagnostics[1].code == diag::PulseControlTwoValues);
CHECK(diagnostics[2].code == diag::PulseControlPATHPULSE);
CHECK(diagnostics[1].code == diag::PulseControlPATHPULSE);
}

TEST_CASE("Invalid package decls") {
Expand Down
1 change: 1 addition & 0 deletions tools/reflect/include/CppEmitter.h
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
#pragma once

#include "fmt/format.h"
#include "fmt/ranges.h"
#include <filesystem>
#include <fstream>
#include <ranges>
Expand Down
1 change: 1 addition & 0 deletions tools/tidy/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -88,3 +88,4 @@ CheckConfigs:
4. Create the new tidy diagnostic in the `TidyDiags.h` file.
5. Add the new check to the corresponding map in the `TidyConfig` constructor.
6. Update the documentation accordingly
7. Add the new `cpp` file to CMakeLists.txt

0 comments on commit 3b79e10

Please sign in to comment.