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

Adds Value.matches #205

Closed
nmigen-issue-migration opened this issue Sep 12, 2019 · 8 comments
Closed

Adds Value.matches #205

nmigen-issue-migration opened this issue Sep 12, 2019 · 8 comments

Comments

@nmigen-issue-migration
Copy link

Issue by RobertBaruch
Thursday Sep 12, 2019 at 01:58 GMT
Originally opened as m-labs/nmigen#204


Allows expressions like s.matches(37) and s.matches("1---000")

Resolved issue.

Tested:

  • Added test cases
  • By inspection using this sample module:
from nmigen import *
from nmigen.cli import main
from nmigen.asserts import *


class TestCase(Elaboratable):
    def __init__(self):
        self.input = Signal(8)
        self.output = Signal()

    def elaborate(self, platform):
        m = Module()

        with m.If(self.input.matches(7)):
            m.d.comb += self.output.eq(1)
        with m.Elif(self.input.matches("1---0000")):
            m.d.comb += self.output.eq(1)
        with m.Else():
            m.d.comb += self.output.eq(0)
        return m


if __name__ == "__main__":
    clk = Signal()
    rst = Signal()

    pos = ClockDomain()
    pos.clk = clk
    pos.rst = rst

    testcase = TestCase()

    m = Module()
    m.domains.pos = pos
    m.submodules.testcase = testcase

    main(
        m,
        ports=[clk, rst, testcase.input, testcase.output],
        platform="formal")

This properly translates to il:

attribute \generator "nMigen"
attribute \nmigen.hierarchy "top.testcase"
module \testcase
  attribute \src "test_case.py:8"
  wire width 8 input 0 \input
  attribute \src "test_case.py:9"
  wire width 1 output 1 \output
  attribute \src "test_case.py:14"
  wire width 1 $1
  attribute \src "test_case.py:14"
  cell $eq $2
    parameter \A_SIGNED 0
    parameter \A_WIDTH 8
    parameter \B_SIGNED 0
    parameter \B_WIDTH 3
    parameter \Y_WIDTH 1
    connect \A \input
    connect \B 3'111
    connect \Y $1
  end
  attribute \src "test_case.py:16"
  wire width 8 $3
  attribute \src "test_case.py:16"
  cell $and $4
    parameter \A_SIGNED 0
    parameter \A_WIDTH 8
    parameter \B_SIGNED 0
    parameter \B_WIDTH 8
    parameter \Y_WIDTH 8
    connect \A \input
    connect \B 8'10001111
    connect \Y $3
  end
  attribute \src "test_case.py:16"
  wire width 1 $5
  attribute \src "test_case.py:16"
  cell $eq $6
    parameter \A_SIGNED 0
    parameter \A_WIDTH 8
    parameter \B_SIGNED 0
    parameter \B_WIDTH 8
    parameter \Y_WIDTH 1
    connect \A $3
    connect \B 8'10000000
    connect \Y $5
  end
  process $group_0
    assign \output 1'0
    attribute \src "test_case.py:14"
    switch { $5 $1 }
      attribute \src "test_case.py:14"
      case 2'-1
        assign \output 1'1
      attribute \src "test_case.py:16"
      case 2'1-
        assign \output 1'1
      attribute \src "test_case.py:18"
      case
        assign \output 1'0
    end
    sync init
  end
end
attribute \generator "nMigen"
attribute \top 1
attribute \nmigen.hierarchy "top"
module \top
  attribute \src "test_case.py:24"
  wire width 1 input 0 \clk
  attribute \src "test_case.py:25"
  wire width 1 input 1 \rst
  attribute \src "test_case.py:8"
  wire width 8 input 2 \input
  attribute \src "test_case.py:9"
  wire width 1 output 3 \output
  cell \testcase \testcase
    connect \input \input
    connect \output \output
  end
end

And to Verilog:

/* Generated by Yosys 0.9+36 (git sha1 4aa505d1, clang 6.0.0-1ubuntu2 -fPIC -Os) */

(* \nmigen.hierarchy  = "top.testcase" *)
(* generator = "nMigen" *)
module testcase(\output , \input );
  (* src = "test_case.py:14" *)
  wire \$1 ;
  (* src = "test_case.py:16" *)
  wire [7:0] \$3 ;
  (* src = "test_case.py:16" *)
  wire \$5 ;
  (* src = "test_case.py:8" *)
  input [7:0] \input ;
  (* src = "test_case.py:9" *)
  output \output ;
  reg \output ;
  assign \$1  = \input  == (* src = "test_case.py:14" *) 3'h7;
  assign \$3  = \input  & (* src = "test_case.py:16" *) 8'h8f;
  assign \$5  = \$3  == (* src = "test_case.py:16" *) 8'h80;
  always @* begin
    (* src = "test_case.py:14" *)
    casez ({ \$5 , \$1  })
      /* src = "test_case.py:14" */
      2'b?1:
          \output  = 1'h1;
      /* src = "test_case.py:16" */
      2'b1?:
          \output  = 1'h1;
      /* src = "test_case.py:18" */
      default:
          \output  = 1'h0;
    endcase
  end
endmodule

(* \nmigen.hierarchy  = "top" *)
(* top =  1  *)
(* generator = "nMigen" *)
module top(rst, \input , \output , clk);
  (* src = "test_case.py:24" *)
  input clk;
  (* src = "test_case.py:8" *)
  input [7:0] \input ;
  (* src = "test_case.py:9" *)
  output \output ;
  (* src = "test_case.py:25" *)
  input rst;
  testcase testcase (
    .\input (\input ),
    .\output (\output )
  );
endmodule

RobertBaruch included the following code: https://github.com/m-labs/nmigen/pull/204/commits

@nmigen-issue-migration
Copy link
Author

Comment by codecov[bot]
Thursday Sep 12, 2019 at 04:31 GMT


Codecov Report

Merging #204 into master will decrease coverage by 0.27%.
The diff coverage is 10%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #204      +/-   ##
==========================================
- Coverage   82.75%   82.48%   -0.28%     
==========================================
  Files          33       33              
  Lines        5340     5360      +20     
  Branches     1145     1151       +6     
==========================================
+ Hits         4419     4421       +2     
- Misses        795      813      +18     
  Partials      126      126
Impacted Files Coverage Δ
nmigen/hdl/ast.py 84.1% <10%> (-1.82%) ⬇️

Continue to review full report at Codecov.

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

@nmigen-issue-migration
Copy link
Author

Comment by RobertBaruch
Saturday Sep 14, 2019 at 17:03 GMT


If this looks good, should I modify so that we can accept more than one value?

  self.input.matches(1, 5, 8, 99)

@nmigen-issue-migration
Copy link
Author

Comment by whitequark
Saturday Sep 14, 2019 at 17:06 GMT


Please do. I'll take one more look at the error messages to make sure they're consistent with the ones in Case and merge.

@nmigen-issue-migration
Copy link
Author

Comment by RobertBaruch
Saturday Sep 14, 2019 at 19:13 GMT


Sorry about the additional commits -- apparently that's what syncing a fork onto upstream/master does. And I don't see code coverage kicking off, either, so it's possible this commit (pretty much like anything I do with git more complicated than "submit this set of files") is borked.

@nmigen-issue-migration
Copy link
Author

Comment by whitequark
Saturday Sep 14, 2019 at 19:15 GMT


I'll just merge it locally. Will take a look somewhere in a few hours.

@nmigen-issue-migration
Copy link
Author

Comment by whitequark
Saturday Sep 14, 2019 at 20:38 GMT


And I don't see code coverage kicking off, either

I think codecov updates the very first comment each time it finishes?

@nmigen-issue-migration
Copy link
Author

Comment by whitequark
Saturday Sep 14, 2019 at 20:38 GMT


I'm confused. Did you delete all tests?

@nmigen-issue-migration
Copy link
Author

Comment by whitequark
Saturday Sep 14, 2019 at 21:06 GMT


Implemented in master.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

1 participant