xml_reader.py misinterpreting XML 'crossExpr'

[apaiva]

The xml_reader.py is is interpreting the 'crossExpr' as a comma-separated-list of all points in the expression; however, the xml_writer.py (and also the accellera sepc) specify a separate 'crossExpr' XML element for each point in the cross. In otherwords, instead of having one 'crossExpr' with a CSL of the two points, there will be one 'crossExpr' element for each point.

Here's a proposal on a fix for xml_reader.py:

                cp_name = self.getAttr(cr_e, "name", "<unknown>")

                if cp_name not in cr_m.keys():
                    cp_l = []
                    # proposed change
                    for crossExpr in cr_e.iter("crossExpr"):
                      cp_n = crossExpr.text
                      logging.debug("cp_n=\"" + cp_n + "\"")
                      if cp_n in cp_m.keys():
                          cp_l.append(cp_m[cp_n][0])
                      else:
                          raise Exception("Cross %s references missing coverpoint %s" % (
                                cp_name, cp_n))
#                   # original 
#                   crossExpr = next(cr_e.iter("crossExpr"))
#                   for cp_n in crossExpr.text.split(','):
#                       logging.debug("cp_n=\"" + cp_n + "\"")
#                       if cp_n in cp_m.keys():
#                           cp_l.append(cp_m[cp_n][0])
#                       else:
#                           raise Exception("Cross %s references missing coverpoint %s" % (
#                               cp_name, cp_n))

Note, there seems to be an additional related gremlin in there somewhere, as after running merges (with PyUCIS) including this proposed fix, the resulting merged XML has only one 'crossExpr'. The single 'crossExpr' is correct, in that it is not a CSL and represents only one of the included points; however the other points do not have their own 'crossExpr'.

[apaiva]

The second half of this issue is narrowed down a bit, but I need some help in understanding the data model to get to the bottom of it. At the time of writing the XML after the XML merge, there's this datastructure representing the covergroup with the problematic cross. The MemCovergroup.m_children[X] entry for the problematic cross does correctly have two "coverpoint" entries... one for each point being crossed. However, the last "m_children" of the covergroup seeming to be another representation of the same covergroup, is what is being used to actually write the group to XML, and that version has only one "coverpoint" in the associated cross entry. My question is, "what's the concept behind this other 'copy/representation' of the group and where's the code behind creating it?"

[mballance]

Hi @apaiva, great timing in filing this issue since I'm in the code fixing another bug today.
Your suggested fix for crossExpr makes sense, and I'll incorporate the suggested change.
UCIS models both type and instance coverage using hierarchy. The upper Covergroup data structure contains type-coverage info, while the lower contains instance info. The UCIS interchange format only contains instance coverage (the expectation being that readers will reconstruct the type-coverage info), which is why the lower Covergroup copy is being written out.

I think you said you were hitting this issue on the result of a merge?

[apaiva]

Hi @mballance, yes, this is occurring in the XML write after a merge using ucis_tools -> db_merger. Something along the lines of this:

ucis_tools.py \
  merge \
  -o scratch/bunk/out.xml \
  fcov_ucis.xml \
  fcov_ucis.xml

[mballance]

Can you double check whether the unit tests pass for you? They all pass for me, and I'm looking at test_1db_2ci_2cp_1cr to see if it can easily be tweaked to show the behavior you're seeing.

[apaiva]

The unit tests are all passing on yesterday's version. I'll dig in a bit and see if the unit test is hitting this.

[apaiva]

@mballance There's another fix pushed to my master to the xml reader. Similar to the initial suggested fix you pulled in but to the cross instance parsing of the xml reader. It explains why the previous merge unit tests were not failing from it as they do a yaml read, not an xml read. The new associated unit test targeting this is passing.

[mballance]

Ah... that makes sense. Thanks for tracking this down -- changes look good to me!