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 #204

Closed
wants to merge 13 commits into from
Closed

Adds Value.matches #204

wants to merge 13 commits into from

Conversation

RobertBaruch
Copy link

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

@codecov
Copy link

codecov bot commented Sep 12, 2019

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/hdl/ast.py Outdated Show resolved Hide resolved
nmigen/hdl/ast.py Outdated Show resolved Hide resolved
@RobertBaruch
Copy link
Author

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

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

@whitequark
Copy link
Contributor

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/hdl/ast.py Outdated Show resolved Hide resolved
@RobertBaruch
Copy link
Author

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.

@whitequark
Copy link
Contributor

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

@whitequark
Copy link
Contributor

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

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

@whitequark
Copy link
Contributor

I'm confused. Did you delete all tests?

@whitequark
Copy link
Contributor

Implemented in master.

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.

Add Value.match(...), an operator with the same semantics as Case(...)
2 participants