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.

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

[slang][port] Fix bugs #1043

Merged
merged 1 commit into from
Jul 4, 2024
Merged

Conversation

Kitaev2003
Copy link
Contributor

@Kitaev2003 Kitaev2003 commented Jul 2, 2024

Hello!

  1. This code is not serializing to json correctly. So I fixed PortConnection::serializeTo.
module top;
    wire[1:0] net1;
    m loc({net1});
endmodule

module m({net1, net2});
    input wire net1, net2;
endmodule

serialization before:

"connections": [
  {
    "port": "2199025177616 ",
    "expr": {
      "kind": "Concatenation",
      "type": "logic[1:0]",
      "operands": [
        {
          "kind": "NamedValue",
          "type": "logic[1:0]",
          "symbol": "2199025173856 net1"
        }
      ]

serialization now

"connections": [
  {
    "port": {
      "name": "",
      "kind": "MultiPort",
      "addr": 2199025177616,
      "type": "logic[1:0]",
      "direction": "In",
      "ports": [
        {
          "type": "logic",
          "direction": "In",
          "internalSymbol": "2199025175320 net1"
        },
        {
          "type": "logic",
          "direction": "In",
          "internalSymbol": "2199025175720 net2"
        }
      ]

  1. When running a debug build on this code:
module top;
    wire net1;
    m loc({net1});
endmodule

module m({net1, net2});
    input wire net1, net2;
endmodule
 internal compiler error: Assertion 'in == internal.end() && ex == external.end()' failed
  in file /etc/slang/source/ast/symbols/PortSymbols.cpp, line 1982
  function: void slang::ast::PortConnection::checkSimulatedNetTypes() const

Slang was crashing with an assert. I couldn't understand why this assert was needed. I think it should just be removed from the code.

Copy link

codecov bot commented Jul 2, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 94.71%. Comparing base (4afa7a2) to head (dacb174).

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1043      +/-   ##
==========================================
- Coverage   94.71%   94.71%   -0.01%     
==========================================
  Files         191      191              
  Lines       47658    47656       -2     
==========================================
- Hits        45138    45136       -2     
  Misses       2520     2520              
Files Coverage Δ
source/ast/symbols/PortSymbols.cpp 97.83% <100.00%> (-0.01%) ⬇️

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 4afa7a2...dacb174. Read the comment docs.

@MikePopoloski MikePopoloski merged commit 1318e77 into MikePopoloski:master Jul 4, 2024
15 checks passed
JoelSole-Semidyn added a commit to JoelSole-Semidyn/slang that referenced this pull request Jul 12, 2024
…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)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants