rdaly525 / coreir

BSD 3-Clause "New" or "Revised" License
101 stars 24 forks source link

Create a Directed Graph view of CoreIR #53

Closed rdaly525 closed 7 years ago

rdaly525 commented 7 years ago

This is HIGH priority so that Caleb can work on his stuff

Things we need to do to get this working.

rdaly525 commented 7 years ago

@leonardt

I have a semi-complete api for what I want the directed graph to be located in branch dview in src/directedview.hpp

Could you start working on updating the C api and python bindings for this?

leonardt commented 7 years ago

https://github.com/rdaly525/coreir/pull/65 implements initial support for the directedview api

leonardt commented 7 years ago

Reminder that I still need to implement directedinstace.get_inputs/get_outputs

rdaly525 commented 7 years ago

I just merged your pull request and a couple of minor changes into branch dview. For some reason when I call pytest, it just does nothing and errors. I am not sure what I screwed up... @leonardt could you take a look? https://travis-ci.org/rdaly525/coreir/builds/231890973 It also fails on osx.

leonardt commented 7 years ago

@rdaly525 it fails on on the call to def->validate() inside module::setDef(). It seems that this function is defined in moduledef.hpp but not in moduledef.cpp, perhaps this function was dropped in the merge?

rdaly525 commented 7 years ago

I will look into that. Out of curiosity, how did you determine that? I am seeing no failure mode

leonardt commented 7 years ago

I also saw nothing when running in GDB which was strange.

I backed off the test_directedview.py test to see which call was causing the failure (i.e. commented out source lines). Saw it was dieing when doing the module.set_definition call so I performed the same procedure inside module.setDef to see what was causing the failure.

rdaly525 commented 7 years ago

ModuleDef::validate() is defined in moduledef_validate.cpp

rdaly525 commented 7 years ago

I am using setDef in all my C++ tests. so it is curious that it only fails in python

leonardt commented 7 years ago

ah, didn't think that it would be defined in separate file, I will keep digging

leonardt commented 7 years ago

Oh, it seems that coreir is dieing but the print out was being captured by pytest

Getting this error

ERROR: Cannot wire together
  adder1.out : Bit[9]
  adder3.in1 : BitIn[8]

ERROR: Cannot wire together
  adder2.out : Bit[9]
  adder3.in2 : BitIn[8]

I AM DYING!
leonardt commented 7 years ago

Perhaps a call to validate should produce an error instead of having coreir die?

Normally pytest will dump stdout/stderr on failure but since coreir was dieing under the hood the process was killed.

rdaly525 commented 7 years ago

Ah that makes sense. Yeah passing an error would probably be better

leonardt commented 7 years ago

I will update the .travis.yml to run with pytest -s for now. The output will be a bit harder to parse since it won't capture stdout/stderr but it at least will print coreir error messages.

rdaly525 commented 7 years ago

This issue is fixed by pull request #72