Clock-crossing errors should have better positions without the -cross-info flag

[nanavati]

This is noted in the testsuite here:

bsc/testsuite/bsc.mcd/Misc/mcd.exp

Line 49 in ceee6da

# these errs should have better position w/o the -cross-info flag (bug 1238)

(and was apparently Bluespec bug 1238). This is just capturing the issue on GitHub.

[quark17]

Here are the comments from Bluespec Inc's bug 1238:

---

DESCRIPTION

In bsc.mcd/Misc/mcd.exp there is a comment that the positions in this error need to be improved:

Error: "ClockCheckCond.bsv", line 2, column 18: (G0007)
  Reference across clock domain in method out .
  Method calls by clock domain:
    Clock domain 1:
      Prelude.read at "Prelude.bs", line 1428, column 26,
    Clock domain 2:
      Prelude.read at "Prelude.bs", line 1458, column 26,

The references are probably to the "read" of the RegN and RegUN imports, and not to the places where _read is called in the user's design.

Not sure what can be done, but the exp file says that a bug should be filed.

---

COMMENT 1

A similar thing happens in another example:

Error: "ClockCheckMethodArg.bsv", line 8, column 6: (G0007)
  Reference across clock domain in rule test .
  Method calls by clock domain:
    Clock domain 1:
      Prelude.write at "Prelude.bs", line 534, column 45,
    Clock domain 2:
      Prelude.read at "ClockCheckMethodArg.bsv", line 9, column 8,

Also, the method/rule name should be put in quotes, in these messages. And the Ids should be displayed with pfPrint -- "Prelude.write" is the Classic form of qualification.

---

COMMENT 2

Oops. It appears that the exp file is compiling with the -cross-info flag on, which gives positions in the user code -- the user shouldn't have to turn on that flag to get better positions, though.

It's possible that when the original exp file comments were made, the position was worse. In any case, the positions with cross-info are still not perfect:

Error: "ClockCheckCond.bsv", line 2, column 18: (G0007)
  Reference across clock domain in method out .
  Method calls by clock domain:
    Clock domain 1:
      Prelude.read at "ClockCheckCond.bsv", line 2, column 18,
    Clock domain 2:
      Prelude.read at "ClockCheckCond.bsv", line 14, column 19,

The location of the error is on the interface declaration, not the definition inside the module. The position for clock domain 2 is correct, but the position of clock domain 1 is not (it should point to the expr inside the method definition, not to the method type declaration at the top of the file).

And in the other example:

Error: "ClockCheckMethodArg.bsv", line 8, column 6: (G0007)
  Reference across clock domain in rule test .
  Method calls by clock domain:
    Clock domain 1:
      Prelude.write at "ClockCheckMethodArg.bsv", line 9, column 5,
    Clock domain 2:
      Prelude.read at "ClockCheckMethodArg.bsv", line 9, column 8,

Here the positions are correct: The position of the error is the rule, the position of domain 2 is the register name being read, and the position of domain 1 is the <= (I convinced myself that this is the correction position, rather than pointing to the identifier or expression on the LHS of the write).

---

COMMENT 3

Looking at the example in bsc.mcd/Misc/ (from the original description), I see that Prelude positions are no longer reported. Without -cross-info, the output is this:

   Error: "ClockCheckCond.bsv", line 6, column 8: (G0007)
     Reference across clock domain in method `out'.
     Method calls by clock domain:
       Clock domain 1:
         the_y.read
       Clock domain 2:
         the_x.read

The position of the error has also changed and is now the module in question and not the interface type declaration for the method (due to improvements for Bug 861).

I also see that, instead of just reporting the method, the instance name is also reported. Having "the_x.read" is a big help over "read"!

If you turn on -cross-info, the method calls have the same positions as before:

   Error: "ClockCheckCond.bsv", line 6, column 8: (G0007)
     Reference across clock domain in method `out'.
     Method calls by clock domain:
       Clock domain 1:
         the_y.read at "ClockCheckCond.bsv", line 2, column 18,
       Clock domain 2:
         the_x.read at "ClockCheckCond.bsv", line 14, column 19,

As, before, the use of "x" is correctly pointing at "x" in the method's predicate, but the use for "y" is pointing at the type declaration for method "out", rather than at the use of "y" in the return value for the definition of "out". That should be fixed. Also, it would be nice if these positions were reported without the user having to turn on the -cross-info flag.

In the example from Comment 2, the positions are unchanged. When -cross-info is not given, no positions are displayed! Not even the one correct position. This suggests that perhaps BSC is not filtering out Prelude positions but is actually displaying no position if -cross-info is not given!

    Clock domain 1:
      r.write at "ClockCheckMethodArg.bsv", line 9, column 5,
    Clock domain 2:
      the_s.read at "ClockCheckMethodArg.bsv", line 9, column 8,

It's also interesting that, because of the 2-line instantiation of "the_s", the method is reported as "the_s.read" when it ought to be "s.read". The method is part of the interface, not the module instance.

---

[quark17]

A Bluespec Inc customer also ran into this error message and couldn't understand why the method calls were in different domains, but would have been able to identify the problem if the clocks of the two domains were given. Here's the comment on bug 1238 about it -- though it was later opened as a separate bug 1703, with two examples attached, when the customer ran into it again.

---

COMMENT 4

[Customer] also recommends giving more info about what the clock domains are. He suggested naming the domains, but where does the name come from (unless there is only one clock in the domain). A simple thing to do would be to list the clocks in the domain, like this:

   Error: "Test.bsv", line 102, column 8: (G0007)
     Reference across clock domain in rule `connect_now'.
     Method calls by clock domain:
       Clock domain 1:
         ...
       Clock domain 2:
         ...
     Clocks in domain 1:
       ...
     Clocks in domain 2:
       ...

Though, as I said to [Customer], some thought might need to go into how to list a clock -- the Verilog port name is the most available handle, but a BSV name would probably be preferred. It's just not clear that there would be one scope in which all of the BSV names would be available -- clocks coming in as parameters to the current module, clocks coming out of instantiated submodules (which might be import-BVI modules), clocks at different levels of the hierarchy where the name at that point in the hierarchy isn't the same as its name when viewed from the elaborated top-module, etc.

---