veryl-lang / veryl

Veryl: A Modern Hardware Description Language
Other
439 stars 20 forks source link

Allow using ModportVariableMember as factor #785

Closed nananapo closed 2 weeks ago

nananapo commented 2 weeks ago

In #753, CheckExpression pass raises InvalidFactor Error when ModportVariableMember is used as factor. But it's incorrect.

interface InterfaceA {
    var a: logic;
    modport master {
        a: input ,
    }
}

module ModuleA (
    clk: input clock,
    mst: modport InterfaceA::master,
) {
    always_ff {
        $display("%d", mst.a);
    }
}
  ๐Ÿ’ฅ a of kind "modport variable member" cannot be used as a factor in an
  โ”‚ expression
    โ•ญโ”€[:12:1]
 12 โ”‚     always_ff {
 13 โ”‚         $display("%d", mst.a);
    ยท                        โ”€โ”€โ”ฌโ”€โ”€
    ยท                          โ•ฐโ”€โ”€ Error location
 14 โ”‚     }
    โ•ฐโ”€โ”€โ”€โ”€
  help: remove modport variable member from expression
taichi-ishitani commented 2 weeks ago

I think it would be nice to add a testcase for this case.

nblei commented 2 weeks ago

Easy fix, and good suggestion, Taichi. Very surprising that there wasn't a test case which included a modport item already.

Question, though: what is a GenericInstance, and should they be useable in an expression?

On Wed, Jun 12, 2024 at 11:12โ€ฏAM Taichi Ishitani @.***> wrote:

I think it would be nice to add a testcase for this case.

โ€” Reply to this email directly, view it on GitHub https://urldefense.com/v3/__https://github.com/veryl-lang/veryl/pull/785*issuecomment-2163429699__;Iw!!DZ3fjg!-QpfpcZR1AXhWID48rwTfUor4U4Maf2xiDPFH3VpXP0qLB_pgqPe7WsC7RmtWbT3qrgCxqqe5Pa1_i9OE54HWqX9q79b$, or unsubscribe https://urldefense.com/v3/__https://github.com/notifications/unsubscribe-auth/AFAMUBDKTFINZGAHV4XOC43ZHBXNFAVCNFSM6AAAAABJGQ33SWVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDCNRTGQZDSNRZHE__;!!DZ3fjg!-QpfpcZR1AXhWID48rwTfUor4U4Maf2xiDPFH3VpXP0qLB_pgqPe7WsC7RmtWbT3qrgCxqqe5Pa1_i9OE54HWiRVwc7K$ . You are receiving this because you are subscribed to this thread.Message ID: @.***>

-- Very Respectfully,

Nathaniel Bleier @.*** 773-531-8229 (Cell)

nblei commented 2 weeks ago

786 fixes several associated issues.

dalance commented 2 weeks ago

Question, though: what is a GenericInstance, and should they be useable in an expression?

GenericInstance is a module or interface instance with generic parameters. I think it can't be used in an expression.

inst a: ModuleA::<10>(); // a is GenericInstance

786 seems to cover this PR, so I'll merge this first.