Fix Issue with saving nodes

[fconlon]

• [x] Cache Edit Window
• [x] Comm Edit Window
• [x] Core Edit Window
• [x] Ext Edit Window
• [x] ISA Edit Window
• [x] Inst Edit Window
• [x] MCtrl Edit Window
• [x] Pseudo Inst Edit Window
• [x] Reg Edit Window
• [x] Reg Class Edit Window
• [x] SoC Edit Window
• [x] Spad Edit Window
• [x] VTP Edit Window

Criteria 1

Given:

• A node is being added to the DAG

When:

• The user clicks save

Then:

• The resulting file produces correct yaml

[fconlon]

This works correctly when updating but not when adding a node. Incorrectly formed yaml is my initial suspect, but may not be the ultimate cause. Initial look at the write yaml doesn't appear to show anything wrong, so stepping through with gdb.

[fconlon]

intermediate file is not being created

[fconlon]

In the case of comms, issue appears to be a problem with writing out yaml when endpoints vector is empty.

[fconlon]

Empty fields lead to yaml in the format

Endpoints: []

This is causing problems for reads. https://github.com/opensocsysarch/CoreGen/issues/189 Issue has been created on the backend for fixing this.

[jleidel]

In the case of comms, issue appears to be a problem with writing out yaml when endpoints vector is empty.

This should be a simple fix to the readers and writers for comm nodes. I can take a look at this later today.

[fconlon]

That works. I was going to take a look at it'll I'll put together a list of the nodes that I'm allowing to be null on the front end. Give me a few min

[fconlon]

These fields are currently allowed to be null on the front end and will likely need changes on the backend. In general I am allowing all fields that point to other nodes to be null so that the user can add/modify independent of other nodes in the DAG. Those fields will only error if something is in the field and it does not point to an already created node. I can change this behavior for the time being if it would make things easier for our first iteration. Having figured out this error I will have to go back and double check the current add/modify functionality for when fields are null.

note: Node name must never be null

cache:

• child node

comm:

• endpoints

core:

• all fields but thread units should be nullable

inst:

• all fields nullable

pinst:

• target inst and encodings nullable

reg:

• subreg fields should be nullable

regclass:

• registers fields should be nullable

soc:

• cores nullable

I created a uifacilitation branch for this since i figured this would be pretty extensive. If you need to work on other things I can take this on

[fconlon]

I haven't looked at all of these yet, but empty child chaches seem to be ok as I'm going through my sad path verification. I'll check off things in #95 if I don't find anything that needs fixed relating to this issue.

[fconlon]

Apologies for the barrage of comments. I put a checklist here instead of using the one in #95 since that issue is intended to just be the UI portion. I'll check off things in this issue related to saving the yaml.

[fconlon]

Notes: Core failing when trying to read a null ISA

[fconlon]

Seg Faulting when reading Psuedo Insts if the IF of the target Inst is null

[jleidel]

I wrote a series of tests for the COMM nodes as a first step. The writers are correct. The YAML reader throws an error if there are no endpoints in place. I can change this. I'll have to start working down the list of nodes, writing tests and adjusting the YAML Reader/Writer functions

[jleidel]

COMM support in https://github.com/opensocsysarch/CoreGen Commit a994674

[jleidel]

CACHE support in opensocsysarch/CoreGen@1a57f01

[jleidel]

As an aside, any time we want to support a "null" node, it should be done using the Yaml reader/writers. In this way, the attribute with a NULL value will be omitted at Yaml emission time. For example, for a core:

INCORRECT: `Cores:

• Core: TEST82.core0 ThreadUnits: 1 Cache: TEST82.core0.L1.cache ISA: RegisterClasses:
  • RegClass: TEST82.regclass
  • RegClass: TEST82.csrregclass Extensions:
  • Extension: TEST82.ext0`

CORRECT: `Cores:

• Core: TEST82.core0 ThreadUnits: 1 Cache: TEST82.core0.L1.cache RegisterClasses:
  • RegClass: TEST82.regclass
  • RegClass: TEST82.csrregclass Extensions:
  • Extension: TEST82.ext0`

[jleidel]

CORE nodes are fixed in opensocsysarch/CoreGen@ 13f8ba8a9f4c880baf05c6ea2afc2113dadbb531

[jleidel]

INST nodes are fixed in opensocsysarch/CoreGen@dfaa82b

[jleidel]

PINST nodes are fixed in opensocsysarch/CoreGen@3d29128

[jleidel]

one additional note... these changes were done on the 'devel' branch as opposed to the uifacilitation branch...

[jleidel]

I'm going to disagree with SubReg fields with NULL attributes. I see no reason to permit this. Each SubReg field must include a unique name and contain start+end bit positions. Its certainly permissible to assign these arbitrary bit values (within the scope of the register width), but I see no reason to permit them to be null

[jleidel]

REGCLASS nodes are fixed in opensocsysarch/CoreGen@2dedf1d

[jleidel]

SOC nodes are fixed in opensocsysarch/CoreGen@8bca557

[fconlon]

I thought I had done them on the ui facilitation branch >.<. That's my mistake. I must have changed it to look at something and forgotten to change it back. I've been doing my null node support in the read/write yaml. I've been commiting it to https://github.com/opensocsysarch/CoreGen/issues/189 . If the YAML has an empty list "ISA:" it won't read correctly in the UI so I've been doing it there. I agree that it doesn't make sense for a subregister to not have a start and end bit. I meant that as a register is allowed to not have subregisters. As of right now I believe that it errors the field and highlights it red if a subregister is added that doesn't have a start/end bit.

[jleidel]

opensocsysarch/CoreGen#189

If the YAML has an empty node "ISA:", then it should fail. If you look at the changes I committed, the Writer code simply won't emit the "ISA:" if it isn't defined for the respective node. In which case, when you read the Yaml back in, the ISA value becomes a 'nullptr'.

Registers are definitely permitted to omit the subregisters. All the general purpose registers are "flat" and void of subregisters. Check the latest series of commits in the 'devel' tree. There are a number of new tests to work through all the various reader/writer states.

[fconlon]

I'll try to take a look at it tonight. I'll rebase onto the uifacilitation branch and double check before I make any commits or push. I'll go back and look to make sure I didn't leave any dangling empty fields like that as well. I know I removed the emits for several of them when I was making my changes, but I definitely want to make a second pass to make sure I didn't miss any before I say everything is ready to go.