vendor._gowin: fix clock name quotes for Gowin IDE

[jeanthom]

Gowin IDE seems unhappy with quotes in clock signal names. For instance, with the following SDC:

create_clock -name "clk50_0__io" -period 20.0 [get_ports
    {clk50_0__io}]

Gowin IDE will complain with the following error message:

ERROR  (TA2000) : "top.sdc":2 | 'syntax error' near token '-period'

Replacing quotes with curly braces fixes the issue. I tested this fix on Amaranth 0.5.1.

[whitequark]

I don't think this change is correct, as it doesn't handle { and } that may appear in the middle of a signal name. (We accept such signal names currently.)

As far as I know there is no way to escape a lone { in a {-delimited Tcl string. However, matched {-} pairs are OK. We should have a new filter, tcl_escape_brace, for targets that do not accept quoted strings because they don't implement the Tcl lexer. This filter would ensure that the string is balanced (i.e. name.count('{') == name.count('}')) or raise an exception.

[jeanthom]

Hi and thanks for your review,

I'm not sure what you're expecting from the tcl_escape_brace filter. Should it be just something enclosing a string in braces and checking that every opening brace pairs with a closing brace? Something in the likes of this?

        def tcl_escape_brace(string):
            if string.count("{") != string.count("}"):
                raise ValueError("Unbalanced braces in string")

            return f"{{{string}}}"

Also, it seems like Gowin's SDC parser really doesn't like braces in names. Both of the following statements generate syntax errors:

create_clock -name clk50_0__io{foobar} -period 20.0 [get_ports
    {clk50_0__io}]
create_clock -name {clk50_0__io{foobar}} -period 20.0 [get_ports
    {clk50_0__io}]

So I'm not sure that what we're trying to fix here is meaningful. FWIW I'm running the latest version available of Gowin IDE (V1.9.10.02 build 75964).

[whitequark]

Also, it seems like Gowin's SDC parser really doesn't like braces in names.

Oh, depressing. So they don't even try to accept correct Tcl. (The reason I bring up braces in names at all is that using [] in names, like to indicate an array, breaks gtkwave, and using () in names, last I checked, was segfaulting Quartus. So using {} could be handy to work that around.)

[whitequark]

I think I'll disassemble their Tcl parser and take a look to see what the grammar is, and if that fails to happen in a timely manner I'll merge this PR as-is.

[whitequark]

The lexer lives in the function sdclex in libgowin.so. It's made using flex, and it's challenging to figure out what exact grammar it accepts from either the side of the input (where you'd have to reconstruct a DFA from yy_nxt and friends), or the side of the output (where the diagnostic is written by msgOut but the way it's invoked is convoluted).

[whitequark]

Thanks!